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

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added inline comments.


================
Comment at: ELF/Driver.cpp:596
+  SetVector<StringRef> Names;
+  for (StringRef S : args::getLines(MB)) {
+    if (!Names.insert(S) && Config->WarnSymbolOrdering)
----------------
Remove {}


================
Comment at: ELF/Driver.cpp:598
+    if (!Names.insert(S) && Config->WarnSymbolOrdering)
+      warn("symbol ordering file: symbol '" + S + "' specified multiple times");
+  }
----------------
I prefer the following style

  warn(MB.getBufferIdentifier() + ": duplicate symbol: " + S);

because (1) it is easier to parse by script, and (2) easier to read if symbol name is extremely long (which is actually common for C++ programs).


================
Comment at: ELF/Writer.cpp:1047
       auto *D = dyn_cast<Defined>(Sym);
+      auto OrderEntry = SymbolOrder.find(Sym->getName());
+      if (OrderEntry == SymbolOrder.end())
----------------
`It` is a conventional name for this.


================
Comment at: ELF/Writer.cpp:1050
+        continue;
+      SymbolOrderEntry &EntryValue = OrderEntry->second;
+      EntryValue.Present = true;
----------------
`Ent` is perhaps more conventional.


================
Comment at: ELF/Writer.cpp:1076
+  if (Config->WarnSymbolOrdering)
+    for (auto OrderEntry : SymbolOrder) {
+      if (!OrderEntry.second.Present)
----------------
Remove {}


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D42475