[PATCH] D83480: Refactored NumericLiteralParser to not require a Preprocessor

Thu Jul 9 07:38:41 PDT 2020

eduucaldas accepted this revision.
eduucaldas added a comment.
This revision is now accepted and ready to land.

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

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

================
Comment at: clang/include/clang/Lex/LiteralSupport.h:57-59
-  NumericLiteralParser(StringRef TokSpelling,
-                       SourceLocation TokLoc,
-                       Preprocessor &PP);
----------------
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.

================
Comment at: clang/lib/Lex/LiteralSupport.cpp:766
+      !isValidUDSuffix(LangOpts, StringRef(s, ThisTokEnd - s))) {
+    Diags.Report(AdvanceToTokenCharacter(TokLoc, s - ThisTokBegin),
+                 diag::err_invalid_digit)
----------------
How about just using `Lexer::AdvanceToTokenCharacter`? 

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D83480/new/

https://reviews.llvm.org/D83480