[PATCH] D64130: [LLD][ELF] - Linkerscript: add a support for expressions for section's filling

[George Rimar via Phabricator via llvm-commits]

grimar marked an inline comment as done.
grimar added inline comments.


================
Comment at: ELF/ScriptParser.cpp:884
 // We are compatible with ld.gold because it's easier to implement.
-std::array<uint8_t, 4> ScriptParser::parseFill(StringRef Tok) {
-  uint32_t V = 0;
-  if (!to_integer(Tok, V))
-    setError("invalid filler expression: " + Tok);
+std::array<uint8_t, 4> ScriptParser::readFill() {
+  uint64_t Value = readExpr()().Val;
----------------
grimar wrote:
> peter.smith wrote:
> > It is worth a comment or FIXME: saying we don't support symbols in fill expressions?
> Done. I also added a test case to demonstrate the current behavior. It shows "symbol not found: foo" currently,
> because tries to find a symbol during evaluating the expression. And at this moment no symbols are added
> to a sym table.
> 
> In theory we can change the message to be more meaningful in case we read an expression
> that should not use any symbols, but since it would be used only for `fillexpr`, I am not sure it worth doing.
I.e. I mean we could add a flag like `DontAllowSymbols` and return a different error message when reading an expression which has a reference to a symbol. But, as I said, It looks a bit too much for this feature.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64130/new/

https://reviews.llvm.org/D64130