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

George Rimar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Feb 12 01:43:35 PST 2018


grimar added inline comments.


================
Comment at: ELF/Driver.cpp:598
+    if (!Names.insert(S) && Config->WarnSymbolOrdering)
+      warn("symbol ordering file: symbol '" + S + "' specified multiple times");
+  }
----------------
jhenderson wrote:
> ruiu wrote:
> > 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).
> I have a slight concern that "duplicate symbol" could cause confusion with a multiply-defined symbol error. How about "duplicate ordered symbol"? Also, unless we store the order file name somewhere (e.g. in the Config variable), we can't use it in the other warnings, so I've used the owning file of the symbol instead. That still leaves the "no such symbol warning" however without an easily identified file.
I think we often include command line option name to error message, for example: 

```
error("max-page-size: value isn't a power of 2");
...
error("-image-base: number expected, but got " + S);
```

So to avoid confusion and make message shorter, we could probably use something like:
`warn("--warn-symbol-ordering: " + MB.getBufferIdentifier() + ": duplicate symbol: " + S);`


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D42475





More information about the llvm-commits mailing list