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

Dmitri Gribenko via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Jul 9 08:04:36 PDT 2020


gribozavr2 added inline comments.


================
Comment at: clang/include/clang/Lex/LiteralSupport.h:57-59
-  NumericLiteralParser(StringRef TokSpelling,
-                       SourceLocation TokLoc,
-                       Preprocessor &PP);
----------------
eduucaldas wrote:
> 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.
I'd prefer a simpler public API over convenience.


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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83480





More information about the cfe-commits mailing list