[PATCH] D153914: [clang-cl] Enable concatenation of predefined identifiers
Tom Honermann via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri Jul 21 14:27:00 PDT 2023
tahonermann requested changes to this revision.
tahonermann added a comment.
This revision now requires changes to proceed.
I think this is looking good. The only big thing I noticed is some code that looks like it should have been removed with the last set of changes. Otherwise, just some minor concerns and suggestions.
================
Comment at: clang/lib/Parse/ParseExpr.cpp:1299-1301
+ Res = Actions.ActOnPredefinedExpr(Tok.getLocation(), SavedKind);
+ ConsumeToken();
+ break;
----------------
I believe this code should be removed now; it is no longer reachable.
================
Comment at: clang/lib/Parse/ParseExpr.cpp:1309-1311
+ if (!(getLangOpts().MicrosoftExt &&
+ TokenIsLikeStringLiteral(Tok, getLangOpts()) &&
+ TokenIsLikeStringLiteral(NextToken(), getLangOpts()))) {
----------------
I think this is good, but how about a comment to make the intent more clear?
================
Comment at: clang/lib/Sema/SemaExpr.cpp:1903
+ case tok::kw___PRETTY_FUNCTION__:
+ return PredefinedExpr::PrettyFunction;
+ }
----------------
================
Comment at: clang/lib/Sema/SemaExpr.cpp:2010-2013
+ OS << "R\"EFLPM("
+ << PredefinedExpr::ComputeName(getPredefinedExprKind(Tok.getKind()),
+ currentDecl)
+ << ")EFLPM\"";
----------------
"EFLPM"? I'm assuming this is an attempt to avoid an accidental clash with the computed name. Does this suffice to ensure a clash can't happen? If not, can we do better? Per http://eel.is/c++draft/lex.string#nt:r-char and http://eel.is/c++draft/lex.charset#1, we have a lot of flexibility regarding which characters to use.
================
Comment at: clang/test/Sema/ms_predefined_expr.cpp:115
+ // expected-warning{{string literal concatenation of predefined identifier 'L__FUNCSIG__' is a Microsoft extension}}
+}
+
----------------
How about testing some other encoding prefix combinations?
u"" __FUNCTION // Ok; UTF-16 string literal
u"" L__FUNCTION__ // ill-formed.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D153914/new/
https://reviews.llvm.org/D153914
More information about the cfe-commits
mailing list