[PATCH] D25567: [MC] Fix Various End Of Line Comment checkings

[Reid Kleckner via llvm-commits]

rnk added inline comments.


================
Comment at: lib/MC/MCParser/AsmParser.cpp:2973
   int64_t FillExpr = 0;
-  if (getLexer().isNot(AsmToken::EndOfStatement)) {
-    if (parseToken(AsmToken::Comma, "unexpected token in '.org' directive") ||
-        parseAbsoluteExpression(FillExpr))
-      return true;
-  }
-
-  if (parseToken(AsmToken::EndOfStatement,
-                 "unexpected token in '.org' directive"))
-    return true;
+  bool hasComma;
+  parseOptionalToken(AsmToken::Comma, hasComma);
----------------
HasComma


================
Comment at: lib/MC/MCParser/MCAsmParser.cpp:71
 
 bool MCAsmParser::parseOptionalToken(AsmToken::TokenKind T, bool &Present) {
   Present = (getTok().getKind() == T);
----------------
I see a lot of usage of this that looks like:
  bool HasComma;
  parseOptionalToken(Comma, HasComma);
  if (HasComma && ...)

Right now, this always returns false. Either the token is there and its not an error, or we consume it and its not an error. In that case, do you think we should indicate presence with the return type? It might be confusing because a boolean parse method return type usually indicates an error. WDYT? Usage would look like:

  if (parseOptionalToken(AsmToken::Comma)) {
    if (parseIdentifier(...) || parseThing(...) || ...))
      return true / error / etc;
  }


================
Comment at: lib/MC/MCParser/MCAsmParser.cpp:77
   if (Present)
-    Lex();
+    parseToken(T);
   return false;
----------------
This feels like it's below parseToken. Any reason to not keep using Lex?


https://reviews.llvm.org/D25567