[PATCH] D105759: [WIP] Implement P2361 Unevaluated string literals

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Sep 28 10:09:29 PDT 2021


aaron.ballman added a comment.

Some naming nits. There are two open questions also: one about module behavior and one about a TODO comment in the patch. If we don't hear back about the modules question, I think that can be handled in a follow-up.



================
Comment at: clang/include/clang/Lex/LiteralSupport.h:207
 
+enum class StringLiteralKind {
+  Evaluated,
----------------
Slight renaming so nobody thinks this is going to be about wide vs narrow vs u8, etc.


================
Comment at: clang/include/clang/Lex/LiteralSupport.h:233
+      ArrayRef<Token> StringToks, Preprocessor &PP,
+      StringLiteralKind StringKind = StringLiteralKind::Evaluated);
+  StringLiteralParser(ArrayRef<Token> StringToks, const SourceManager &sm,
----------------
We should rename anything mentioning `StringKind` similarly -- this will also help avoid confusion with the `StringKind` type in Expr.h.


================
Comment at: clang/include/clang/Lex/LiteralSupport.h:246
   bool Pascal;
+  StringLiteralKind StringKind;
 
----------------
Can we make this private now rather than letting callers access it directly?


================
Comment at: clang/lib/Lex/LiteralSupport.cpp:1704
+
+  auto CheckStringKind = [&](const Token &Tok) {
+    if (isUnevaluated() && Tok.getKind() != tok::string_literal) {
----------------
When I hear "check" I think it'll return a value; I think this name is a bit more clear.


================
Comment at: clang/lib/Lex/LiteralSupport.cpp:1821-1823
+                << UDSuffixBuf << UDSuffix
+                << SourceRange(UDSuffixTokLoc, UDSuffixTokLoc)
+                << SourceRange(TokLoc, TokLoc);
----------------



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105759



More information about the cfe-commits mailing list