[ELF] - Remove ScriptLexer::Error field and check ErrorCount instead.

[grimar]

Summary

D35945 introduces change when there is useless to check Error flag
in few places, but ErrorCount must be checked instead.

But then we probably can just check ErrorCount always. That should simplify
things. Patch do that.

[ruiu]

LGTM

ELF/ScriptLexer.h

public:
   void expect(StringRef Expect);
   bool consumeLabel(StringRef Tok);
   std::string getCurrentLocation();
 
   std::vector<MemoryBufferRef> MBs;
   std::vector<StringRef> Tokens;
   bool InExpr = false;
   size_t Pos = 0;
-  bool Error = false;
 
 private:
   void maybeSplitExpr();
   StringRef getLine();
   size_t getLineNumber();
   size_t getColumnNumber();
 
   MemoryBufferRef getCurrentMB();
 };
 
 } // namespace elf
 } // namespace lld
 
 #endif

ELF/ScriptLexer.cpp

if (!Pos)
     return Filename;
   return (Filename + ":" + Twine(getLineNumber())).str();
 }
 
 ScriptLexer::ScriptLexer(MemoryBufferRef MB) { tokenize(MB); }
 
 // We don't want to record cascading errors. Keep only the first one.
 void ScriptLexer::setError(const Twine &Msg) {
-  if (Error)
+  if (ErrorCount)
     return;
-  Error = true;
 
   if (!Pos) {
     error(getCurrentLocation() + ": " + Msg);
     return;
   }
 
   std::string S = getCurrentLocation() + ": ";
   error(S + Msg);
for (;;) {
     size_t Size = S.size();
     S = S.ltrim();
     if (S.size() == Size)
       return S;
   }
 }
 
 // An erroneous token is handled as if it were the last token before EOF.
-bool ScriptLexer::atEOF() { return Error || Tokens.size() == Pos; }
+bool ScriptLexer::atEOF() { return ErrorCount || Tokens.size() == Pos; }
 
 // Split a given string as an expression.
 // This function returns "3", "*" and "5" for "3*5" for example.
 static std::vector<StringRef> tokenizeExpr(StringRef S) {
   StringRef Ops = "+-*/:"; // List of operators
 
   // Quoted strings are literal strings, so we don't want to split it.
   if (S.startswith("\""))

 // arithmetic operator characters are regular characters, but in the
 // expression context, they should be independent tokens.
 //
 // For example, "foo*3" should be tokenized to "foo", "*" and "3" only
 // in the expression context.
 //
 // This function may split the current token into multiple tokens.
 void ScriptLexer::maybeSplitExpr() {
-  if (!InExpr || Error || atEOF())
+  if (!InExpr || ErrorCount || atEOF())
     return;
 
   std::vector<StringRef> V = tokenizeExpr(Tokens[Pos]);
   if (V.size() == 1)
     return;
   Tokens.erase(Tokens.begin() + Pos);
   Tokens.insert(Tokens.begin() + Pos, V.begin(), V.end());
 }
 
 StringRef ScriptLexer::next() {
   maybeSplitExpr();
 
-  if (Error)
+  if (ErrorCount)
     return "";
   if (atEOF()) {
     setError("unexpected EOF");
     return "";
   }
   return Tokens[Pos++];
 }
 
 StringRef ScriptLexer::peek() {
   StringRef Tok = next();
-  if (Error)
+  if (ErrorCount)
     return "";
   Pos = Pos - 1;
   return Tok;
 }
 
 bool ScriptLexer::consume(StringRef Tok) {
   if (peek() == Tok) {
     skip();
if (Tokens.size() >= Pos + 2 && Tokens[Pos] == Tok &&
     return true;
   }
   return false;
 }
 
 void ScriptLexer::skip() { (void)next(); }
 
 void ScriptLexer::expect(StringRef Expect) {
-  if (Error)
+  if (ErrorCount)
     return;
   StringRef Tok = next();
   if (Tok != Expect)
     setError(Expect + " expected, but got " + Tok);
 }
 
 // Returns true if S encloses T.
 static bool encloses(StringRef S, StringRef T) {

ELF/ScriptParser.cpp

 }
 
 void ScriptParser::readVersionScriptCommand() {
   if (consume("{")) {
     readAnonymousDeclaration();
     return;
   }
 
-  while (!atEOF() && !Error && peek() != "}") {
+  while (!atEOF() && !ErrorCount && peek() != "}") {
     StringRef VerStr = next();
     if (VerStr == "{") {
       setError("anonymous version definition is used in "
                "combination with other version definitions");
       return;
     }
     expect("{");
     readVersionDeclaration(VerStr);
else
       setError("unable to find " + S);
   }
 }
 
 void ScriptParser::readAsNeeded() {
   expect("(");
   bool Orig = Config->AsNeeded;
   Config->AsNeeded = true;
-  while (!Error && !consume(")"))
+  while (!ErrorCount && !consume(")"))
     addFile(unquote(next()));
   Config->AsNeeded = Orig;
 }
 
 void ScriptParser::readEntry() {
   // -e <symbol> takes predecence over ENTRY(<symbol>).
   expect("(");
   StringRef Tok = next();
   if (Config->Entry.empty())
     Config->Entry = Tok;
   expect(")");
 }
 
 void ScriptParser::readExtern() {
   expect("(");
-  while (!Error && !consume(")"))
+  while (!ErrorCount && !consume(")"))
     Config->Undefined.push_back(next());
 }
 
 void ScriptParser::readGroup() {
   expect("(");
-  while (!Error && !consume(")")) {
+  while (!ErrorCount && !consume(")")) {
     if (consume("AS_NEEDED"))
       readAsNeeded();
     else
       addFile(unquote(next()));
   }
 }
 
 void ScriptParser::readInclude() {
void ScriptParser::readOutput() {
   if (Config->OutputFile.empty())
     Config->OutputFile = unquote(Tok);
   expect(")");
 }
 
 void ScriptParser::readOutputArch() {
   // OUTPUT_ARCH is ignored for now.
   expect("(");
-  while (!Error && !consume(")"))
+  while (!ErrorCount && !consume(")"))
     skip();
 }
 
 void ScriptParser::readOutputFormat() {
   // Error checking only for now.
   expect("(");
   skip();
   if (consume(")"))
     return;
   expect(",");
   skip();
   expect(",");
   skip();
   expect(")");
 }
 
 void ScriptParser::readPhdrs() {
   expect("{");
-  while (!Error && !consume("}")) {
+  while (!ErrorCount && !consume("}")) {
     Script->Opt.PhdrsCommands.push_back(
         {next(), PT_NULL, false, false, UINT_MAX, nullptr});
 
     PhdrsCommand &PhdrCmd = Script->Opt.PhdrsCommands.back();
     PhdrCmd.Type = readPhdrType();
 
-    while (!Error && !consume(";")) {
+    while (!ErrorCount && !consume(";")) {
       if (consume("FILEHDR"))
         PhdrCmd.HasFilehdr = true;
       else if (consume("PHDRS"))
         PhdrCmd.HasPhdrs = true;
       else if (consume("AT"))
         PhdrCmd.LMAExpr = readParenExpr();
       else if (consume("FLAGS"))
         PhdrCmd.Flags = readParenExpr()().getValue();
void ScriptParser::readSections() {
   Script->Opt.HasSections = true;
 
   // -no-rosegment is used to avoid placing read only non-executable sections in
   // their own segment. We do the same if SECTIONS command is present in linker
   // script. See comment for computeFlags().
   Config->SingleRoRx = true;
 
   expect("{");
-  while (!Error && !consume("}")) {
+  while (!ErrorCount && !consume("}")) {
     StringRef Tok = next();
     BaseCommand *Cmd = readProvideOrAssignment(Tok);
     if (!Cmd) {
       if (Tok == "ASSERT")
         Cmd = readAssert();
       else
         Cmd = readOutputSectionDescription(Tok);
     }
     Script->Opt.Commands.push_back(Cmd);
   }
 }
 
 static int precedence(StringRef Op) {
   return StringSwitch<int>(Op)
       .Cases("*", "/", 5)
       .Cases("+", "-", 4)
       .Cases("<<", ">>", 3)
       .Cases("<", "<=", ">", ">=", "==", "!=", 2)
       .Cases("&", "|", 1)
       .Default(-1);
 }
 
 StringMatcher ScriptParser::readFilePatterns() {
   std::vector<StringRef> V;
-  while (!Error && !consume(")"))
+  while (!ErrorCount && !consume(")"))
     V.push_back(next());
   return StringMatcher(V);
 }
 
 SortSectionPolicy ScriptParser::readSortKind() {
   if (consume("SORT") || consume("SORT_BY_NAME"))
     return SortSectionPolicy::Name;
   if (consume("SORT_BY_ALIGNMENT"))

 //
 // *(.foo EXCLUDE_FILE (a.o) .bar EXCLUDE_FILE (b.o) .baz)
 //
 // is parsed as ".foo", ".bar" with "a.o", and ".baz" with "b.o".
 // The semantics of that is section .foo in any file, section .bar in
 // any file but a.o, and section .baz in any file but b.o.
 std::vector<SectionPattern> ScriptParser::readInputSectionsList() {
   std::vector<SectionPattern> Ret;
-  while (!Error && peek() != ")") {
+  while (!ErrorCount && peek() != ")") {
     StringMatcher ExcludeFilePat;
     if (consume("EXCLUDE_FILE")) {
       expect("(");
       ExcludeFilePat = readFilePatterns();
     }
 
     std::vector<StringRef> V;
-    while (!Error && peek() != ")" && peek() != "EXCLUDE_FILE")
+    while (!ErrorCount && peek() != ")" && peek() != "EXCLUDE_FILE")
       V.push_back(next());
 
     if (!V.empty())
       Ret.push_back({std::move(ExcludeFilePat), StringMatcher(V)});
     else
       setError("section pattern is expected");
   }
   return Ret;

 //              | "SORT_BY_INIT_PRIORITY" | "SORT_NONE"
 //
 // <section-list> is parsed by readInputSectionsList().
 InputSectionDescription *
 ScriptParser::readInputSectionRules(StringRef FilePattern) {
   auto *Cmd = make<InputSectionDescription>(FilePattern);
   expect("(");
 
-  while (!Error && !consume(")")) {
+  while (!ErrorCount && !consume(")")) {
     SortSectionPolicy Outer = readSortKind();
     SortSectionPolicy Inner = SortSectionPolicy::Default;
     std::vector<SectionPattern> V;
     if (Outer != SortSectionPolicy::Default) {
       expect("(");
       Inner = readSortKind();
       if (Inner != SortSectionPolicy::Default) {
         expect("(");
OutputSection *ScriptParser::readOutputSectionDescription(StringRef OutSec) {
 
   // Parse constraints.
   if (consume("ONLY_IF_RO"))
     Cmd->Constraint = ConstraintKind::ReadOnly;
   if (consume("ONLY_IF_RW"))
     Cmd->Constraint = ConstraintKind::ReadWrite;
   expect("{");
 
-  while (!Error && !consume("}")) {
+  while (!ErrorCount && !consume("}")) {
     StringRef Tok = next();
     if (Tok == ";") {
       // Empty commands are allowed. Do nothing here.
     } else if (SymbolAssignment *Assign = readProvideOrAssignment(Tok)) {
       Cmd->Commands.push_back(Assign);
     } else if (BytesDataCommand *Data = readBytesDataCommand(Tok)) {
       Cmd->Commands.push_back(Data);
     } else if (Tok == "ASSERT") {
static Expr combine(StringRef Op, Expr L, Expr R) {
   if (Op == "|")
     return [=] { return bitOr(L(), R()); };
   llvm_unreachable("invalid operator");
 }
 
 // This is a part of the operator-precedence parser. This function
 // assumes that the remaining token stream starts with an operator.
 Expr ScriptParser::readExpr1(Expr Lhs, int MinPrec) {
-  while (!atEOF() && !Error) {
+  while (!atEOF() && !ErrorCount) {
     // Read an operator and an expression.
     if (consume("?"))
       return readTernary(Lhs);
     StringRef Op1 = peek();
     if (precedence(Op1) < MinPrec)
       break;
     skip();
     Expr Rhs = readPrimary();
Expr ScriptParser::readParenExpr() {
   expect("(");
   Expr E = readExpr();
   expect(")");
   return E;
 }
 
 std::vector<StringRef> ScriptParser::readOutputSectionPhdrs() {
   std::vector<StringRef> Phdrs;
-  while (!Error && peek().startswith(":")) {
+  while (!ErrorCount && peek().startswith(":")) {
     StringRef Tok = next();
     Phdrs.push_back((Tok.size() == 1) ? next() : Tok.substr(1));
   }
   return Phdrs;
 }
 
 // Read a program header type name. The next token must be a
 // name of a program header type or a constant (e.g. "0x3").

 
 // Reads a list of symbols, e.g. "{ global: foo; bar; local: *; };".
 std::pair<std::vector<SymbolVersion>, std::vector<SymbolVersion>>
 ScriptParser::readSymbols() {
   std::vector<SymbolVersion> Locals;
   std::vector<SymbolVersion> Globals;
   std::vector<SymbolVersion> *V = &Globals;
 
-  while (!Error) {
+  while (!ErrorCount) {
     if (consume("}"))
       break;
     if (consumeLabel("local")) {
       V = &Locals;
       continue;
     }
     if (consumeLabel("global")) {
       V = &Globals;

 std::vector<SymbolVersion> ScriptParser::readVersionExtern() {
   StringRef Tok = next();
   bool IsCXX = Tok == "\"C++\"";
   if (!IsCXX && Tok != "\"C\"")
     setError("Unknown language");
   expect("{");
 
   std::vector<SymbolVersion> Ret;
-  while (!Error && peek() != "}") {
+  while (!ErrorCount && peek() != "}") {
     StringRef Tok = next();
     bool HasWildcard = !Tok.startswith("\"") && hasWildcard(Tok);
     Ret.push_back({unquote(Tok), IsCXX, HasWildcard});
     expect(";");
   }
 
   expect("}");
   return Ret;

 }
 
 // Parse the MEMORY command as specified in:
 // https://sourceware.org/binutils/docs/ld/MEMORY.html
 //
 // MEMORY { name [(attr)] : ORIGIN = origin, LENGTH = len ... }
 void ScriptParser::readMemory() {
   expect("{");
-  while (!Error && !consume("}")) {
+  while (!ErrorCount && !consume("}")) {
     StringRef Name = next();
 
     uint32_t Flags = 0;
     uint32_t NegFlags = 0;
     if (consume("(")) {
       std::tie(Flags, NegFlags) = readMemoryAttributes();
       expect(")");
     }

test/ELF/version-script-err.s

 // REQUIRES: x86
 
 // RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
 // RUN: not ld.lld -shared %t.o -o %t.so --version-script %p/Inputs/version-script-err.script 2>&1 | FileCheck %s
 // CHECK: ; expected, but got }
 
 // RUN: echo    "\"" > %terr1.script
 // RUN: not ld.lld --version-script %terr1.script -shared %t.o -o %t.so 2>&1 | \
 // RUN:   FileCheck -check-prefix=ERR1 %s
 // ERR1: {{.*}}:1: unclosed quote
-// ERR1-NEXT: {{.*}}: unexpected EOF