[PATCH] D39511: [ELF] Support expressions with -defsym option

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added inline comments.


================
Comment at: ELF/ScriptParser.cpp:1342
+
+void elf::readDefsym(MemoryBufferRef MB) {
+  ScriptParser(MB).readDefsym();
----------------
phosek wrote:
> ruiu wrote:
> > Actually, I think a better way of doing this is to pass two StringRefs, one for symbol name and the other for expression to this function.  Introducing a different tokenization scheme only to separate "<sym>=<expr>" into <sym> and <expr> seems a bit too much. You probably should split a -defsym argument into two StringRefs in the driver.
> That's what I've done in the first version: https://reviews.llvm.org/D39511?id=121190. Would you prefer that approach? The current approach is more akin to what gold and BFD.ld does. The main advantage of this approach is that the error messages that come out of script parser have the right offset, i.e. for `-defsym=foo=bar`, the error at offset 1 points to `f` while with the original approach it'd be `b`.
Yes, I think I prefer that over this patch. (A reason why you added defsym'ed symbols to the symbol table using addSymbolAlias in that patch isn't clear to me though.)


Repository:
  rL LLVM

https://reviews.llvm.org/D39511