[PATCH] D30500: [ELF] - Do not treat colon(":") as separate token in script parser.

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added inline comments.


================
Comment at: ELF/LinkerScript.cpp:1945
+    // "global:" is default, so if there's no label, we assume global symbols.
+    consumeLabel("global:");
+    for (SymbolVersion V : readSymbols(false))
----------------
`:` is not a part of a label. It's a punctuation after a label. So please remove it.


================
Comment at: ELF/LinkerScript.cpp:2004-2009
+    if (consumeLabel("local:")) {
+      if (Locals)
+        setError("nested local label");
+      readLocals();
+      return Ret;
+    }
----------------
Do you need this error check?


================
Comment at: ELF/ScriptLexer.cpp:252
+bool ScriptLexer::consumeLabel(StringRef Tok) {
+  if (peek() == Tok ||
+    (peek() == Tok.drop_back(1) && peek(1) == Tok.take_back(1))) {
----------------
This seems complicated. You want to skip either `Tok` or `Tok:`. In code, it's something like this.

  if (consume(Tok))
    expect(":");
  else
    expect(Tok + ":");


================
Comment at: ELF/ScriptLexer.cpp:260-263
+void ScriptLexer::skip(unsigned N) {
+  while (N--)
+    (void)next();
+}
----------------
Please do not introduce a new utility function. If you want to skip two tokens, call next() twice.


https://reviews.llvm.org/D30500