[PATCH] D42911: [ELF] - Make defsym to work correctly with reserved symbols.

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: ELF/LinkerScript.cpp:74-78
   // If the alignment is trivial, we don't have to compute the full
   // value to know the offset. This allows this function to succeed in
   // cases where the output section is not yet known.
-  if (Alignment == 1)
+  // In case of output section we always want to compute the full value
+  // because use special offset value (-1) as end of section mark.
----------------
ruiu wrote:
> In general, when you change existing code, you should update the comment instead of adding sentences to existing comment so that the comment doesn't look patchy. Comments shouldn't reflect the history of code change but just describe why we are doing this.
> 
> So, this is just that if an expression is absolute and has no alignment requirement, its value is simply the output value, no?
Technically yes, but we are unable to use something like following here I think.
(I guess you mean that)

```
if (isAbsolute() && Alignment == 1)
  return Val;
```

Because, for example we have in early-assign-symbol.s the following script:
```
SECTIONS { aaa = ABSOLUTE(foo - 1) + 1; .text  : { *(.text*) } }
```

And we first evaluate expression and then convert it to absolute:
```
  if (Tok == "ABSOLUTE") {
    Expr Inner = readParenExpr();
    return [=] {
      ExprValue I = Inner();
      I.ForceAbsolute = true;
      return I;
    };
  }
```
Not sure we should change something here.

I rewrote the check though.


https://reviews.llvm.org/D42911