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

[Puyan Lotfi via Phabricator via llvm-commits]

plotfi marked 7 inline comments as done.
plotfi added inline comments.


================
Comment at: llvm/tools/llvm-ifs/llvm-ifs.cpp:426
+      if (Stub.ObjectFileFormat == "")
+        Stub.ObjectFileFormat = TargetStub->ObjectFileFormat;
+
----------------
compnerd wrote:
> I would probably just write this as a ternary.
So Stub.ObjectFileFormat = !Stub.ObjectFileFormat.empty() ? Stub.ObjectFileFormat : TargetStub->ObjectFileFormat; ? That doesn't seem that much cleaner to read to me than the if


================
Comment at: llvm/tools/llvm-ifs/llvm-ifs.cpp:441
+      if (Stub.ObjectFileFormat != TargetStub->ObjectFileFormat &&
+          "" != TargetStub->ObjectFileFormat) {
         WithColor::error() << "Interface Stub: ObjectFileFormat Mismatch."
----------------
compnerd wrote:
> Why the inverse check style here?
A little less error prone? Should I change it to the other way around? 


================
Comment at: llvm/tools/llvm-ifs/llvm-ifs.cpp:501
       }
-      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;
+       if (Symbol.Weak != SI->second.Weak) {
+        Symbol.Weak = false;
----------------
compnerd wrote:
> Indentation seems off?   Does this ensure that the rest of the symbol is identical?  What happens if one of the size is different?
Yeah I realized this and changed it locally. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70834/new/

https://reviews.llvm.org/D70834