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

[Nirav Dave via llvm-commits]

niravd added inline comments.


================
Comment at: lib/MC/MCParser/MCAsmParser.cpp:71
 
 bool MCAsmParser::parseOptionalToken(AsmToken::TokenKind T, bool &Present) {
   Present = (getTok().getKind() == T);
----------------
rnk wrote:
> 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;
>   }
I had a similar thought myself but I was unhappy with the fact that the clean way to write this involves multiple conditional statements which duplicates the error suffix additions so I was pushing it off to another cleanup patch while I pondered a cleaner solution. In any case there's no real issue doing it here so I've folded it in. 



================
Comment at: lib/MC/MCParser/MCAsmParser.cpp:77
   if (Present)
-    Lex();
+    parseToken(T);
   return false;
----------------
rnk wrote:
> This feels like it's below parseToken. Any reason to not keep using Lex?
This is to correctly handle # preprocessor line comments as EndOfStatement for those cases that the lexer doesn't have the context.


https://reviews.llvm.org/D25567