[PATCH] D22625: [ELF] Support PROVIDE and PROVIDE_HIDDEN within SECTIONS {} block

[Rui Ueyama via llvm-commits]

ruiu added inline comments.

================
Comment at: ELF/LinkerScript.cpp:281
@@ -280,4 +280,3 @@
       uint64_t Val = evalExpr(Cmd->Expr, Dot);
-      if (Cmd->Name == ".") {
-
+      if (Cmd->Name == ".")
         Dot = Val;
----------------
Why did you remove {}?

================
Comment at: ELF/LinkerScript.cpp:459
@@ +458,3 @@
+        else if (Cmd->Provision != SymbolProvision::Create)
+          Cmd->Provision = SymbolProvision::Ignore;
+      }
----------------
This doesn't look beautiful. Isn't there any better way?

================
Comment at: ELF/LinkerScript.cpp:720
@@ +719,3 @@
+    if (Tok == "PROVIDE")
+      readProvideBody(SymbolProvision::Provide);
+    else if (Tok == "PROVIDE_HIDDEN")
----------------
Let's name this `readProvide` as `Body` is used often to mean a symbol.

================
Comment at: ELF/LinkerScript.cpp:724
@@ -714,1 +723,3 @@
+    else if (peek() == "=")
+      readSymbolAssignment(Tok, SymbolProvision::Create);
     else
----------------
`Create` doesn't feel like a good name. How about `Assign`?

================
Comment at: ELF/LinkerScript.cpp:799-807
@@ -778,5 +798,11 @@
 std::vector<StringRef> ScriptParser::readSectionsCommandExpr() {
+  int Braces = 0;
   std::vector<StringRef> Expr;
   while (!Error) {
-    StringRef Tok = next();
+    StringRef Tok = peek();
+    Braces += (Tok == "(");
+    Braces -= (Tok == ")");
+    if (Braces < 0)
+      break;
+    next();
     if (Tok == ";")
----------------
Is this change related to the feature you are adding in this patch?

================
Comment at: ELF/LinkerScript.h:60
@@ -59,1 +59,3 @@
 
+enum class SymbolProvision { Create, Provide, ProvideHidden, Ignore };
+
----------------
Please define this enum in SymbolAssignment class (as a non-enum class).

================
Comment at: ELF/LinkerScript.h:64
@@ +63,3 @@
+  SymbolAssignment(StringRef Name, std::vector<StringRef> &Expr,
+                   SymbolProvision Provision = SymbolProvision::Create)
+      : BaseCommand(AssignmentKind), Name(Name), Expr(std::move(Expr)),
----------------
I wouldn't use a default argument unless it really improves to readability.


Repository:
  rL LLVM

https://reviews.llvm.org/D22625