Ignore Lexing errors in macro body definitions

[niravd]

Summary

Do not issue lexing errors found during the parsing of macro body
definitions and parseIdentifier function in AsmParser. This changes the
Parser to not issue a lexing error when we reach an error, but rather
when it is consumed allowing us time to examine and recover from an
error.

As a result, of this, we stop issuing a both lexing error and a parsing
error in floating-literals test. Minor tweak to parseDirectiveRealValue
to favor more meaningful lexing error over less helpful parse error.

[niravd]

Add missing test case for macro parsing

[rnk]

lgtm I spent a fair amount of time thinking about how to do this better, but let's go with the solution we have now.

[niravd]

Merge in follow up fix minor error cleanup to make work with tip

[niravd]

Merged in subsequent error cleanup patch as accepted patch breaks in tip. Can I get another LGTM?

In D20535#444732, @rnk wrote:

lgtm I spent a fair amount of time thinking about how to do this better, but let's go with the solution we have now.

[rnk]

Still looks good. In general, you don't need explicit approval for things like fixing the build, merging patches, fixing broken tests, etc.

llvm/trunk/lib/MC/MCParser/AsmLexer.cpp

void AsmLexer::setBuffer(StringRef Buf, const char *ptr) {
   TokStart = nullptr;
 }
 
 /// ReturnError - Set the error to the specified string at the specified
 /// location.  This is defined to always return AsmToken::Error.
 AsmToken AsmLexer::ReturnError(const char *Loc, const std::string &Msg) {
   SetError(SMLoc::getFromPointer(Loc), Msg);
 
-  return AsmToken(AsmToken::Error, StringRef(Loc, 0));
+  return AsmToken(AsmToken::Error, StringRef(Loc, CurPtr - Loc));
 }
 
 int AsmLexer::getNextChar() {
   char CurChar = *CurPtr++;
   switch (CurChar) {
   default:
     return (unsigned char)CurChar;
   case 0:

llvm/trunk/lib/MC/MCParser/AsmParser.cpp

 
 void AsmParser::jumpToLoc(SMLoc Loc, unsigned InBuffer) {
   CurBuffer = InBuffer ? InBuffer : SrcMgr.FindBufferContainingLoc(Loc);
   Lexer.setBuffer(SrcMgr.getMemoryBuffer(CurBuffer)->getBuffer(),
                   Loc.getPointer());
 }
 
 const AsmToken &AsmParser::Lex() {
+  if (Lexer.getTok().is(AsmToken::Error))
+    Error(Lexer.getErrLoc(), Lexer.getErr());
+
   const AsmToken *tok = &Lexer.Lex();
 
   if (tok->is(AsmToken::Eof)) {
     // If this is the end of an included file, pop the parent file off the
     // include stack.
     SMLoc ParentIncludeLoc = SrcMgr.getParentIncludeLoc(CurBuffer);
     if (ParentIncludeLoc != SMLoc()) {
       jumpToLoc(ParentIncludeLoc);
       tok = &Lexer.Lex();
     }
   }
 
-  if (tok->is(AsmToken::Error))
-    Error(Lexer.getErrLoc(), Lexer.getErr());
 
   return *tok;
 }
 
 bool AsmParser::Run(bool NoInitialTextSection, bool NoFinalize) {
   // Create the initial section, if requested.
   if (!NoInitialTextSection)
     Out.InitSections(false);
bool AsmParser::Run(bool NoInitialTextSection, bool NoFinalize) {
   }
 
   // While we have input, parse each statement.
   while (Lexer.isNot(AsmToken::Eof)) {
     ParseStatementInfo Info;
     if (!parseStatement(Info, nullptr))
       continue;
 
+    // If we've failed, but on a Error Token, but did not consume it in
+    // favor of a better message, emit it now.
+    if (Lexer.getTok().is(AsmToken::Error)) {
+      Lex();
+    }
+
     // We had an error, validate that one was emitted and recover by skipping to
     // the next line.
     assert(HadError && "Parse statement returned an error, but none emitted!");
     eatToEndOfStatement();
   }
 
   if (TheCondState.TheCond != StartingCondState.TheCond ||
       TheCondState.Ignore != StartingCondState.Ignore)
if (!ParsingInlineAsm && !getStreamer().getCurrentSection().first) {
     TokError("expected section directive before assembly directive");
     Out.InitSections(false);
   }
 }
 
 /// \brief Throw away the rest of the line for testing purposes.
 void AsmParser::eatToEndOfStatement() {
   while (Lexer.isNot(AsmToken::EndOfStatement) && Lexer.isNot(AsmToken::Eof))
-    Lex();
+    Lexer.Lex();
 
   // Eat EOL.
   if (Lexer.is(AsmToken::EndOfStatement))
-    Lex();
+    Lexer.Lex();
 }
 
 StringRef AsmParser::parseStringToEndOfStatement() {
   const char *Start = getTok().getLoc().getPointer();
 
   while (Lexer.isNot(AsmToken::EndOfStatement) && Lexer.isNot(AsmToken::Eof))
     Lex();
 
for (;;) {
 
     if (ParenLevel == 0) {
 
       if (Lexer.is(AsmToken::Comma))
         break;
 
       if (Lexer.is(AsmToken::Space)) {
         SpaceEaten = true;
-        Lex(); // Eat spaces
+        Lexer.Lex(); // Eat spaces
       }
 
       // Spaces can delimit parameters, but could also be part an expression.
       // If the token after a space is an operator, add the token and the next
       // one into this argument
       if (!IsDarwin) {
         if (isOperator(Lexer.getKind())) {
           MA.push_back(getTok());
-          Lex();
+          Lexer.Lex();
 
           // Whitespace after an operator can be ignored.
           if (Lexer.is(AsmToken::Space))
-            Lex();
+            Lexer.Lex();
 
           continue;
         }
       }
       if (SpaceEaten)
         break;
     }
 
     // handleMacroEntry relies on not advancing the lexer here
     // to be able to fill in the remaining default parameter values
     if (Lexer.is(AsmToken::EndOfStatement))
       break;
 
     // Adjust the current parentheses level.
     if (Lexer.is(AsmToken::LParen))
       ++ParenLevel;
     else if (Lexer.is(AsmToken::RParen) && ParenLevel)
       --ParenLevel;
 
     // Append the token to the current argument list.
     MA.push_back(getTok());
-    Lex();
+    Lexer.Lex();
   }
 
   if (ParenLevel != 0)
     return TokError("unbalanced parentheses in macro argument");
   return false;
 }
 
 // Parse the macro instantiation arguments.
bool AsmParser::parseIdentifier(StringRef &Res) {
   // allow things like '.globl $foo' and '.def @feat.00', which would normally be
   // separate tokens. At this level, we have already lexed so we cannot (currently)
   // handle this as a context dependent token, instead we detect adjacent tokens
   // and return the combined identifier.
   if (Lexer.is(AsmToken::Dollar) || Lexer.is(AsmToken::At)) {
     SMLoc PrefixLoc = getLexer().getLoc();
 
     // Consume the prefix character, and check for a following identifier.
-    Lex();
+    Lexer.Lex(); // Lexer's Lex guarantees consecutive token.
     if (Lexer.isNot(AsmToken::Identifier))
       return true;
 
     // We have a '$' or '@' followed by an identifier, make sure they are adjacent.
     if (PrefixLoc.getPointer() + 1 != getTok().getLoc().getPointer())
       return true;
 
     // Construct the joined identifier and consume the token.
     Res =
         StringRef(PrefixLoc.getPointer(), getTok().getIdentifier().size() + 1);
-    Lex();
+    Lexer.Lex(); // Lexer's Lex guarantees consecutive token
     return false;
   }
 
   if (Lexer.isNot(AsmToken::Identifier) && Lexer.isNot(AsmToken::String))
     return true;
 
   Res = getTok().getIdentifier();
 
bool AsmParser::parseDirectiveRealValue(const fltSemantics &Semantics) {
   if (getLexer().isNot(AsmToken::EndOfStatement)) {
     checkForValidSection();
 
     for (;;) {
       // We don't truly support arithmetic on floating point expressions, so we
       // have to manually parse unary prefixes.
       bool IsNeg = false;
       if (getLexer().is(AsmToken::Minus)) {
-        Lex();
+        Lexer.Lex();
         IsNeg = true;
       } else if (getLexer().is(AsmToken::Plus))
-        Lex();
+        Lexer.Lex();
 
-      if (getLexer().isNot(AsmToken::Integer) &&
-          getLexer().isNot(AsmToken::Real) &&
-          getLexer().isNot(AsmToken::Identifier))
+      if (Lexer.is(AsmToken::Error))
+        return TokError(Lexer.getErr());
+      if (Lexer.isNot(AsmToken::Integer) && Lexer.isNot(AsmToken::Real) &&
+          Lexer.isNot(AsmToken::Identifier))
         return TokError("unexpected token in directive");
 
       // Convert to an APFloat.
       APFloat Value(Semantics);
       StringRef IDVal = getTok().getString();
       if (getLexer().is(AsmToken::Identifier)) {
         if (!IDVal.compare_lower("infinity") || !IDVal.compare_lower("inf"))
           Value = APFloat::getInf(Semantics);
for (;;) {
       // Consume the numeric token.
       Lex();
 
       // Emit the value as an integer.
       APInt AsInt = Value.bitcastToAPInt();
       getStreamer().EmitIntValue(AsInt.getLimitedValue(),
                                  AsInt.getBitWidth() / 8);
 
-      if (getLexer().is(AsmToken::EndOfStatement))
+      if (Lexer.is(AsmToken::EndOfStatement))
         break;
 
-      if (getLexer().isNot(AsmToken::Comma))
+      if (Lexer.isNot(AsmToken::Comma))
         return TokError("unexpected token in directive");
       Lex();
     }
   }
 
   Lex();
   return false;
 }
while (getLexer().isNot(AsmToken::EndOfStatement)) {
     }
 
     Parameters.push_back(std::move(Parameter));
 
     if (getLexer().is(AsmToken::Comma))
       Lex();
   }
 
-  // Eat the end of statement.
-  Lex();
+  // Eat just the end of statement.
+  Lexer.Lex();
 
+  // Consuming deferred text, so use Lexer.Lex to ignore Lexing Errors
   AsmToken EndToken, StartToken = getTok();
   unsigned MacroDepth = 0;
-
   // Lex the macro definition.
   for (;;) {
+    // Ignore Lexing errors in macros.
+    while (Lexer.is(AsmToken::Error)) {
+      Lexer.Lex();
+    }
+
     // Check whether we have reached the end of the file.
     if (getLexer().is(AsmToken::Eof))
       return Error(DirectiveLoc, "no matching '.endmacro' in definition");
 
     // Otherwise, check whether we have reach the .endmacro.
     if (getLexer().is(AsmToken::Identifier)) {
       if (getTok().getIdentifier() == ".endm" ||
           getTok().getIdentifier() == ".endmacro") {
         if (MacroDepth == 0) { // Outermost macro.
           EndToken = getTok();
-          Lex();
+          Lexer.Lex();
           if (getLexer().isNot(AsmToken::EndOfStatement))
             return TokError("unexpected token in '" + EndToken.getIdentifier() +
                             "' directive");
           break;
         } else {
           // Otherwise we just found the end of an inner macro.
           --MacroDepth;
         }

llvm/trunk/lib/MC/MCParser/MCAsmLexer.cpp

 //===-- MCAsmLexer.cpp - Abstract Asm Lexer Interface ---------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/MC/MCParser/MCAsmLexer.h"
 #include "llvm/Support/SourceMgr.h"
 
 using namespace llvm;
 
 MCAsmLexer::MCAsmLexer() : TokStart(nullptr), SkipSpace(true) {
-  CurTok.emplace_back(AsmToken::Error, StringRef());
+  CurTok.emplace_back(AsmToken::Space, StringRef());
 }
 
 MCAsmLexer::~MCAsmLexer() {
 }
 
 SMLoc MCAsmLexer::getLoc() const {
   return SMLoc::getFromPointer(TokStart);
 }

llvm/trunk/test/MC/AsmParser/floating-literals.s

 # CHECK: .long 1094713344
 .float 0x3p2
 # CHECK: .long 872284160
 .float 0x7fp-30
 # CHECK: .long 3212836864
 .float -0x1.0p0
 
 # CHECK-ERROR: invalid hexadecimal floating-point constant: expected at least one exponent digit
-# CHECK-ERROR: unexpected token in directive
 .float 0xa.apa
 
 # CHECK-ERROR: invalid hexadecimal floating-point constant: expected at least one exponent digit
-# CHECK-ERROR: unexpected token in directive
 .double -0x1.2p+
 
 # CHECK-ERROR: invalid hexadecimal floating-point constant: expected at least one exponent digit
-# CHECK-ERROR: unexpected token in directive
 .double -0x1.2p
 
 # CHECK-ERROR: invalid hexadecimal floating-point constant: expected at least one significand digit
-# CHECK-ERROR: unexpected token in directive
 .float 0xp2
 
 # CHECK-ERROR: invalid hexadecimal floating-point constant: expected at least one significand digit
-# CHECK-ERROR: unexpected token in directive
 .float 0x.p5
 
 # CHECK-ERROR: error: invalid hexadecimal floating-point constant: expected exponent part 'p'
-# CHECK-ERROR: unexpected token in directive
 .float 0x1.2

llvm/trunk/test/MC/AsmParser/macro_parsing.s

+# RUN: llvm-mc -triple i386-unknown-unknown %s | FileCheck %s
+
+        .macro DEF num
+        int $0x\num
+        .endm
+        DEF 02
+        DEF 08
+        DEF 09
+        DEF 0A
+        DEF 10
+
+# CHECK: int $2
+# CHECK: int $8
+# CHECK: int $9
+# CHECK: int $10
+# CHECK: int $16