Refactored NumericLiteralParser to not require a Preprocessor

[gribozavr]

Summary

We would like to use NumericLiteralParser in the implementation of the
syntax tree builder, and plumbing a preprocessor there seems
inconvenient and superfluous.

[eduucaldas]

Additionally, consider applying a similar change to CharLiteralParser, for consistency.

All the comments are just suggestions. Feel free to land this without answering them.

clang/include/clang/Lex/LiteralSupport.h

57–59

We don't need to remove this constructor, we can keep the same signature and make it call the new constructor. The same is done for StringLiteralParser.

That would allow some callers that don't care much about the implementation details to just use the simpler to write version.

clang/lib/Lex/LiteralSupport.cpp

770

How about just using Lexer::AdvanceToTokenCharacter?

[gribozavr]

Addressed code review comments.

[gribozavr2]

clang/include/clang/Lex/LiteralSupport.h

57–59

I'd prefer a simpler public API over convenience.

clang/lib/Lex/LiteralSupport.cpp

770

Done.

clang/include/clang/Lex/LiteralSupport.h

 
 /// Copy characters from Input to Buf, expanding any UCNs.
 void expandUCNs(SmallVectorImpl<char> &Buf, StringRef Input);
 
 /// NumericLiteralParser - This performs strict semantic analysis of the content
 /// of a ppnumber, classifying it as either integer, floating, or erroneous,
 /// determines the radix of the value and can convert it to a useful value.
 class NumericLiteralParser {
-  Preprocessor &PP; // needed for diagnostics
+  const SourceManager &SM;
+  const LangOptions &LangOpts;
+  DiagnosticsEngine &Diags;
 
   const char *const ThisTokBegin;
   const char *const ThisTokEnd;
   const char *DigitsBegin, *SuffixBegin; // markers
   const char *s; // cursor
 
   unsigned radix;
 
   bool saw_exponent, saw_period, saw_ud_suffix, saw_fixed_point_suffix;
 
   SmallString<32> UDSuffixBuf;
 
 public:
-  NumericLiteralParser(StringRef TokSpelling,
-                       SourceLocation TokLoc,
-                       Preprocessor &PP);
+  NumericLiteralParser(StringRef TokSpelling, SourceLocation TokLoc,
+                       const SourceManager &SM, const LangOptions &LangOpts,
+                       const TargetInfo &Target, DiagnosticsEngine &Diags);
   bool hadError : 1;
   bool isUnsigned : 1;
   bool isLong : 1;          // This is *not* set for long long.
   bool isLongLong : 1;
   bool isHalf : 1;          // 1.0h
   bool isFloat : 1;         // 1.0f
   bool isImaginary : 1;     // 1.0i
   bool isFloat16 : 1;       // 1.0f16

clang/lib/Lex/LiteralSupport.cpp

 ///       long-long-suffix: one of
 ///         ll LL
 ///
 ///       floating-constant: [C99 6.4.4.2]
 ///         TODO: add rules...
 ///
 NumericLiteralParser::NumericLiteralParser(StringRef TokSpelling,
                                            SourceLocation TokLoc,
-                                           Preprocessor &PP)
-  : PP(PP), ThisTokBegin(TokSpelling.begin()), ThisTokEnd(TokSpelling.end()) {
+                                           const SourceManager &SM,
+                                           const LangOptions &LangOpts,
+                                           const TargetInfo &Target,
+                                           DiagnosticsEngine &Diags)
+    : SM(SM), LangOpts(LangOpts), Diags(Diags),
+      ThisTokBegin(TokSpelling.begin()), ThisTokEnd(TokSpelling.end()) {
 
   // This routine assumes that the range begin/end matches the regex for integer
   // and FP constants (specifically, the 'pp-number' regex), and assumes that
   // the byte at "*end" is both valid and not part of the regex.  Because of
   // this, it doesn't have to check for 'overscan' in various places.
   assert(!isPreprocessingNumberBody(*ThisTokEnd) && "didn't maximally munch?");
 
   s = DigitsBegin = ThisTokBegin;
if (s == ThisTokEnd) {
         return;
     }
   }
 
   SuffixBegin = s;
   checkSeparator(TokLoc, s, CSK_AfterDigits);
 
   // Initial scan to lookahead for fixed point suffix.
-  if (PP.getLangOpts().FixedPoint) {
+  if (LangOpts.FixedPoint) {
     for (const char *c = s; c != ThisTokEnd; ++c) {
       if (*c == 'r' || *c == 'k' || *c == 'R' || *c == 'K') {
         saw_fixed_point_suffix = true;
         break;
       }
     }
   }
 
   // Parse the suffix.  At this point we can classify whether we have an FP or
   // integer constant.
   bool isFixedPointConstant = isFixedPointLiteral();
   bool isFPConstant = isFloatingLiteral();
 
   // Loop over all of the characters of the suffix.  If we see something bad,
   // we break out of the loop.
   for (; s != ThisTokEnd; ++s) {
     switch (*s) {
     case 'R':
     case 'r':
-      if (!PP.getLangOpts().FixedPoint) break;
+      if (!LangOpts.FixedPoint)
+        break;
       if (isFract || isAccum) break;
       if (!(saw_period || saw_exponent)) break;
       isFract = true;
       continue;
     case 'K':
     case 'k':
-      if (!PP.getLangOpts().FixedPoint) break;
+      if (!LangOpts.FixedPoint)
+        break;
       if (isFract || isAccum) break;
       if (!(saw_period || saw_exponent)) break;
       isAccum = true;
       continue;
     case 'h':      // FP Suffix for "half".
     case 'H':
       // OpenCL Extension v1.2 s9.5 - h or H suffix for half type.
-      if (!(PP.getLangOpts().Half || PP.getLangOpts().FixedPoint)) break;
+      if (!(LangOpts.Half || LangOpts.FixedPoint))
+        break;
       if (isIntegerLiteral()) break;  // Error for integer constant.
       if (isHalf || isFloat || isLong) break; // HH, FH, LH invalid.
       isHalf = true;
       continue;  // Success.
     case 'f':      // FP Suffix for "float"
     case 'F':
       if (!isFPConstant) break;  // Error for integer constant.
       if (isHalf || isFloat || isLong || isFloat128)
         break; // HF, FF, LF, QF invalid.
 
       // CUDA host and device may have different _Float16 support, therefore
       // allows f16 literals to avoid false alarm.
       // ToDo: more precise check for CUDA.
-      if ((PP.getTargetInfo().hasFloat16Type() || PP.getLangOpts().CUDA) &&
-          s + 2 < ThisTokEnd && s[1] == '1' && s[2] == '6') {
+      if ((Target.hasFloat16Type() || LangOpts.CUDA) && s + 2 < ThisTokEnd &&
+          s[1] == '1' && s[2] == '6') {
         s += 2; // success, eat up 2 characters.
         isFloat16 = true;
         continue;
       }
 
       isFloat = true;
       continue;  // Success.
     case 'q':    // FP Suffix for "__float128"
case 'L':
       if (s[1] == s[0]) {
         assert(s + 1 < ThisTokEnd && "didn't maximally munch?");
         if (isFPConstant) break;        // long long invalid for floats.
         isLongLong = true;
         ++s;  // Eat both of them.
       } else {
         isLong = true;
       }
-      continue;  // Success.
+      continue; // Success.
     case 'i':
     case 'I':
-      if (PP.getLangOpts().MicrosoftExt) {
+      if (LangOpts.MicrosoftExt) {
         if (isLong || isLongLong || MicrosoftInteger)
           break;
 
         if (!isFPConstant) {
           // Allow i8, i16, i32, and i64.
           switch (s[1]) {
           case '8':
             s += 2; // i8 suffix
for (; s != ThisTokEnd; ++s) {
     // If we reached here, there was an error or a ud-suffix.
     break;
   }
 
   // "i", "if", and "il" are user-defined suffixes in C++1y.
   if (s != ThisTokEnd || isImaginary) {
     // FIXME: Don't bother expanding UCNs if !tok.hasUCN().
     expandUCNs(UDSuffixBuf, StringRef(SuffixBegin, ThisTokEnd - SuffixBegin));
-    if (isValidUDSuffix(PP.getLangOpts(), UDSuffixBuf)) {
+    if (isValidUDSuffix(LangOpts, UDSuffixBuf)) {
       if (!isImaginary) {
         // Any suffix pieces we might have parsed are actually part of the
         // ud-suffix.
         isLong = false;
         isUnsigned = false;
         isLongLong = false;
         isFloat = false;
         isFloat16 = false;
         isHalf = false;
         isImaginary = false;
         MicrosoftInteger = 0;
         saw_fixed_point_suffix = false;
         isFract = false;
         isAccum = false;
       }
 
       saw_ud_suffix = true;
       return;
     }
 
     if (s != ThisTokEnd) {
       // Report an error if there are any.
-      PP.Diag(PP.AdvanceToTokenCharacter(TokLoc, SuffixBegin - ThisTokBegin),
+      Diags.Report(Lexer::AdvanceToTokenCharacter(
+                       TokLoc, SuffixBegin - ThisTokBegin, SM, LangOpts),
                    diag::err_invalid_suffix_constant)
           << StringRef(SuffixBegin, ThisTokEnd - SuffixBegin)
           << (isFixedPointConstant ? 2 : isFPConstant);
       hadError = true;
     }
   }
 
   if (!hadError && saw_fixed_point_suffix) {
     assert(isFract || isAccum);
   }
 }
 
 /// ParseDecimalOrOctalCommon - This method is called for decimal or octal
 /// numbers. It issues an error for illegal digits, and handles floating point
 /// parsing. If it detects a floating point number, the radix is set to 10.
 void NumericLiteralParser::ParseDecimalOrOctalCommon(SourceLocation TokLoc){
   assert((radix == 8 || radix == 10) && "Unexpected radix");
 
   // If we have a hex digit other than 'e' (which denotes a FP exponent) then
   // the code is using an incorrect base.
   if (isHexDigit(*s) && *s != 'e' && *s != 'E' &&
-      !isValidUDSuffix(PP.getLangOpts(), StringRef(s, ThisTokEnd - s))) {
-    PP.Diag(PP.AdvanceToTokenCharacter(TokLoc, s-ThisTokBegin),
-            diag::err_invalid_digit) << StringRef(s, 1) << (radix == 8 ? 1 : 0);
+      !isValidUDSuffix(LangOpts, StringRef(s, ThisTokEnd - s))) {
+    Diags.Report(
+        Lexer::AdvanceToTokenCharacter(TokLoc, s - ThisTokBegin, SM, LangOpts),
+        diag::err_invalid_digit)
+        << StringRef(s, 1) << (radix == 8 ? 1 : 0);
     hadError = true;
     return;
   }
 
   if (*s == '.') {
     checkSeparator(TokLoc, s, CSK_AfterDigits);
     s++;
     radix = 10;
if (*s == 'e' || *s == 'E') { // exponent
     saw_exponent = true;
     if (s != ThisTokEnd && (*s == '+' || *s == '-'))  s++; // sign
     const char *first_non_digit = SkipDigits(s);
     if (containsDigits(s, first_non_digit)) {
       checkSeparator(TokLoc, s, CSK_BeforeDigits);
       s = first_non_digit;
     } else {
       if (!hadError) {
-        PP.Diag(PP.AdvanceToTokenCharacter(TokLoc, Exponent-ThisTokBegin),
+        Diags.Report(Lexer::AdvanceToTokenCharacter(
+                         TokLoc, Exponent - ThisTokBegin, SM, LangOpts),
                      diag::err_exponent_has_no_digits);
         hadError = true;
       }
       return;
     }
   }
 }
 
 /// Determine whether a suffix is a valid ud-suffix. We avoid treating reserved
void NumericLiteralParser::checkSeparator(SourceLocation TokLoc,
   if (IsAfterDigits == CSK_AfterDigits) {
     if (Pos == ThisTokBegin)
       return;
     --Pos;
   } else if (Pos == ThisTokEnd)
     return;
 
   if (isDigitSeparator(*Pos)) {
-    PP.Diag(PP.AdvanceToTokenCharacter(TokLoc, Pos - ThisTokBegin),
+    Diags.Report(Lexer::AdvanceToTokenCharacter(TokLoc, Pos - ThisTokBegin, SM,
+                                                LangOpts),
                  diag::err_digit_separator_not_between_digits)
         << IsAfterDigits;
     hadError = true;
   }
 }
 
 /// ParseNumberStartingWithZero - This method is called when the first character
 /// of the number is found to be a zero.  This means it is either an octal
 /// number (like '04') or a hex number ('0x123a') a binary number ('0b1010') or
 /// a floating point number (01239.123e4).  Eat the prefix, determining the
if (s == ThisTokEnd) {
       s = SkipHexDigits(s);
       if (containsDigits(floatDigitsBegin, s))
         HasSignificandDigits = true;
       if (HasSignificandDigits)
         checkSeparator(TokLoc, floatDigitsBegin, CSK_BeforeDigits);
     }
 
     if (!HasSignificandDigits) {
-      PP.Diag(PP.AdvanceToTokenCharacter(TokLoc, s - ThisTokBegin),
+      Diags.Report(Lexer::AdvanceToTokenCharacter(TokLoc, s - ThisTokBegin, SM,
+                                                  LangOpts),
                    diag::err_hex_constant_requires)
-          << PP.getLangOpts().CPlusPlus << 1;
+          << LangOpts.CPlusPlus << 1;
       hadError = true;
       return;
     }
 
     // A binary exponent can appear with or with a '.'. If dotted, the
     // binary exponent is required.
     if (*s == 'p' || *s == 'P') {
       checkSeparator(TokLoc, s, CSK_AfterDigits);
       const char *Exponent = s;
       s++;
       saw_exponent = true;
       if (s != ThisTokEnd && (*s == '+' || *s == '-'))  s++; // sign
       const char *first_non_digit = SkipDigits(s);
       if (!containsDigits(s, first_non_digit)) {
         if (!hadError) {
-          PP.Diag(PP.AdvanceToTokenCharacter(TokLoc, Exponent-ThisTokBegin),
+          Diags.Report(Lexer::AdvanceToTokenCharacter(
+                           TokLoc, Exponent - ThisTokBegin, SM, LangOpts),
                        diag::err_exponent_has_no_digits);
           hadError = true;
         }
         return;
       }
       checkSeparator(TokLoc, s, CSK_BeforeDigits);
       s = first_non_digit;
 
-      if (!PP.getLangOpts().HexFloats)
-        PP.Diag(TokLoc, PP.getLangOpts().CPlusPlus
+      if (!LangOpts.HexFloats)
+        Diags.Report(TokLoc, LangOpts.CPlusPlus
                                  ? diag::ext_hex_literal_invalid
                                  : diag::ext_hex_constant_invalid);
-      else if (PP.getLangOpts().CPlusPlus17)
-        PP.Diag(TokLoc, diag::warn_cxx17_hex_literal);
+      else if (LangOpts.CPlusPlus17)
+        Diags.Report(TokLoc, diag::warn_cxx17_hex_literal);
     } else if (saw_period) {
-      PP.Diag(PP.AdvanceToTokenCharacter(TokLoc, s - ThisTokBegin),
+      Diags.Report(Lexer::AdvanceToTokenCharacter(TokLoc, s - ThisTokBegin, SM,
+                                                  LangOpts),
                    diag::err_hex_constant_requires)
-          << PP.getLangOpts().CPlusPlus << 0;
+          << LangOpts.CPlusPlus << 0;
       hadError = true;
     }
     return;
   }
 
   // Handle simple binary numbers 0b01010
   if ((c1 == 'b' || c1 == 'B') && (s[1] == '0' || s[1] == '1')) {
     // 0b101010 is a C++1y / GCC extension.
-    PP.Diag(TokLoc,
-            PP.getLangOpts().CPlusPlus14
+    Diags.Report(TokLoc, LangOpts.CPlusPlus14
                              ? diag::warn_cxx11_compat_binary_literal
-              : PP.getLangOpts().CPlusPlus
-                ? diag::ext_binary_literal_cxx14
+                         : LangOpts.CPlusPlus ? diag::ext_binary_literal_cxx14
                                               : diag::ext_binary_literal);
     ++s;
     assert(s < ThisTokEnd && "didn't maximally munch?");
     radix = 2;
     DigitsBegin = s;
     s = SkipBinaryDigits(s);
     if (s == ThisTokEnd) {
       // Done.
     } else if (isHexDigit(*s) &&
-               !isValidUDSuffix(PP.getLangOpts(),
-                                StringRef(s, ThisTokEnd - s))) {
-      PP.Diag(PP.AdvanceToTokenCharacter(TokLoc, s-ThisTokBegin),
-              diag::err_invalid_digit) << StringRef(s, 1) << 2;
+               !isValidUDSuffix(LangOpts, StringRef(s, ThisTokEnd - s))) {
+      Diags.Report(Lexer::AdvanceToTokenCharacter(TokLoc, s - ThisTokBegin, SM,
+                                                  LangOpts),
+                   diag::err_invalid_digit)
+          << StringRef(s, 1) << 2;
       hadError = true;
     }
     // Other suffixes will be diagnosed by the caller.
     return;
   }
 
   // For now, the radix is set to 8. If we discover that we have a
   // floating point constant, the radix will change to 10. Octal floating

clang/lib/Lex/PPExpressions.cpp

static bool EvaluateValue(PPValue &Result, Token &PeekTok, DefinedTracker &DT,
   case tok::numeric_constant: {
     SmallString<64> IntegerBuffer;
     bool NumberInvalid = false;
     StringRef Spelling = PP.getSpelling(PeekTok, IntegerBuffer,
                                               &NumberInvalid);
     if (NumberInvalid)
       return true; // a diagnostic was already reported
 
-    NumericLiteralParser Literal(Spelling, PeekTok.getLocation(), PP);
+    NumericLiteralParser Literal(Spelling, PeekTok.getLocation(),
+                                 PP.getSourceManager(), PP.getLangOpts(),
+                                 PP.getTargetInfo(), PP.getDiagnostics());
     if (Literal.hadError)
       return true; // a diagnostic was already reported.
 
     if (Literal.isFloatingLiteral() || Literal.isImaginary) {
       PP.Diag(PeekTok, diag::err_pp_illegal_floating_literal);
       return true;
     }
     assert(Literal.isIntegerLiteral() && "Unknown ppnumber");

clang/lib/Lex/Preprocessor.cpp

 
 bool Preprocessor::parseSimpleIntegerLiteral(Token &Tok, uint64_t &Value) {
   assert(Tok.is(tok::numeric_constant));
   SmallString<8> IntegerBuffer;
   bool NumberInvalid = false;
   StringRef Spelling = getSpelling(Tok, IntegerBuffer, &NumberInvalid);
   if (NumberInvalid)
     return false;
-  NumericLiteralParser Literal(Spelling, Tok.getLocation(), *this);
+  NumericLiteralParser Literal(Spelling, Tok.getLocation(), getSourceManager(),
+                               getLangOpts(), getTargetInfo(),
+                               getDiagnostics());
   if (Literal.hadError || !Literal.isIntegerLiteral() || Literal.hasUDSuffix())
     return false;
   llvm::APInt APVal(64, 0);
   if (Literal.GetIntegerValue(APVal))
     return false;
   Lex(Tok);
   Value = APVal.getLimitedValue();
   return true;

clang/lib/Sema/SemaExpr.cpp

ExprResult Sema::ActOnNumericConstant(const Token &Tok, Scope *UDLScope) {
   SpellingBuffer.resize(Tok.getLength() + 1);
 
   // Get the spelling of the token, which eliminates trigraphs, etc.
   bool Invalid = false;
   StringRef TokSpelling = PP.getSpelling(Tok, SpellingBuffer, &Invalid);
   if (Invalid)
     return ExprError();
 
-  NumericLiteralParser Literal(TokSpelling, Tok.getLocation(), PP);
+  NumericLiteralParser Literal(TokSpelling, Tok.getLocation(),
+                               PP.getSourceManager(), PP.getLangOpts(),
+                               PP.getTargetInfo(), PP.getDiagnostics());
   if (Literal.hadError)
     return ExprError();
 
   if (Literal.hasUDSuffix()) {
     // We're building a user-defined literal.
     IdentifierInfo *UDSuffix = &Context.Idents.get(Literal.getUDSuffix());
     SourceLocation UDSuffixLoc =
       getUDSuffixLoc(*this, Tok.getLocation(), Literal.getUDSuffixOffset());