[PATCH] [lld] Add basic linker script parsing.

Michael Spencer bigcheesegs at gmail.com
Wed Feb 27 19:54:07 PST 2013



================
Comment at: include/lld/ReaderWriter/LinkerScript.h:29
@@ +28,3 @@
+namespace lld {
+class Token {
+public:
----------------
kledzik at apple.com wrote:
> These are a bunch of generic class names (e.g. Token, Parser, etc) in the lld namespace.  Can you nest these classes inside LinkerScript or put them in an anonymous namespace inside the .cpp file?
Yeah, I can stick them in a namespace.

================
Comment at: include/lld/ReaderWriter/LinkerScript.h:77
@@ +76,3 @@
+    CK_AsNeeded
+  };
+
----------------
kledzik at apple.com wrote:
> Can this be a class enum, so you don't need the CK_ prefix?
Yep.

================
Comment at: lib/ReaderWriter/ELF/ELFTargetInfo.cpp:79
@@ +78,3 @@
+    return *_linkerScriptReader;
+  }
+
----------------
kledzik at apple.com wrote:
> It would be nice if this could be refactored to not need the std::bind and std:: placeholders.  This flexibility seems like over engineering that is hurting readability.
Yes, this can actually just use TargetInfo::getReader now.


http://llvm-reviews.chandlerc.com/D477



More information about the llvm-commits mailing list