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

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added inline comments.


================
Comment at: ELF/ScriptParser.cpp:647
+  return [=] {
+    uint64_t Alignment = std::max((uint64_t)1, E().getValue());
+    if (!isPowerOf2_64(Alignment)) {
----------------
The use of std::max is not obvious. I prefer

  uint64_t Alignment = E().getValue();
  if (Alignment == 0)
    return (uint64_t)1;
  if (!isPowerOf2_64...


================
Comment at: ELF/ScriptParser.cpp:650
+      error(Loc + ": alignment must be power of 2");
+      return (uint64_t)1; // Return a dummy value.
+    }
----------------
Remove the comment because it is obvious.


================
Comment at: test/ELF/linkerscript/subalign.s:26
+## Test we do not assert or crash when dot(.) is used inside SUBALIGN. 
+## ld.bfd does not allow to use dot in such expressions, our behavior is inconsistent
+## here for simplicity of implementation. Value of dot is undefined.
----------------
This comment is wrong. Our behavior is not exactly the same as GNU linkers, but it is consistent to our way.


https://reviews.llvm.org/D38846