[PATCH] D38846: [ELF] - Linkerscript: fix issue with SUBALIGN.

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: ELF/ScriptParser.cpp:645
 
+static Expr checkAlignment(Expr E, std::string &Loc) {
+  return [=] {
----------------
ruiu wrote:
> I'd make this a member of ScriptParser to eliminate `Loc`.
That will not work.
If I take current `Location` in member instead of passing it as parameter in `Expr ScriptParser::readPrimary()`,
location will be different. That is why we pass location to other places, for example to 'checkIfExists'.


================
Comment at: ELF/ScriptParser.cpp:649
+    if (!isPowerOf2_64(Alignment))
+      error(Loc + ": alignment must be power of 2");
+    return Alignment;
----------------
ruiu wrote:
> You should return 1 instead of some erroneous value.
It does not make sence I think. 
What we should do here is to report an error and a value that is not 0, so that possible alignTo() call will not asset.
My code already do that.


https://reviews.llvm.org/D38846