[PATCH] D22916: [ELF] - Linkerscript: implemented += operator.

[George Rimar via llvm-commits]

grimar added inline comments.

================
Comment at: ELF/LinkerScript.cpp:922-923
@@ -918,3 +921,4 @@
   if (Tok.getAsInteger(0, V)) {
-    if (!isValidCIdentifier(Tok))
+    if (Tok != "." && !isValidCIdentifier(Tok))
       setError("malformed number: " + Tok);
+    return [=](uint64_t Dot) { return getSymbolValue(Tok, Dot); };
----------------
grimar wrote:
> ruiu wrote:
> > I think you can remove this error check. If Tok is not a valid symbol name, getSymbolValue will find it.
> Looks few tests begins to fail if I remove. Since this line is not added by this patch, I suggest to leave it for now, I'll check how to remove it and prepare a patch tomorrow.
After little investigation I am not sure will removing of check be good change or not now. Imagine we have the next script:

```
SECTIONS {
boom .temp : { *(.temp) } }
}
```

And imagine we have no .temp sections. While check is here, parser will catch the ".temp" and report mailformed number. Without that check since we never will create boom as there is no .temp sections, error will never shown.
But at the same time:

```
SECTIONS {
boom temp : { *(.temp) } }
}
```

Here temp is valid C identifier. So we will never see any error in that case. So that check helps only sometimes here.
But there are cases when this check really helps. Example:

```
SECTIONS {   . = ;  }
```
It finds that ";" is not a number and not a symbol and error outs. Without check it writes:
"; expected but got }" because of:

```
    if (peek() == "=" || peek() == "+=") {
      readAssignment(Tok);
      expect(";");
```

Probably we should check somehow that expression that readExpr() returns is not empty.


Repository:
  rL LLVM

https://reviews.llvm.org/D22916