[PATCH] D38239: [ELF] - Define linkerscript symbols early.

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added inline comments.


================
Comment at: ELF/LinkerScript.cpp:149
 
+void LinkerScript::handleAssignment(SymbolAssignment *Cmd) {
+  if (Cmd->Name == ".")
----------------
Please always write a function comment and choose an appropriate function name if a function is not trivial. Usually if there are more than one way to "handle" something, you should avoid naming a function "handle", as it is ambiguous. I expected that this function would handle assignments (because the function name says so), but at the beginning it returns if it is an assignment to ".", which is obviously odd.


================
Comment at: ELF/LinkerScript.cpp:178
+      // underlying expression and temporarily swap it with dummy one.
+      SaveAndRestore<Expr> R(Cmd->Expression, [] { return ExprValue(0); });
+      handleAssignment(Cmd);
----------------
Please avoid using some rarely used magical RAII class.


https://reviews.llvm.org/D38239