[PATCH] [lld] [LinkerScript] Implement linker script expression evaluation

Rui Ueyama ruiu at google.com
Mon Mar 9 11:13:27 PDT 2015


LGTM with this change.


================
Comment at: include/lld/ReaderWriter/LinkerScript.h:794-795
@@ -767,2 +793,4 @@
   void dump(raw_ostream &os) const override;
+  const_iterator begin() const { return _sectionsCommands.begin(); }
+  const_iterator end() const { return _sectionsCommands.end(); }
 
----------------
rafaelauler wrote:
> shankarke wrote:
> > Use lld::range,once you replace it with range, you can remove the typedef for const_iterator.
> Thanks for the tip. Now that I'm thinking about this, there is also little reason to also use the typedef here, since we typically iterate through these elements with "auto", so we don't need to type the full name anyway. I used this mostly to keep code consistent with other classes and to make function signature cleaner. But if we're changing this, I'd rather do it in the follow-up patch, which introduces a whole lot more of iterators for accessing linker script AST nodes.
> 
Typedef is fine and preferred. I found lld::range is used in some places in LLD to "hide" actual types of return values. It on first sight seemed like a good idea, but it actually introduced a dependency to lld::range instead of to an actual type, adding an additional layer which doesn't do anything specific. Use of lld::range cannot be replaced with llvm::iterator_range in some cases because there is code assuming lld::range is a random access iterator (llvm::iterator_range isn't). I don't disagree that an abstraction like lld::range would be meaningful in some cases, but many existing use cases in LLD doesn't make much sense.

================
Comment at: lib/ReaderWriter/LinkerScript.cpp:699
@@ +698,3 @@
+    return falseExpr.getError();
+  return *falseExpr;
+}
----------------
You can just return a return value of evalExpr.

  if (conditional) {
    return _trueExpr->evalExpr(symbolTable);
  return _falseExpr->evalExpr(symbolTable);

http://reviews.llvm.org/D8156

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list