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

[Rui Ueyama via Phabricator via llvm-commits]

ruiu accepted this revision.
ruiu added a comment.
This revision is now accepted and ready to land.

LGTM

I think this is much better than the previous version.



================
Comment at: ELF/Driver.cpp:1047-1048
+    std::tie(From, To) = StringRef(Arg->getValue()).split('=');
+    MemoryBufferRef MB(To, "-defsym");
+    readDefsym(From, MB);
+  }
----------------
nit: you can eliminate by MB by `readDefsym(From, MemoryBufferRef(To, "-defsym"))`.


================
Comment at: ELF/ScriptParser.cpp:274
+void ScriptParser::readDefsym(StringRef Name) {
+  Expr E = readExpr();
+  SymbolAssignment *Cmd = make<SymbolAssignment>(Name, E, getCurrentLocation());
----------------
After this, you should call atEOF() to make sure that you've read all characters. Otherwise, I think you'll miss an error in `-defsym="(1+2)++++" for example, as readExpr will return at the closing parenthesis.


Repository:
  rL LLVM

https://reviews.llvm.org/D39511