[PATCH] D153914: [clang-cl] Enable concatenation of predefined identifiers

This revision was automatically updated to reflect the committed changes. Closed by commit rG66c43fbd271a: Enable concatenation of predefined identifiers (authored by RIscRIpt, committed by aaron.ballman). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D153914: [clang-cl] Enable concatenation of predefined identifiers

RIscRIpt added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:2034-2035 + // StringToks is read-only, we need backstorage for expanded macros + std::vector ExpandedToks; + if (getLangOpts().MicrosoftExt) aaron.ballman wrote: > Thanks, that's

[PATCH] D153914: [clang-cl] Enable concatenation of predefined identifiers

RIscRIpt updated this revision to Diff 548502. RIscRIpt marked an inline comment as done. RIscRIpt added a comment. Updated comment; rebased onto main Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153914/new/ https://reviews.llvm.org/D153914

[PATCH] D153914: [clang-cl] Enable concatenation of predefined identifiers

aaron.ballman added inline comments. Comment at: clang/lib/Sema/Sema.cpp:1494-1505 +Decl *Sema::getCurLocalScopeDecl() { + if (const BlockScopeInfo *BSI = getCurBlock()) +return BSI->TheDecl; + else if (const LambdaScopeInfo *LSI = getCurLambda()) +return

[PATCH] D153914: [clang-cl] Enable concatenation of predefined identifiers

RIscRIpt added a comment. In D153914#4570148 , @cor3ntin wrote: > We will commit on your behalf, what name/email do you want us to use? > Thanks! Thanks for asking; keep the name/email as-is in the commit you see: `Author: Richard Dzenis `

[PATCH] D153914: [clang-cl] Enable concatenation of predefined identifiers

RIscRIpt updated this revision to Diff 548366. RIscRIpt marked 4 inline comments as done. RIscRIpt added a comment. Address review comments, rebase onto main Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153914/new/

[PATCH] D153914: [clang-cl] Enable concatenation of predefined identifiers

aaron.ballman added inline comments. Comment at: clang/lib/Sema/Sema.cpp:1494-1505 +Decl *Sema::getCurLocalScopeDecl() { + if (const BlockScopeInfo *BSI = getCurBlock()) +return BSI->TheDecl; + else if (const LambdaScopeInfo *LSI = getCurLambda()) +return

[PATCH] D153914: [clang-cl] Enable concatenation of predefined identifiers

cor3ntin added a comment. In D153914#4570109 , @RIscRIpt wrote: > Rebased onto main, run local lit clang/tests. (bump for @cor3ntin) > As far as I understand this shall be merged into main by maintainers. > Please let me know if something else is

[PATCH] D153914: [clang-cl] Enable concatenation of predefined identifiers

RIscRIpt updated this revision to Diff 548274. RIscRIpt added a comment. Rebased onto main, run local lit clang/tests. (bump for @cor3ntin) As far as I understand this shall be merged into main by maintainers. Please let me know if something else is expected from me. Repository: rG LLVM

[PATCH] D153914: [clang-cl] Enable concatenation of predefined identifiers

RIscRIpt added a comment. Thank you for the review! I apologize for missing the small details; I should have noticed and addressed them myself. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153914/new/ https://reviews.llvm.org/D153914

[PATCH] D153914: [clang-cl] Enable concatenation of predefined identifiers

RIscRIpt updated this revision to Diff 544235. RIscRIpt marked an inline comment as done. RIscRIpt added a comment. Rename MicrosoftStringLiteralFromPredefined Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153914/new/

[PATCH] D153914: [clang-cl] Enable concatenation of predefined identifiers

tahonermann accepted this revision. tahonermann added a comment. This revision is now accepted and ready to land. I'm happy with this. I noted one last rename suggestion to maintain consistency, but am accepting. Please give Corentin a final chance to raise any concerns. Thank you for being so

[PATCH] D153914: [clang-cl] Enable concatenation of predefined identifiers

RIscRIpt updated this revision to Diff 544114. RIscRIpt marked 4 inline comments as done. RIscRIpt added a comment. Rename diagnostic messages constants Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153914/new/ https://reviews.llvm.org/D153914

[PATCH] D153914: [clang-cl] Enable concatenation of predefined identifiers

tahonermann added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:1981 + // parsed yet). + assert(getLangOpts().MicrosoftExt); + RIscRIpt wrote: > I think I should remove this assertion so this function would be usable > without MS ext, on the other

[PATCH] D153914: [clang-cl] Enable concatenation of predefined identifiers

tahonermann added a comment. I think I'm happy with this. I noted two code changes that I think are no longer needed and offered some final suggestions on some names. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:118-120 +def ext_expand_predef_ms : ExtWarn< +

[PATCH] D153914: [clang-cl] Enable concatenation of predefined identifiers

RIscRIpt added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:1981 + // parsed yet). + assert(getLangOpts().MicrosoftExt); + I think I should remove this assertion so this function would be usable without MS ext, on the other hand it would be a noop

[PATCH] D153914: [clang-cl] Enable concatenation of predefined identifiers

RIscRIpt updated this revision to Diff 544088. RIscRIpt added a comment. Removed irrelevant changes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153914/new/ https://reviews.llvm.org/D153914 Files: clang/include/clang/Basic/DiagnosticGroups.td

[PATCH] D153914: [clang-cl] Enable concatenation of predefined identifiers

RIscRIpt added inline comments. Comment at: clang/include/clang/Basic/TokenKinds.h:87-93 +/// Return true if this token is a predefined macro +/// unexpandable by MSVC preprocessor. +inline bool isUnexpandableMsMacro(TokenKind K) { + return K == tok::kw___FUNCTION__ || K ==

[PATCH] D153914: [clang-cl] Enable concatenation of predefined identifiers

RIscRIpt updated this revision to Diff 544085. RIscRIpt marked 6 inline comments as done. RIscRIpt added a comment. Addressed review comments, rebased onto main Noticable changes: 1. Diagnostics message 2. Support of expansion of these attributes in 2nd argument of `static_assert` 3. Moved

[PATCH] D153914: [clang-cl] Enable concatenation of predefined identifiers

tahonermann added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:2010-2013 +OS << "R\"EFLPM(" + << PredefinedExpr::ComputeName(getPredefinedExprKind(Tok.getKind()), + currentDecl) + << ")EFLPM\"";

[PATCH] D153914: [clang-cl] Enable concatenation of predefined identifiers

cor3ntin added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:2010-2013 +OS << "R\"EFLPM(" + << PredefinedExpr::ComputeName(getPredefinedExprKind(Tok.getKind()), + currentDecl) + << ")EFLPM\"";

[PATCH] D153914: [clang-cl] Enable concatenation of predefined identifiers

tahonermann added inline comments. Comment at: clang/test/Sema/ms_predefined_expr.cpp:156 +static_assert(__FUNCTION__ ""_len == 14); // expected-warning{{string literal concatenation of predefined identifier '__FUNCTION__' is a Microsoft extension}} }

[PATCH] D153914: [clang-cl] Enable concatenation of predefined identifiers

tahonermann added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:2010-2013 +OS << "R\"EFLPM(" + << PredefinedExpr::ComputeName(getPredefinedExprKind(Tok.getKind()), + currentDecl) + << ")EFLPM\"";

[PATCH] D153914: [clang-cl] Enable concatenation of predefined identifiers

cor3ntin added inline comments. Comment at: clang/include/clang/Basic/TokenKinds.h:87-93 +/// Return true if this token is a predefined macro +/// unexpandable by MSVC preprocessor. +inline bool isUnexpandableMsMacro(TokenKind K) { + return K == tok::kw___FUNCTION__ || K ==

[PATCH] D153914: [clang-cl] Enable concatenation of predefined identifiers

RIscRIpt added a comment. Thanks to your suggestion of testing different types, I realized clang does not support MSVC macros with `u`, `u8`, and `U` prefixes in addition to `L__FUNCDNAME`. By the way, clang replicates MSVC behavior a little bit incorrectly: `L__FUNCTION__` is not a valid

[PATCH] D153914: [clang-cl] Enable concatenation of predefined identifiers

RIscRIpt updated this revision to Diff 543109. RIscRIpt marked 5 inline comments as done. RIscRIpt added a comment. Address review comments, rebase onto main Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153914/new/

[PATCH] D153914: [clang-cl] Enable concatenation of predefined identifiers

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

[PATCH] D153914: [clang-cl] Enable concatenation of predefined identifiers

RIscRIpt added inline comments. Comment at: clang/lib/Parse/ParseExpr.cpp:1310-1311 + case tok::kw_L__FUNCSIG__: // primary-expression: L__FUNCSIG__ [MS] +if (!getLangOpts().MicrosoftExt || +!TokenIsLikeStringLiteral(NextToken(), getLangOpts())) { + Res =

[PATCH] D153914: [clang-cl] Enable concatenation of predefined identifiers

RIscRIpt updated this revision to Diff 542412. RIscRIpt marked 5 inline comments as done. RIscRIpt added a comment. Addressed review comments, rebased onto main Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153914/new/

[PATCH] D153914: [clang-cl] Enable concatenation of predefined identifiers

RIscRIpt added inline comments. Comment at: clang/include/clang/Basic/TokenKinds.h:87-93 +/// Return true if this token is a predefined macro +/// unexpandable by MSVC preprocessor. +inline bool isUnexpandableMsMacro(TokenKind K) { + return K == tok::kw___FUNCTION__ || K ==

[PATCH] D153914: [clang-cl] Enable concatenation of predefined identifiers

tahonermann added inline comments. Comment at: clang/include/clang/Basic/TokenKinds.h:87-93 +/// Return true if this token is a predefined macro +/// unexpandable by MSVC preprocessor. +inline bool isUnexpandableMsMacro(TokenKind K) { + return K == tok::kw___FUNCTION__ || K ==

[PATCH] D153914: [clang-cl] Enable concatenation of predefined identifiers

cor3ntin added inline comments. Comment at: clang/include/clang/Basic/TokenKinds.h:87-93 +/// Return true if this token is a predefined macro +/// unexpandable by MSVC preprocessor. +inline bool isUnexpandableMsMacro(TokenKind K) { + return K == tok::kw___FUNCTION__ || K ==

[PATCH] D153914: [clang-cl] Enable concatenation of predefined identifiers

tahonermann requested changes to this revision. tahonermann added inline comments. This revision now requires changes to proceed. Comment at: clang/include/clang/Basic/TokenKinds.h:87-93 +/// Return true if this token is a predefined macro +/// unexpandable by MSVC preprocessor.

[PATCH] D153914: [clang-cl] Enable concatenation of predefined identifiers

RIscRIpt updated this revision to Diff 540741. RIscRIpt marked 7 inline comments as done. RIscRIpt added a comment. Addressed review comments, rebased onto main Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153914/new/

[PATCH] D153914: [clang-cl] Enable concatenation of predefined identifiers

RIscRIpt added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:1972-1987 + SmallString<64> Str; + llvm::raw_svector_ostream OS(Str); + Token Exp; + Exp.startToken(); + if (Tok.getKind() == tok::kw_L__FUNCTION__ || + Tok.getKind() ==

[PATCH] D153914: [clang-cl] Enable concatenation of predefined identifiers

cor3ntin added inline comments. Comment at: clang/lib/Parse/ParseExpr.cpp:3288-3291 + assert( + (StringToks.size() != 1 || + !tok::isUnexpandableMsMacro(StringToks[0].getKind())) && + "single predefined identifiers shall be handled by ActOnPredefinedExpr");

[PATCH] D153914: [clang-cl] Enable concatenation of predefined identifiers

RIscRIpt added inline comments. Comment at: clang/include/clang/Basic/TokenKinds.h:87-93 +/// Return true if this token is a predefined macro +/// unexpandable by MSVC preprocessor. +inline bool isUnexpandableMsMacro(TokenKind K) { + return K == tok::kw___FUNCTION__ || K ==

[PATCH] D153914: [clang-cl] Enable concatenation of predefined identifiers

RIscRIpt updated this revision to Diff 540600. RIscRIpt marked 17 inline comments as done. RIscRIpt added a comment. Addressed review comments, rebased onto main. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153914/new/

[PATCH] D153914: [clang-cl] Enable concatenation of predefined identifiers

tahonermann requested changes to this revision. tahonermann added inline comments. This revision now requires changes to proceed. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:118-120 +def ext_concat_predef_ms : ExtWarn< + "concatenation of predefined identifier

[PATCH] D153914: [clang-cl] Enable concatenation of predefined identifiers

RIscRIpt updated this revision to Diff 540427. RIscRIpt added a comment. Rebased onto main Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153914/new/ https://reviews.llvm.org/D153914 Files: clang/include/clang/Basic/DiagnosticGroups.td

[PATCH] D153914: [clang-cl] Enable concatenation of predefined identifiers

RIscRIpt added inline comments. Comment at: clang/include/clang/Parse/Parser.h:578-582 + bool isTokenConcatenable() const { +return isTokenStringLiteral() || + getLangOpts().MicrosoftExt && + tok::isMSPredefinedMacro(Tok.getKind()); + }

[PATCH] D153914: [clang-cl] Enable concatenation of predefined identifiers

RIscRIpt updated this revision to Diff 540395. RIscRIpt added a comment. Addressed review comments. In D153914#4497696 , @aaron.ballman wrote: > Sema seems like the wrong place to perform such concatenations, but given > that adjacent string literals

[PATCH] D153914: [clang-cl] Enable concatenation of predefined identifiers

cor3ntin added inline comments. Comment at: clang/lib/Parse/ParseExpr.cpp:1300 case tok::kw___PRETTY_FUNCTION__: // primary-expression: __P..Y_F..N__ [GNU] -Res = Actions.ActOnPredefinedExpr(Tok.getLocation(), SavedKind); -ConsumeToken(); +Res =

[PATCH] D153914: [clang-cl] Enable concatenation of predefined identifiers

cor3ntin added inline comments. Comment at: clang/include/clang/Parse/Parser.h:578-582 + bool isTokenConcatenable() const { +return isTokenStringLiteral() || + getLangOpts().MicrosoftExt && + tok::isMSPredefinedMacro(Tok.getKind()); + }

[PATCH] D153914: [clang-cl] Enable concatenation of predefined identifiers

aaron.ballman added reviewers: cor3ntin, tahonermann. aaron.ballman added a comment. Adding some other reviewers for more opinions while I ponder the changes. Sema seems like the wrong place to perform such concatenations, but given that adjacent string literals are concatenated during phase 6,

[PATCH] D153914: [clang-cl] Enable concatenation of predefined identifiers

RIscRIpt added a comment. Added @aaron.ballman as reviewer, because he was reviewer of related patch: 878e590503dff Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153914/new/

[PATCH] D153914: [clang-cl] Enable concatenation of predefined identifiers

RIscRIpt updated this revision to Diff 538025. RIscRIpt added a comment. Rebased onto main Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153914/new/ https://reviews.llvm.org/D153914 Files: clang/include/clang/Basic/DiagnosticGroups.td

[PATCH] D153914: [clang-cl] Enable concatenation of predefined identifiers

RIscRIpt added a comment. GitHub issue: https://github.com/llvm/llvm-project/issues/63563 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153914/new/ https://reviews.llvm.org/D153914 ___ cfe-commits

[PATCH] D153914: [clang-cl] Enable concatenation of predefined identifiers

RIscRIpt created this revision. Herald added a project: All. RIscRIpt requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D153914 Files: