MIR Lexer: adopt the 'maybeLex...' pattern.

[arphaman]

Summary

This patch refactors the MI lexer so that the lexing functions use the 'maybeLex...' pattern, where the lexing functions determine if they can lex the current token by themselves.

[silvas]

LGTM with a nit.

lib/CodeGen/MIRParser/MILexer.cpp

182

Even though I know this pattern returns a cursor, I found myself looking around to confirm that the auto was hiding a cursor. I would prefer to just say Cursor.

[arphaman]

2015-06-29 18:36 GMT-07:00 Sean Silva <[e-mail]>:

LGTM with a nit.

REPOSITORY

rL LLVM

Comment at: lib/CodeGen/MIRParser/MILexer.cpp:182
@@ -164,16 +181,3 @@

• auto Char = C.peek();
• if (isalpha(Char) || Char == '_')
• return lexIdentifier(C, Token).remaining();
• if (Char == '%') {
• if (C.remaining().startswith("%bb."))
• return lexMachineBasicBlock(C, Token, ErrorCallback).remaining();
• return lexPercent(C, Token).remaining();
• }
• if (Char == '@')
• return lexGlobalValue(C, Token).remaining();
• if (isdigit(Char) || (Char == '-' && isdigit(C.peek(1))))
• return lexIntegerLiteral(C, Token).remaining();
• MIToken::TokenKind Kind = symbolToken(Char);
• if (Kind != MIToken::Error)
• return lexSymbol(C, Kind, Token).remaining();

+ if (auto R = maybeLexIdentifier(C, Token))

+ return R.remaining();

Even though I know this pattern returns a cursor, I found myself looking
around to confirm that the auto was hiding a cursor. I would prefer to
just say Cursor.

Thanks, I will commit this patch with 'auto' replaced by 'Cursor'.
Alex

http://reviews.llvm.org/D10817

EMAIL PREFERENCES

http://reviews.llvm.org/settings/panel/emailpreferences/

lib/CodeGen/MIRParser/MILexer.cpp

 
 /// This class provides a way to iterate and get characters from the source
 /// string.
 class Cursor {
   const char *Ptr;
   const char *End;
 
 public:
+  Cursor(NoneType) : Ptr(nullptr), End(nullptr) {}
+
   explicit Cursor(StringRef Str) {
     Ptr = Str.data();
     End = Ptr + Str.size();
   }
 
   bool isEOF() const { return Ptr == End; }
 
   char peek(int I = 0) const { return End - Ptr <= I ? 0 : Ptr[I]; }
 
   void advance(unsigned I = 1) { Ptr += I; }
 
   StringRef remaining() const { return StringRef(Ptr, End - Ptr); }
 
   StringRef upto(Cursor C) const {
     assert(C.Ptr >= Ptr && C.Ptr <= End);
     return StringRef(Ptr, C.Ptr - Ptr);
   }
 
   StringRef::iterator location() const { return Ptr; }
+
+  operator bool() const { return Ptr != nullptr; }
 };
 
 } // end anonymous namespace
 
 /// Skip the leading whitespace characters and return the updated cursor.
 static Cursor skipWhitespace(Cursor C) {
   while (isspace(C.peek()))
     C.advance();
   return C;
 }
 
 static bool isIdentifierChar(char C) {
   return isalpha(C) || isdigit(C) || C == '_' || C == '-' || C == '.';
 }
 
-static Cursor lexIdentifier(Cursor C, MIToken &Token) {
+static Cursor maybeLexIdentifier(Cursor C, MIToken &Token) {
+  if (!isalpha(C.peek()) && C.peek() != '_')
+    return None;
   auto Range = C;
   while (isIdentifierChar(C.peek()))
     C.advance();
   auto Identifier = Range.upto(C);
   Token = MIToken(Identifier == "_" ? MIToken::underscore : MIToken::Identifier,
                   Identifier);
   return C;
 }
 
-static Cursor lexMachineBasicBlock(
+static Cursor maybeLexMachineBasicBlock(
     Cursor C, MIToken &Token,
     function_ref<void(StringRef::iterator Loc, const Twine &)> ErrorCallback) {
+  if (!C.remaining().startswith("%bb."))
+    return None;
   auto Range = C;
   C.advance(4); // Skip '%bb.'
   if (!isdigit(C.peek())) {
     Token = MIToken(MIToken::Error, C.remaining());
     ErrorCallback(C.location(), "expected a number after '%bb.'");
     return C;
   }
   auto NumberRange = C;
   while (isdigit(C.peek()))
     C.advance();
   StringRef Number = NumberRange.upto(C);
   unsigned StringOffset = 4 + Number.size(); // Drop '%bb.<id>'
   if (C.peek() == '.') {
     C.advance(); // Skip '.'
     ++StringOffset;
     while (isIdentifierChar(C.peek()))
       C.advance();
   }
   Token = MIToken(MIToken::MachineBasicBlock, Range.upto(C), APSInt(Number),
                   StringOffset);
   return C;
 }
 
-static Cursor lexPercent(Cursor C, MIToken &Token) {
+static Cursor maybeLexRegister(Cursor C, MIToken &Token) {
+  if (C.peek() != '%')
+    return None;
   auto Range = C;
   C.advance(); // Skip '%'
   while (isIdentifierChar(C.peek()))
     C.advance();
   Token = MIToken(MIToken::NamedRegister, Range.upto(C),
                   /*StringOffset=*/1); // Drop the '%'
   return C;
 }
 
-static Cursor lexGlobalValue(Cursor C, MIToken &Token) {
+static Cursor maybeLexGlobalValue(Cursor C, MIToken &Token) {
+  if (C.peek() != '@')
+    return None;
   auto Range = C;
   C.advance(); // Skip the '@'
   // TODO: add support for quoted names.
   if (!isdigit(C.peek())) {
     while (isIdentifierChar(C.peek()))
       C.advance();
     Token = MIToken(MIToken::NamedGlobalValue, Range.upto(C),
                     /*StringOffset=*/1); // Drop the '@'
     return C;
   }
   auto NumberRange = C;
   while (isdigit(C.peek()))
     C.advance();
   Token =
       MIToken(MIToken::GlobalValue, Range.upto(C), APSInt(NumberRange.upto(C)));
   return C;
 }
 
-static Cursor lexIntegerLiteral(Cursor C, MIToken &Token) {
+static Cursor maybeLexIntegerLiteral(Cursor C, MIToken &Token) {
+  if (!isdigit(C.peek()) && (C.peek() != '-' || !isdigit(C.peek(1))))
+    return None;
   auto Range = C;
   C.advance();
   while (isdigit(C.peek()))
     C.advance();
   StringRef StrVal = Range.upto(C);
   Token = MIToken(MIToken::IntegerLiteral, StrVal, APSInt(StrVal));
   return C;
 }
 
 static MIToken::TokenKind symbolToken(char C) {
   switch (C) {
   case ',':
     return MIToken::comma;
   case '=':
     return MIToken::equal;
   default:
     return MIToken::Error;
   }
 }
 
-static Cursor lexSymbol(Cursor C, MIToken::TokenKind Kind, MIToken &Token) {
+static Cursor maybeLexSymbol(Cursor C, MIToken &Token) {
+  auto Kind = symbolToken(C.peek());
+  if (Kind == MIToken::Error)
+    return None;
   auto Range = C;
   C.advance();
   Token = MIToken(Kind, Range.upto(C));
   return C;
 }
 
 StringRef llvm::lexMIToken(
     StringRef Source, MIToken &Token,
     function_ref<void(StringRef::iterator Loc, const Twine &)> ErrorCallback) {
   auto C = skipWhitespace(Cursor(Source));
   if (C.isEOF()) {
     Token = MIToken(MIToken::Eof, C.remaining());
     return C.remaining();
   }
 
-  auto Char = C.peek();
-  if (isalpha(Char) || Char == '_')
-    return lexIdentifier(C, Token).remaining();
-  if (Char == '%') {
-    if (C.remaining().startswith("%bb."))
-      return lexMachineBasicBlock(C, Token, ErrorCallback).remaining();
-    return lexPercent(C, Token).remaining();
-  }
-  if (Char == '@')
-    return lexGlobalValue(C, Token).remaining();
-  if (isdigit(Char) || (Char == '-' && isdigit(C.peek(1))))
-    return lexIntegerLiteral(C, Token).remaining();
-  MIToken::TokenKind Kind = symbolToken(Char);
-  if (Kind != MIToken::Error)
-    return lexSymbol(C, Kind, Token).remaining();
+  if (auto R = maybeLexIdentifier(C, Token))
+    return R.remaining();
+  if (auto R = maybeLexMachineBasicBlock(C, Token, ErrorCallback))
+    return R.remaining();
+  if (auto R = maybeLexRegister(C, Token))
+    return R.remaining();
+  if (auto R = maybeLexGlobalValue(C, Token))
+    return R.remaining();
+  if (auto R = maybeLexIntegerLiteral(C, Token))
+    return R.remaining();
+  if (auto R = maybeLexSymbol(C, Token))
+    return R.remaining();
+
   Token = MIToken(MIToken::Error, C.remaining());
   ErrorCallback(C.location(),
-                Twine("unexpected character '") + Twine(Char) + "'");
+                Twine("unexpected character '") + Twine(C.peek()) + "'");
   return C.remaining();
 }