[PATCH] [lld] Make ELFLinkingContext own LinkerScript buffers

Rui Ueyama ruiu at google.com
Mon Feb 2 11:16:07 PST 2015


================
Comment at: include/lld/ReaderWriter/ELFLinkingContext.h:304
@@ -299,1 +303,3 @@
+    _scripts.push_back(std::move(script));
+  }
 private:
----------------
Add a blank line after this line.

================
Comment at: include/lld/ReaderWriter/LinkerScript.h:748
@@ +747,3 @@
+  /// to update all the AST pointers to a new buffer.
+  Parser(const Parser &instance) LLVM_DELETED_FUNCTION;
+
----------------
Maybe we can write "= delete" instead of LLVM_DELETED_FUNCTION? I believe the macro is for pre-C++11 compilers.

================
Comment at: include/lld/ReaderWriter/LinkerScript.h:765
@@ -743,1 +764,3 @@
+    return res;
+  }
 
----------------
This function allows the user to lazy-evaluate linker scripts, but I think lazy evaluation is not a good idea here. I'd like get errors as soon as possible if there's an error in a linker script. How about this.

Change the signature of parse to std::error_code parse(). We call this function after instantiating the class. If it returns an error, it means there's an error in the linker script. If it doesn't return an error, subsequent getTopLevelNode will always succeed.

Also, getTopLevelNode seems a bit too long. Maybe get is enough?

http://reviews.llvm.org/D7323

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






More information about the llvm-commits mailing list