[PATCH] D42475: [ELF] Add warnings for various symbols that cannot be ordered

Rui Ueyama via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Feb 1 12:12:01 PST 2018


ruiu added inline comments.


================
Comment at: ELF/Writer.cpp:1036-1038
+    bool Inserted = SymbolOrder.insert({S, {Priority++, false}}).second;
+    if (Config->WarnSymbolOrdering && !Inserted)
+      warn("symbol ordering file: symbol '" + S + "' specified multiple times");
----------------
ruiu wrote:
> Actually I'd do this in the driver so that we can make the writer focus on writing.
At least you can move this to the driver, no?


================
Comment at: ELF/Writer.cpp:1048
+        continue;
+      OrderEntry->second.second = true;
+
----------------
I don't like `second.second` because it's not that meaningful. Can you define a struct for this?


================
Comment at: ELF/Writer.cpp:1068
       int &Priority = SectionOrder[D->Section];
-      Priority = std::min(Priority, SymbolOrder.lookup(D->getName()));
+      Priority = std::min(Priority, OrderEntry->second.first);
     }
----------------
I wouldn't repeat `OrderEntry->second`. Instead, I'd assign it to a variable with no `auto` type.


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D42475





More information about the llvm-commits mailing list