[PATCH] D28911: [LinkerScript] WIP: Implement `MEMORY` command

[Meador Inge via Phabricator via llvm-commits]

meadori added inline comments.


================
Comment at: ELF/LinkerScript.cpp:743
+      } else {
+        warn("memory region `" + Cmd->MemoryRegionName + "' not declared");
+      }
----------------
grimar wrote:
> Should it be error instead ?
I think so.  Thanks.


================
Comment at: ELF/LinkerScript.cpp:2000
+    StringRef Tok = next();
+    if (!readInteger(Tok, Origin))
+      setError("nonconstant expression for origin");
----------------
grimar wrote:
> I think you do not need Tok:
> 
> ```
> if (!readInteger(next(), Origin))
> ```
You're right.  Will fix.


================
Comment at: ELF/LinkerScript.cpp:2012
+    // TODO: Fully support constant expressions.
+    if (!readInteger(Tok, Length))
+      setError("nonconstant expression for length");
----------------
grimar wrote:
> Same here.
Will fix.


https://reviews.llvm.org/D28911