[PATCH] [lld] [LinkerScript] Implement linker script expression evaluation
Shankar Kalpathi Easwaran
shankarke at gmail.com
Sun Mar 8 19:57:25 PDT 2015
================
Comment at: include/lld/ReaderWriter/ELFLinkingContext.h:304-306
@@ -303,1 +303,5 @@
+ const std::vector<std::unique_ptr<script::Parser>> &scripts() const {
+ return _scripts;
+ }
+
----------------
do you want to add a boolean hasLinkerScript that checks the size and returns ?
================
Comment at: include/lld/ReaderWriter/LinkerScript.h:366
@@ +365,3 @@
+ // here is an overkill.
+ typedef llvm::StringMap<int64_t, llvm::BumpPtrAllocator> SymbolTableTy;
+
----------------
you could replace this with a llvm::DenseMap<StringRef, int64_t> and use a common StringRefMappingInfo.
================
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(); }
----------------
Use lld::range,once you replace it with range, you can remove the typedef for const_iterator.
================
Comment at: lib/ReaderWriter/LinkerScript.cpp:581
@@ +580,3 @@
+ switch (_op) {
+ case Unary::Minus:
+ return 0 - childRes;
----------------
Do you want to handle Unary::Plus ??
================
Comment at: lib/ReaderWriter/LinkerScript.cpp:584
@@ +583,3 @@
+ case Unary::Not:
+ return ~childRes;
+ }
----------------
I thought linker script uses only a boolean value of a Unary Not, no ??
================
Comment at: lib/ReaderWriter/LinkerScript.cpp:703
@@ +702,3 @@
+ return trueExprRes;
+ else
+ return falseExprRes;
----------------
no need of else.
================
Comment at: unittests/DriverTests/GnuLdDriverTest.cpp:262
@@ +261,3 @@
+ EXPECT_EQ(script::SymbolAssignment::Simple, sa1->assignmentKind());
+ EXPECT_EQ(script::SymbolAssignment::Normal, sa1->assignmentVisibility());
+
----------------
Can we change SymbolAssignment::Normal to SymbolAssignment::Default ?
Looks like you are using the same SymboAssignment to name two different things, assignmentKind and assignmentVisibilty. I would recommend separating them.
http://reviews.llvm.org/D8156
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
More information about the llvm-commits
mailing list