[llvm] llvm-ifs: Support for handling empty IFS and merging weak+strong symbols.

[plotfi]

Summary

The following changes enable llvm-ifs to handle the following merge conflicts:

* Weak + Strong symbol merging for the same symbol
* empty vs non-empty triple field
* empty vs non-empty object file format

[compnerd]

llvm/tools/llvm-ifs/llvm-ifs.cpp

426

I would probably just write this as a ternary.

441

Why the inverse check style here?

450

Similar

501

Indentation seems off? Does this ensure that the rest of the symbol is identical? What happens if one of the size is different?

[plotfi]

llvm/tools/llvm-ifs/llvm-ifs.cpp

426

So Stub.ObjectFileFormat = !Stub.ObjectFileFormat.empty() ? Stub.ObjectFileFormat : TargetStub->ObjectFileFormat; ? That doesn't seem that much cleaner to read to me than the if

441

A little less error prone? Should I change it to the other way around?

501

Yeah I realized this and changed it locally.

[plotfi]

Updated based on @compnerd's suggestions.

[plotfi]

fixing indentation

[plotfi]

Update based on @compnerd's feedback

[plotfi]

llvm/tools/llvm-ifs/llvm-ifs.cpp

501

The Type and Size checks are above before the Weak/Strong check. So yes they will be identical. I should add a test case though that exercises this.

[compnerd]

LGTM with the additional test for the size/type mismatch validation.

[plotfi]

llvm/tools/llvm-ifs/llvm-ifs.cpp

501

I won't land until I add a lit test exercising strong/weak mismatch plus size mismatch.

[plotfi]

adding tests for weak/strong mismatch that is also a size/type mismatch for a given symbol.

llvm/test/tools/llvm-ifs/conflict-weak.ifs

-# RUN: not llvm-ifs -action write-ifs -o - %s %S/func.ifs 2>&1 | \
+# RUN: llvm-ifs -action write-ifs -o - %s %S/func.ifs 2>&1 | \
 # RUN: FileCheck %s --check-prefixes=CHECK-IFS
 
-# Here we are testing to see if two symbols with identical names will fail to
-# merge in conflict due to one being weak and one not. Eventually this will work
-# when llvm-ifs has support for resolving these kinds of conflicts.
-# CHECK-IFS: error: Interface Stub: Weak Mismatch for a.
-# CHECK-IFS-NEXT: Filename:
-# CHECK-IFS-NEXT: Weak Values: 1 0
+# CHECK-IFS: Symbols:
+# CHECK-IFS-NEXT: a: { Type: Func, Weak: true }
 
 --- !experimental-ifs-v1
 IfsVersion: 1.0

llvm/test/tools/llvm-ifs/default-empty.ifs

+# RUN: llvm-ifs -action write-ifs -o - %s | FileCheck --check-prefixes=CHECK-DEFAULT %s
+# RUN: llvm-ifs -action write-ifs -o - %s %S/weak.ifs | FileCheck --check-prefixes=CHECK-MERGE %s
+
+# CHECK-DEFAULT: --- !experimental-ifs-v1
+# CHECK-DEFAULT-NEXT: IfsVersion:      1.2
+# CHECK-DEFAULT-NEXT: Triple:          ''
+# CHECK-DEFAULT-NEXT: ObjectFileFormat: ELF
+# CHECK-DEFAULT-NEXT: Symbols:         {}
+# CHECK-DEFAULT-NEXT: ...
+
+# CHECK-MERGE: --- !experimental-ifs-v1
+# CHECK-MERGE-NEXT: IfsVersion: 1.0
+# CHECK-MERGE-NEXT: Triple: x86_64-unknown-linux-gnu
+# CHECK-MERGE-NEXT: ObjectFileFormat: ELF
+# CHECK-MERGE-NEXT: Symbols:
+# CHECK-MERGE-DAG:   _Z8weakFuncv: { Type: Func, Weak: true }
+# CHECK-MERGE-DAG:   _Z10strongFuncv: { Type: Func }
+# CHECK-MERGE: ...
+
+--- !experimental-ifs-v1
+IfsVersion:      1.2
+Triple:          ''
+ObjectFileFormat: ELF
+Symbols:         {}
+...

llvm/test/tools/llvm-ifs/strong.ifs

+# RUN: llvm-ifs -action write-ifs -o - %s %S/strong.ifs | FileCheck %s --check-prefixes=CHECK-IFS
+
+# CHECK-IFS: --- !experimental-ifs-v1
+# CHECK-IFS-NEXT: IfsVersion: 1.0
+# CHECK-IFS-NEXT: Triple: x86_64-unknown-linux-gnu
+# CHECK-IFS-NEXT: ObjectFileFormat: ELF
+# CHECK-IFS-NEXT: Symbols:
+# CHECK-IFS-DAG:   _Z8weakFuncv: { Type: Func }
+# CHECK-IFS: ...
+
+--- !experimental-ifs-v1
+IfsVersion: 1.0
+Triple: x86_64-unknown-linux-gnu
+ObjectFileFormat: ELF
+Symbols:
+  _Z8weakFuncv: { Type: Func }
+...

llvm/tools/llvm-ifs/llvm-ifs.cpp

       Stub.SOName = TargetStub->SOName;
       Stub.NeededLibs = TargetStub->NeededLibs;
     } else {
+      if (Stub.ObjectFileFormat == "")
+        Stub.ObjectFileFormat = TargetStub->ObjectFileFormat;
+
       if (Stub.IfsVersion != TargetStub->IfsVersion) {
         if (Stub.IfsVersion.getMajor() != IFSVersionCurrent.getMajor()) {
           WithColor::error()

         if (TargetStub->IfsVersion > Stub.IfsVersion)
           Stub.IfsVersion = TargetStub->IfsVersion;
       }
-      if (Stub.ObjectFileFormat != TargetStub->ObjectFileFormat) {
+      if (Stub.ObjectFileFormat != TargetStub->ObjectFileFormat &&
+          "" != TargetStub->ObjectFileFormat) {
         WithColor::error() << "Interface Stub: ObjectFileFormat Mismatch."
                            << "\nFilenames: " << PreviousInputFilePath << " "
                            << InputFilePath << "\nObjectFileFormat Values: "

                            << TargetStub->ObjectFileFormat << "\n";
         return -1;
       }
-      if (Stub.Triple != TargetStub->Triple) {
+      if (Stub.Triple != TargetStub->Triple &&
+          "" != TargetStub->Triple) {
         WithColor::error() << "Interface Stub: Triple Mismatch."
                            << "\nFilenames: " << PreviousInputFilePath << " "
                            << InputFilePath

 
         return -1;
       }
        if (Symbol.Weak != SI->second.Weak) {
-        // TODO: Add conflict resolution for Weak vs non-Weak.
-        WithColor::error() << "Interface Stub: Weak Mismatch for "
-                           << Symbol.Name << ".\nFilename: " << InputFilePath
-                           << "\nWeak Values: " << SI->second.Weak << " "
-                           << Symbol.Weak << "\n";
-
-        return -1;
+        Symbol.Weak = false;
+        continue;
       }
       // TODO: Not checking Warning. Will be dropped.
     }