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

[Aaron Ballman via Phabricator via cfe-commits]

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
  https://reviews.llvm.org/D153914/new/

https://reviews.llvm.org/D153914

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Lex/LiteralSupport.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Lex/LiteralSupport.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/ms_predefined_expr.cpp
  clang/test/Sema/ms_wide_predefined_expr.cpp

Index: clang/test/Sema/ms_wide_predefined_expr.cpp
===
--- clang/test/Sema/ms_wide_predefined_expr.cpp
+++ /dev/null
@@ -1,28 +0,0 @@
-// RUN: %clang_cc1 %s -fsyntax-only -Wno-unused-value -Wmicrosoft -verify -fms-extensions
-// expected-no-diagnostics
-
-// Wide character predefined identifiers
-#define _STR2WSTR(str) L##str
-#define STR2WSTR(str) _STR2WSTR(str)
-void abcdefghi12(void) {
- const wchar_t (*ss)[12] = (__FUNCTION__);
- static int arr[sizeof(STR2WSTR(__FUNCTION__))==12*sizeof(wchar_t) ? 1 : -1];
- const wchar_t (*ss2)[31] = (__FUNCSIG__);
- static int arr2[sizeof(STR2WSTR(__FUNCSIG__))==31*sizeof(wchar_t) ? 1 : -1];
-}
-
-namespace PR13206 {
-void foo(const wchar_t *);
-
-template class A {
-public:
- void method() {
-  foo(L__FUNCTION__);
- }
-};
-
-void bar() {
- A x;
- x.method();
-}
-}
Index: clang/test/Sema/ms_predefined_expr.cpp
===
--- clang/test/Sema/ms_predefined_expr.cpp
+++ clang/test/Sema/ms_predefined_expr.cpp
@@ -1,9 +1,170 @@
 // RUN: %clang_cc1 %s -fsyntax-only -Wmicrosoft -verify -fms-extensions
 
-void f() {
+using size_t = __SIZE_TYPE__;
+
+// Test array initialization
+void array_init() {
  const char a[] = __FUNCTION__; // expected-warning{{initializing an array from a '__FUNCTION__' predefined identifier is a Microsoft extension}}
  const char b[] = __FUNCDNAME__; // expected-warning{{initializing an array from a '__FUNCDNAME__' predefined identifier is a Microsoft extension}}
  const char c[] = __FUNCSIG__; // expected-warning{{initializing an array from a '__FUNCSIG__' predefined identifier is a Microsoft extension}}
  const char d[] = __func__; // expected-warning{{initializing an array from a '__func__' predefined identifier is a Microsoft extension}}
  const char e[] = __PRETTY_FUNCTION__; // expected-warning{{initializing an array from a '__PRETTY_FUNCTION__' predefined identifier is a Microsoft extension}}
+ const wchar_t f[] = L__FUNCTION__; // expected-warning{{initializing an array from a 'L__FUNCTION__' predefined identifier is a Microsoft extension}}
+ const wchar_t g[] = L__FUNCSIG__; // expected-warning{{initializing an array from a 'L__FUNCSIG__' predefined identifier is a Microsoft extension}}
+}
+
+// Test function local identifiers outside of a function
+const char* g_function = __FUNCTION__;// expected-warning{{predefined identifier is only valid inside function}}
+const char* g_function_lconcat = "" __FUNCTION__; // expected-warning{{predefined identifier is only valid inside function}} \
+  // expected-warning{{expansion of predefined identifier '__FUNCTION__' to a string literal is a Microsoft extension}}
+const char* g_function_rconcat = __FUNCTION__ ""; // expected-warning{{predefined identifier is only valid inside function}} \
+  // expected-warning{{expansion of predefined identifier '__FUNCTION__' to a string literal is a Microsoft extension}}
+
+namespace NS
+{
+  const char* function = __FUNCTION__;// expected-warning{{predefined identifier is only valid inside function}}
+  const char* function_lconcat = "" __FUNCTION__; // expected-warning{{predefined identifier is only valid inside function}} \
+  // expected-warning{{expansion of predefined identifier '__FUNCTION__' to a string literal is a Microsoft extension}}
+  const char* function_rconcat = __FUNCTION__ ""; // expected-warning{{predefined identifier is only valid inside function}} \
+  // expected-warning{{expansion of predefined identifier '__FUNCTION__' to a string literal is a Microsoft extension}}
+
+  struct S
+  {
+static constexpr const char* function = __FUNCTION__;// expected-warning{{predefined identifier is only valid inside function}}
+static constexpr const char* function_lconcat = "" __FUNCTION__; // expected-warning{{predefined identifier is only valid inside function}} \
+ // 

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

[Richard Dzenis via Phabricator via cfe-commits]

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 better. Changed in both places.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153914

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Richard Dzenis via Phabricator via cfe-commits]

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

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Lex/LiteralSupport.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Lex/LiteralSupport.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/ms_predefined_expr.cpp
  clang/test/Sema/ms_wide_predefined_expr.cpp

Index: clang/test/Sema/ms_wide_predefined_expr.cpp
===
--- clang/test/Sema/ms_wide_predefined_expr.cpp
+++ /dev/null
@@ -1,28 +0,0 @@
-// RUN: %clang_cc1 %s -fsyntax-only -Wno-unused-value -Wmicrosoft -verify -fms-extensions
-// expected-no-diagnostics
-
-// Wide character predefined identifiers
-#define _STR2WSTR(str) L##str
-#define STR2WSTR(str) _STR2WSTR(str)
-void abcdefghi12(void) {
- const wchar_t (*ss)[12] = (__FUNCTION__);
- static int arr[sizeof(STR2WSTR(__FUNCTION__))==12*sizeof(wchar_t) ? 1 : -1];
- const wchar_t (*ss2)[31] = (__FUNCSIG__);
- static int arr2[sizeof(STR2WSTR(__FUNCSIG__))==31*sizeof(wchar_t) ? 1 : -1];
-}
-
-namespace PR13206 {
-void foo(const wchar_t *);
-
-template class A {
-public:
- void method() {
-  foo(L__FUNCTION__);
- }
-};
-
-void bar() {
- A x;
- x.method();
-}
-}
Index: clang/test/Sema/ms_predefined_expr.cpp
===
--- clang/test/Sema/ms_predefined_expr.cpp
+++ clang/test/Sema/ms_predefined_expr.cpp
@@ -1,9 +1,170 @@
 // RUN: %clang_cc1 %s -fsyntax-only -Wmicrosoft -verify -fms-extensions
 
-void f() {
+using size_t = __SIZE_TYPE__;
+
+// Test array initialization
+void array_init() {
  const char a[] = __FUNCTION__; // expected-warning{{initializing an array from a '__FUNCTION__' predefined identifier is a Microsoft extension}}
  const char b[] = __FUNCDNAME__; // expected-warning{{initializing an array from a '__FUNCDNAME__' predefined identifier is a Microsoft extension}}
  const char c[] = __FUNCSIG__; // expected-warning{{initializing an array from a '__FUNCSIG__' predefined identifier is a Microsoft extension}}
  const char d[] = __func__; // expected-warning{{initializing an array from a '__func__' predefined identifier is a Microsoft extension}}
  const char e[] = __PRETTY_FUNCTION__; // expected-warning{{initializing an array from a '__PRETTY_FUNCTION__' predefined identifier is a Microsoft extension}}
+ const wchar_t f[] = L__FUNCTION__; // expected-warning{{initializing an array from a 'L__FUNCTION__' predefined identifier is a Microsoft extension}}
+ const wchar_t g[] = L__FUNCSIG__; // expected-warning{{initializing an array from a 'L__FUNCSIG__' predefined identifier is a Microsoft extension}}
+}
+
+// Test function local identifiers outside of a function
+const char* g_function = __FUNCTION__;// expected-warning{{predefined identifier is only valid inside function}}
+const char* g_function_lconcat = "" __FUNCTION__; // expected-warning{{predefined identifier is only valid inside function}} \
+  // expected-warning{{expansion of predefined identifier '__FUNCTION__' to a string literal is a Microsoft extension}}
+const char* g_function_rconcat = __FUNCTION__ ""; // expected-warning{{predefined identifier is only valid inside function}} \
+  // expected-warning{{expansion of predefined identifier '__FUNCTION__' to a string literal is a Microsoft extension}}
+
+namespace NS
+{
+  const char* function = __FUNCTION__;// expected-warning{{predefined identifier is only valid inside function}}
+  const char* function_lconcat = "" __FUNCTION__; // expected-warning{{predefined identifier is only valid inside function}} \
+  // expected-warning{{expansion of predefined identifier '__FUNCTION__' to a string literal is a Microsoft extension}}
+  const char* function_rconcat = __FUNCTION__ ""; // expected-warning{{predefined identifier is only valid inside function}} \
+  // expected-warning{{expansion of predefined identifier '__FUNCTION__' to a string literal is a Microsoft extension}}
+
+  struct S
+  {
+static constexpr const char* function = __FUNCTION__;// expected-warning{{predefined identifier is only valid inside function}}
+static constexpr const char* function_lconcat = "" __FUNCTION__; // expected-warning{{predefined identifier is only valid inside function}} \
+ // expected-warning{{expansion of predefined identifier 

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

[Aaron Ballman via Phabricator via cfe-commits]

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 LSI->CallOperator;
+  else if (const CapturedRegionScopeInfo *CSI = getCurCapturedRegion())
+return CSI->TheCapturedDecl;

RIscRIpt wrote:
> aaron.ballman wrote:
> > Made the function `const` because it's not writing to any fields, removed 
> > `else` because of preceding `return`.
> I would love to, and I tried. But unfortunately it's not possible without 
> introducing more changes: all the member functions (except 
> `getCurFunctionOrMethodDecl()`) are non-const.
> Removed `else`.
Ah, thanks!



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)





Comment at: clang/lib/Sema/SemaExpr.cpp:1998-2000
+if (isa(currentDecl)) {
+  Diag(Tok.getLocation(), diag::ext_predef_outside_function);
+}

RIscRIpt wrote:
> aaron.ballman wrote:
> > This will miss the diagnostic if the current declaration is a namespace 
> > instead of at file scope, right?
> Right. But `getCurLocalScopeDecl()` returns function scope at most. See Note 
> in a code comment above.
Ah, exactly right, thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153914

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Richard Dzenis via Phabricator via cfe-commits]

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 `




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 LSI->CallOperator;
+  else if (const CapturedRegionScopeInfo *CSI = getCurCapturedRegion())
+return CSI->TheCapturedDecl;

aaron.ballman wrote:
> Made the function `const` because it's not writing to any fields, removed 
> `else` because of preceding `return`.
I would love to, and I tried. But unfortunately it's not possible without 
introducing more changes: all the member functions (except 
`getCurFunctionOrMethodDecl()`) are non-const.
Removed `else`.



Comment at: clang/lib/Sema/SemaExpr.cpp:1998-2000
+if (isa(currentDecl)) {
+  Diag(Tok.getLocation(), diag::ext_predef_outside_function);
+}

aaron.ballman wrote:
> This will miss the diagnostic if the current declaration is a namespace 
> instead of at file scope, right?
Right. But `getCurLocalScopeDecl()` returns function scope at most. See Note in 
a code comment above.



Comment at: clang/lib/Sema/SemaExpr.cpp:2001-2004
+OS << '"'
+   << Lexer::Stringify(PredefinedExpr::ComputeName(
+  getPredefinedExprKind(Tok.getKind()), currentDecl))
+   << '"';

tahonermann wrote:
> RIscRIpt wrote:
> > tahonermann wrote:
> > > A diagnostic is issued if the call to `getCurScopeDecl()` returned a 
> > > translation unit declaration (at least in Microsoft mode). Does it make 
> > > sense to pass a pointer to a `TranslationUnitDecl` here?
> > Shortly, yes, kind of. `ComputeName` can handle `TranslationUnitDecl` in 
> > which case it returns an empty string. This way we implicitly (without 
> > additional code) create empty string-literal Tokens which can be handled in 
> > `StringLiteralParser`. And we cannot pass non-string-literal tokens into 
> > `StringLiteralParser`.
> Ah, ok. And I see it even differentiates what name is produced for 
> `__PRETTY_FUNCTION__`. That leaves me wondering what the right behavior 
> should be at class and namespace scope, but maybe I'm better off not asking 
> questions I don't want to know the answer to.
> A diagnostic is issued if the call to `getCurScopeDecl()` returned a 
> translation unit declaration (at least in Microsoft mode). Does it make sense 
> to pass a pointer to a `TranslationUnitDecl` here?




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153914

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Richard Dzenis via Phabricator via cfe-commits]

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/

https://reviews.llvm.org/D153914

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Lex/LiteralSupport.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Lex/LiteralSupport.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/ms_predefined_expr.cpp
  clang/test/Sema/ms_wide_predefined_expr.cpp

Index: clang/test/Sema/ms_wide_predefined_expr.cpp
===
--- clang/test/Sema/ms_wide_predefined_expr.cpp
+++ /dev/null
@@ -1,28 +0,0 @@
-// RUN: %clang_cc1 %s -fsyntax-only -Wno-unused-value -Wmicrosoft -verify -fms-extensions
-// expected-no-diagnostics
-
-// Wide character predefined identifiers
-#define _STR2WSTR(str) L##str
-#define STR2WSTR(str) _STR2WSTR(str)
-void abcdefghi12(void) {
- const wchar_t (*ss)[12] = (__FUNCTION__);
- static int arr[sizeof(STR2WSTR(__FUNCTION__))==12*sizeof(wchar_t) ? 1 : -1];
- const wchar_t (*ss2)[31] = (__FUNCSIG__);
- static int arr2[sizeof(STR2WSTR(__FUNCSIG__))==31*sizeof(wchar_t) ? 1 : -1];
-}
-
-namespace PR13206 {
-void foo(const wchar_t *);
-
-template class A {
-public:
- void method() {
-  foo(L__FUNCTION__);
- }
-};
-
-void bar() {
- A x;
- x.method();
-}
-}
Index: clang/test/Sema/ms_predefined_expr.cpp
===
--- clang/test/Sema/ms_predefined_expr.cpp
+++ clang/test/Sema/ms_predefined_expr.cpp
@@ -1,9 +1,170 @@
 // RUN: %clang_cc1 %s -fsyntax-only -Wmicrosoft -verify -fms-extensions
 
-void f() {
+using size_t = __SIZE_TYPE__;
+
+// Test array initialization
+void array_init() {
  const char a[] = __FUNCTION__; // expected-warning{{initializing an array from a '__FUNCTION__' predefined identifier is a Microsoft extension}}
  const char b[] = __FUNCDNAME__; // expected-warning{{initializing an array from a '__FUNCDNAME__' predefined identifier is a Microsoft extension}}
  const char c[] = __FUNCSIG__; // expected-warning{{initializing an array from a '__FUNCSIG__' predefined identifier is a Microsoft extension}}
  const char d[] = __func__; // expected-warning{{initializing an array from a '__func__' predefined identifier is a Microsoft extension}}
  const char e[] = __PRETTY_FUNCTION__; // expected-warning{{initializing an array from a '__PRETTY_FUNCTION__' predefined identifier is a Microsoft extension}}
+ const wchar_t f[] = L__FUNCTION__; // expected-warning{{initializing an array from a 'L__FUNCTION__' predefined identifier is a Microsoft extension}}
+ const wchar_t g[] = L__FUNCSIG__; // expected-warning{{initializing an array from a 'L__FUNCSIG__' predefined identifier is a Microsoft extension}}
+}
+
+// Test function local identifiers outside of a function
+const char* g_function = __FUNCTION__;// expected-warning{{predefined identifier is only valid inside function}}
+const char* g_function_lconcat = "" __FUNCTION__; // expected-warning{{predefined identifier is only valid inside function}} \
+  // expected-warning{{expansion of predefined identifier '__FUNCTION__' to a string literal is a Microsoft extension}}
+const char* g_function_rconcat = __FUNCTION__ ""; // expected-warning{{predefined identifier is only valid inside function}} \
+  // expected-warning{{expansion of predefined identifier '__FUNCTION__' to a string literal is a Microsoft extension}}
+
+namespace NS
+{
+  const char* function = __FUNCTION__;// expected-warning{{predefined identifier is only valid inside function}}
+  const char* function_lconcat = "" __FUNCTION__; // expected-warning{{predefined identifier is only valid inside function}} \
+  // expected-warning{{expansion of predefined identifier '__FUNCTION__' to a string literal is a Microsoft extension}}
+  const char* function_rconcat = __FUNCTION__ ""; // expected-warning{{predefined identifier is only valid inside function}} \
+  // expected-warning{{expansion of predefined identifier '__FUNCTION__' to a string literal is a Microsoft extension}}
+
+  struct S
+  {
+static constexpr const char* function = __FUNCTION__;// expected-warning{{predefined identifier is only valid inside function}}
+static constexpr const char* function_lconcat = "" __FUNCTION__; // expected-warning{{predefined identifier is only valid inside function}} \
+ // expected-warning{{expansion of predefined 

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

[Aaron Ballman via Phabricator via cfe-commits]

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 LSI->CallOperator;
+  else if (const CapturedRegionScopeInfo *CSI = getCurCapturedRegion())
+return CSI->TheCapturedDecl;

Made the function `const` because it's not writing to any fields, removed 
`else` because of preceding `return`.



Comment at: clang/lib/Sema/SemaExpr.cpp:1986
+  // expansion of macros into empty string literals without additional checks.
+  Decl *currentDecl = getCurLocalScopeDecl();
+  if (!currentDecl)

Matching our usual naming conventions.



Comment at: clang/lib/Sema/SemaExpr.cpp:1998-2000
+if (isa(currentDecl)) {
+  Diag(Tok.getLocation(), diag::ext_predef_outside_function);
+}

This will miss the diagnostic if the current declaration is a namespace instead 
of at file scope, right?



Comment at: clang/lib/Sema/SemaExpr.cpp:2034-2036
+  std::vector ExpandedToks;
+  if (getLangOpts().MicrosoftExt)
+StringToks = ExpandedToks = 
ExpandFunctionLocalPredefinedMacros(StringToks);

Probably worth a comment here explaining that `ExpandedToks` acts as the 
backing store for `StringToks`, which is why both are being assigned to here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153914

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Corentin Jabot via Phabricator via cfe-commits]

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 expected from me.

We will commit on your behalf, what name/email do you want us to use? 
Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153914

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Richard Dzenis via Phabricator via cfe-commits]

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 Github Monorepo

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

https://reviews.llvm.org/D153914

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Lex/LiteralSupport.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Lex/LiteralSupport.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/ms_predefined_expr.cpp
  clang/test/Sema/ms_wide_predefined_expr.cpp

Index: clang/test/Sema/ms_wide_predefined_expr.cpp
===
--- clang/test/Sema/ms_wide_predefined_expr.cpp
+++ /dev/null
@@ -1,28 +0,0 @@
-// RUN: %clang_cc1 %s -fsyntax-only -Wno-unused-value -Wmicrosoft -verify -fms-extensions
-// expected-no-diagnostics
-
-// Wide character predefined identifiers
-#define _STR2WSTR(str) L##str
-#define STR2WSTR(str) _STR2WSTR(str)
-void abcdefghi12(void) {
- const wchar_t (*ss)[12] = (__FUNCTION__);
- static int arr[sizeof(STR2WSTR(__FUNCTION__))==12*sizeof(wchar_t) ? 1 : -1];
- const wchar_t (*ss2)[31] = (__FUNCSIG__);
- static int arr2[sizeof(STR2WSTR(__FUNCSIG__))==31*sizeof(wchar_t) ? 1 : -1];
-}
-
-namespace PR13206 {
-void foo(const wchar_t *);
-
-template class A {
-public:
- void method() {
-  foo(L__FUNCTION__);
- }
-};
-
-void bar() {
- A x;
- x.method();
-}
-}
Index: clang/test/Sema/ms_predefined_expr.cpp
===
--- clang/test/Sema/ms_predefined_expr.cpp
+++ clang/test/Sema/ms_predefined_expr.cpp
@@ -1,9 +1,170 @@
 // RUN: %clang_cc1 %s -fsyntax-only -Wmicrosoft -verify -fms-extensions
 
-void f() {
+using size_t = __SIZE_TYPE__;
+
+// Test array initialization
+void array_init() {
  const char a[] = __FUNCTION__; // expected-warning{{initializing an array from a '__FUNCTION__' predefined identifier is a Microsoft extension}}
  const char b[] = __FUNCDNAME__; // expected-warning{{initializing an array from a '__FUNCDNAME__' predefined identifier is a Microsoft extension}}
  const char c[] = __FUNCSIG__; // expected-warning{{initializing an array from a '__FUNCSIG__' predefined identifier is a Microsoft extension}}
  const char d[] = __func__; // expected-warning{{initializing an array from a '__func__' predefined identifier is a Microsoft extension}}
  const char e[] = __PRETTY_FUNCTION__; // expected-warning{{initializing an array from a '__PRETTY_FUNCTION__' predefined identifier is a Microsoft extension}}
+ const wchar_t f[] = L__FUNCTION__; // expected-warning{{initializing an array from a 'L__FUNCTION__' predefined identifier is a Microsoft extension}}
+ const wchar_t g[] = L__FUNCSIG__; // expected-warning{{initializing an array from a 'L__FUNCSIG__' predefined identifier is a Microsoft extension}}
+}
+
+// Test function local identifiers outside of a function
+const char* g_function = __FUNCTION__;// expected-warning{{predefined identifier is only valid inside function}}
+const char* g_function_lconcat = "" __FUNCTION__; // expected-warning{{predefined identifier is only valid inside function}} \
+  // expected-warning{{expansion of predefined identifier '__FUNCTION__' to a string literal is a Microsoft extension}}
+const char* g_function_rconcat = __FUNCTION__ ""; // expected-warning{{predefined identifier is only valid inside function}} \
+  // expected-warning{{expansion of predefined identifier '__FUNCTION__' to a string literal is a Microsoft extension}}
+
+namespace NS
+{
+  const char* function = __FUNCTION__;// expected-warning{{predefined identifier is only valid inside function}}
+  const char* function_lconcat = "" __FUNCTION__; // expected-warning{{predefined identifier is only valid inside function}} \
+  // expected-warning{{expansion of predefined identifier '__FUNCTION__' to a string literal is a Microsoft extension}}
+  const char* function_rconcat = __FUNCTION__ ""; // expected-warning{{predefined identifier is only valid inside function}} \
+  // expected-warning{{expansion of predefined identifier '__FUNCTION__' to a string literal is a Microsoft extension}}
+
+  struct S
+  {
+static constexpr const char* function = __FUNCTION__;// expected-warning{{predefined identifier is only valid inside function}}
+static constexpr const char* function_lconcat = "" __FUNCTION__; // expected-warning{{predefined identifier is only valid inside function}} \
+  

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

[Richard Dzenis via Phabricator via cfe-commits]

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

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Richard Dzenis via Phabricator via cfe-commits]

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/

https://reviews.llvm.org/D153914

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Lex/LiteralSupport.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Lex/LiteralSupport.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/ms_predefined_expr.cpp
  clang/test/Sema/ms_wide_predefined_expr.cpp

Index: clang/test/Sema/ms_wide_predefined_expr.cpp
===
--- clang/test/Sema/ms_wide_predefined_expr.cpp
+++ /dev/null
@@ -1,28 +0,0 @@
-// RUN: %clang_cc1 %s -fsyntax-only -Wno-unused-value -Wmicrosoft -verify -fms-extensions
-// expected-no-diagnostics
-
-// Wide character predefined identifiers
-#define _STR2WSTR(str) L##str
-#define STR2WSTR(str) _STR2WSTR(str)
-void abcdefghi12(void) {
- const wchar_t (*ss)[12] = (__FUNCTION__);
- static int arr[sizeof(STR2WSTR(__FUNCTION__))==12*sizeof(wchar_t) ? 1 : -1];
- const wchar_t (*ss2)[31] = (__FUNCSIG__);
- static int arr2[sizeof(STR2WSTR(__FUNCSIG__))==31*sizeof(wchar_t) ? 1 : -1];
-}
-
-namespace PR13206 {
-void foo(const wchar_t *);
-
-template class A {
-public:
- void method() {
-  foo(L__FUNCTION__);
- }
-};
-
-void bar() {
- A x;
- x.method();
-}
-}
Index: clang/test/Sema/ms_predefined_expr.cpp
===
--- clang/test/Sema/ms_predefined_expr.cpp
+++ clang/test/Sema/ms_predefined_expr.cpp
@@ -1,9 +1,170 @@
 // RUN: %clang_cc1 %s -fsyntax-only -Wmicrosoft -verify -fms-extensions
 
-void f() {
+using size_t = __SIZE_TYPE__;
+
+// Test array initialization
+void array_init() {
  const char a[] = __FUNCTION__; // expected-warning{{initializing an array from a '__FUNCTION__' predefined identifier is a Microsoft extension}}
  const char b[] = __FUNCDNAME__; // expected-warning{{initializing an array from a '__FUNCDNAME__' predefined identifier is a Microsoft extension}}
  const char c[] = __FUNCSIG__; // expected-warning{{initializing an array from a '__FUNCSIG__' predefined identifier is a Microsoft extension}}
  const char d[] = __func__; // expected-warning{{initializing an array from a '__func__' predefined identifier is a Microsoft extension}}
  const char e[] = __PRETTY_FUNCTION__; // expected-warning{{initializing an array from a '__PRETTY_FUNCTION__' predefined identifier is a Microsoft extension}}
+ const wchar_t f[] = L__FUNCTION__; // expected-warning{{initializing an array from a 'L__FUNCTION__' predefined identifier is a Microsoft extension}}
+ const wchar_t g[] = L__FUNCSIG__; // expected-warning{{initializing an array from a 'L__FUNCSIG__' predefined identifier is a Microsoft extension}}
+}
+
+// Test function local identifiers outside of a function
+const char* g_function = __FUNCTION__;// expected-warning{{predefined identifier is only valid inside function}}
+const char* g_function_lconcat = "" __FUNCTION__; // expected-warning{{predefined identifier is only valid inside function}} \
+  // expected-warning{{expansion of predefined identifier '__FUNCTION__' to a string literal is a Microsoft extension}}
+const char* g_function_rconcat = __FUNCTION__ ""; // expected-warning{{predefined identifier is only valid inside function}} \
+  // expected-warning{{expansion of predefined identifier '__FUNCTION__' to a string literal is a Microsoft extension}}
+
+namespace NS
+{
+  const char* function = __FUNCTION__;// expected-warning{{predefined identifier is only valid inside function}}
+  const char* function_lconcat = "" __FUNCTION__; // expected-warning{{predefined identifier is only valid inside function}} \
+  // expected-warning{{expansion of predefined identifier '__FUNCTION__' to a string literal is a Microsoft extension}}
+  const char* function_rconcat = __FUNCTION__ ""; // expected-warning{{predefined identifier is only valid inside function}} \
+  // expected-warning{{expansion of predefined identifier '__FUNCTION__' to a string literal is a Microsoft extension}}
+
+  struct S
+  {
+static constexpr const char* function = __FUNCTION__;// expected-warning{{predefined identifier is only valid inside function}}
+static constexpr const char* function_lconcat = "" __FUNCTION__; // expected-warning{{predefined identifier is only valid inside function}} \
+ // expected-warning{{expansion of predefined 

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

[Tom Honermann via Phabricator via cfe-commits]

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 responsive through all the review iterations!




Comment at: clang/include/clang/Basic/DiagnosticGroups.td:1210
 def MicrosoftInitFromPredefined : DiagGroup<"microsoft-init-from-predefined">;
+def MicrosoftStringLiteralFromPredefined : 
DiagGroup<"microsoft-concat-predefined">;
 

We should probably name the diag group to match.
  - microsoft-concat-predefined -> microsoft-string-literal-from-predefined


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153914

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Richard Dzenis via Phabricator via cfe-commits]

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

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Lex/LiteralSupport.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Lex/LiteralSupport.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/ms_predefined_expr.cpp
  clang/test/Sema/ms_wide_predefined_expr.cpp

Index: clang/test/Sema/ms_wide_predefined_expr.cpp
===
--- clang/test/Sema/ms_wide_predefined_expr.cpp
+++ /dev/null
@@ -1,28 +0,0 @@
-// RUN: %clang_cc1 %s -fsyntax-only -Wno-unused-value -Wmicrosoft -verify -fms-extensions
-// expected-no-diagnostics
-
-// Wide character predefined identifiers
-#define _STR2WSTR(str) L##str
-#define STR2WSTR(str) _STR2WSTR(str)
-void abcdefghi12(void) {
- const wchar_t (*ss)[12] = (__FUNCTION__);
- static int arr[sizeof(STR2WSTR(__FUNCTION__))==12*sizeof(wchar_t) ? 1 : -1];
- const wchar_t (*ss2)[31] = (__FUNCSIG__);
- static int arr2[sizeof(STR2WSTR(__FUNCSIG__))==31*sizeof(wchar_t) ? 1 : -1];
-}
-
-namespace PR13206 {
-void foo(const wchar_t *);
-
-template class A {
-public:
- void method() {
-  foo(L__FUNCTION__);
- }
-};
-
-void bar() {
- A x;
- x.method();
-}
-}
Index: clang/test/Sema/ms_predefined_expr.cpp
===
--- clang/test/Sema/ms_predefined_expr.cpp
+++ clang/test/Sema/ms_predefined_expr.cpp
@@ -1,9 +1,170 @@
 // RUN: %clang_cc1 %s -fsyntax-only -Wmicrosoft -verify -fms-extensions
 
-void f() {
+using size_t = __SIZE_TYPE__;
+
+// Test array initialization
+void array_init() {
  const char a[] = __FUNCTION__; // expected-warning{{initializing an array from a '__FUNCTION__' predefined identifier is a Microsoft extension}}
  const char b[] = __FUNCDNAME__; // expected-warning{{initializing an array from a '__FUNCDNAME__' predefined identifier is a Microsoft extension}}
  const char c[] = __FUNCSIG__; // expected-warning{{initializing an array from a '__FUNCSIG__' predefined identifier is a Microsoft extension}}
  const char d[] = __func__; // expected-warning{{initializing an array from a '__func__' predefined identifier is a Microsoft extension}}
  const char e[] = __PRETTY_FUNCTION__; // expected-warning{{initializing an array from a '__PRETTY_FUNCTION__' predefined identifier is a Microsoft extension}}
+ const wchar_t f[] = L__FUNCTION__; // expected-warning{{initializing an array from a 'L__FUNCTION__' predefined identifier is a Microsoft extension}}
+ const wchar_t g[] = L__FUNCSIG__; // expected-warning{{initializing an array from a 'L__FUNCSIG__' predefined identifier is a Microsoft extension}}
+}
+
+// Test function local identifiers outside of a function
+const char* g_function = __FUNCTION__;// expected-warning{{predefined identifier is only valid inside function}}
+const char* g_function_lconcat = "" __FUNCTION__; // expected-warning{{predefined identifier is only valid inside function}} \
+  // expected-warning{{expansion of predefined identifier '__FUNCTION__' to a string literal is a Microsoft extension}}
+const char* g_function_rconcat = __FUNCTION__ ""; // expected-warning{{predefined identifier is only valid inside function}} \
+  // expected-warning{{expansion of predefined identifier '__FUNCTION__' to a string literal is a Microsoft extension}}
+
+namespace NS
+{
+  const char* function = __FUNCTION__;// expected-warning{{predefined identifier is only valid inside function}}
+  const char* function_lconcat = "" __FUNCTION__; // expected-warning{{predefined identifier is only valid inside function}} \
+  // expected-warning{{expansion of predefined identifier '__FUNCTION__' to a string literal is a Microsoft extension}}
+  const char* function_rconcat = __FUNCTION__ ""; // expected-warning{{predefined identifier is only valid inside function}} \
+  // expected-warning{{expansion of predefined identifier '__FUNCTION__' to a string literal is a Microsoft extension}}
+
+  struct S
+  {
+static constexpr const char* function = __FUNCTION__;// expected-warning{{predefined identifier is only valid inside function}}
+static constexpr const char* function_lconcat = "" __FUNCTION__; // expected-warning{{predefined identifier is only valid inside function}} \
+ // expected-warning{{expansion of predefined identifier 

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

[Tom Honermann via Phabricator via cfe-commits]

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 hand it would be a noop without MS ext.
I suggest leaving it in solely because use of the function does impose some 
overhead in the construction of the `std::vector` that is returned. If that 
overhead can be avoided (I guess by passing the container to populate by 
reference as a separate argument and then return either the original `ArrayRef` 
or a new one referencing the populated container), then this could be made a 
no-op and the checks for `MicrosoftExt` at the call sites could be removed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153914

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Tom Honermann via Phabricator via cfe-commits]

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<
+  "expansion of predefined identifier '%0' to a string literal is a Microsoft 
extension">,
+  InGroup;

For consistency with other names, I'll suggest some alternative names here. I 
think these better reflect the actual diagnostic message.
  - ext_expand_predef_ms -> ext_string_literal_from_predefined
  - MicrosoftConcatPredefined -> MicrosoftStringLiteralFromPredefined



Comment at: clang/include/clang/Basic/TokenKinds.h:22-23
 
+class LangOptions;
+
 namespace tok {

It looks like this change is no longer needed.



Comment at: clang/lib/Basic/TokenKinds.cpp:14-15
 #include "clang/Basic/TokenKinds.h"
+
+#include "clang/Basic/LangOptions.h"
 #include "llvm/Support/ErrorHandling.h"

This change appears to no longer be needed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153914

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Richard Dzenis via Phabricator via cfe-commits]

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 without MS ext.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153914

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Richard Dzenis via Phabricator via cfe-commits]

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
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Lex/LiteralSupport.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Lex/LiteralSupport.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/ms_predefined_expr.cpp
  clang/test/Sema/ms_wide_predefined_expr.cpp

Index: clang/test/Sema/ms_wide_predefined_expr.cpp
===
--- clang/test/Sema/ms_wide_predefined_expr.cpp
+++ /dev/null
@@ -1,28 +0,0 @@
-// RUN: %clang_cc1 %s -fsyntax-only -Wno-unused-value -Wmicrosoft -verify -fms-extensions
-// expected-no-diagnostics
-
-// Wide character predefined identifiers
-#define _STR2WSTR(str) L##str
-#define STR2WSTR(str) _STR2WSTR(str)
-void abcdefghi12(void) {
- const wchar_t (*ss)[12] = (__FUNCTION__);
- static int arr[sizeof(STR2WSTR(__FUNCTION__))==12*sizeof(wchar_t) ? 1 : -1];
- const wchar_t (*ss2)[31] = (__FUNCSIG__);
- static int arr2[sizeof(STR2WSTR(__FUNCSIG__))==31*sizeof(wchar_t) ? 1 : -1];
-}
-
-namespace PR13206 {
-void foo(const wchar_t *);
-
-template class A {
-public:
- void method() {
-  foo(L__FUNCTION__);
- }
-};
-
-void bar() {
- A x;
- x.method();
-}
-}
Index: clang/test/Sema/ms_predefined_expr.cpp
===
--- clang/test/Sema/ms_predefined_expr.cpp
+++ clang/test/Sema/ms_predefined_expr.cpp
@@ -1,9 +1,170 @@
 // RUN: %clang_cc1 %s -fsyntax-only -Wmicrosoft -verify -fms-extensions
 
-void f() {
+using size_t = __SIZE_TYPE__;
+
+// Test array initialization
+void array_init() {
  const char a[] = __FUNCTION__; // expected-warning{{initializing an array from a '__FUNCTION__' predefined identifier is a Microsoft extension}}
  const char b[] = __FUNCDNAME__; // expected-warning{{initializing an array from a '__FUNCDNAME__' predefined identifier is a Microsoft extension}}
  const char c[] = __FUNCSIG__; // expected-warning{{initializing an array from a '__FUNCSIG__' predefined identifier is a Microsoft extension}}
  const char d[] = __func__; // expected-warning{{initializing an array from a '__func__' predefined identifier is a Microsoft extension}}
  const char e[] = __PRETTY_FUNCTION__; // expected-warning{{initializing an array from a '__PRETTY_FUNCTION__' predefined identifier is a Microsoft extension}}
+ const wchar_t f[] = L__FUNCTION__; // expected-warning{{initializing an array from a 'L__FUNCTION__' predefined identifier is a Microsoft extension}}
+ const wchar_t g[] = L__FUNCSIG__; // expected-warning{{initializing an array from a 'L__FUNCSIG__' predefined identifier is a Microsoft extension}}
+}
+
+// Test function local identifiers outside of a function
+const char* g_function = __FUNCTION__;// expected-warning{{predefined identifier is only valid inside function}}
+const char* g_function_lconcat = "" __FUNCTION__; // expected-warning{{predefined identifier is only valid inside function}} \
+  // expected-warning{{expansion of predefined identifier '__FUNCTION__' to a string literal is a Microsoft extension}}
+const char* g_function_rconcat = __FUNCTION__ ""; // expected-warning{{predefined identifier is only valid inside function}} \
+  // expected-warning{{expansion of predefined identifier '__FUNCTION__' to a string literal is a Microsoft extension}}
+
+namespace NS
+{
+  const char* function = __FUNCTION__;// expected-warning{{predefined identifier is only valid inside function}}
+  const char* function_lconcat = "" __FUNCTION__; // expected-warning{{predefined identifier is only valid inside function}} \
+  // expected-warning{{expansion of predefined identifier '__FUNCTION__' to a string literal is a Microsoft extension}}
+  const char* function_rconcat = __FUNCTION__ ""; // expected-warning{{predefined identifier is only valid inside function}} \
+  // expected-warning{{expansion of predefined identifier '__FUNCTION__' to a string literal is a Microsoft extension}}
+
+  struct S
+  {
+static constexpr const char* function = __FUNCTION__;// expected-warning{{predefined identifier is only valid inside function}}
+static constexpr const char* function_lconcat = "" __FUNCTION__; // expected-warning{{predefined identifier is only valid inside function}} \
+ // expected-warning{{expansion of predefined identifier '__FUNCTION__' to a string literal is a Microsoft 

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

[Richard Dzenis via Phabricator via cfe-commits]

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 == tok::kw___FUNCSIG__ ||
+ K == tok::kw_L__FUNCTION__ || K == tok::kw_L__FUNCSIG__ ||
+ K == tok::kw___FUNCDNAME__;
+}

cor3ntin wrote:
> RIscRIpt wrote:
> > tahonermann wrote:
> > > cor3ntin wrote:
> > > > tahonermann wrote:
> > > > > RIscRIpt wrote:
> > > > > > tahonermann wrote:
> > > > > > > 
> > > > > > Thanks, I like the name. But shouldn't we reflect that we are 
> > > > > > referring to only Microsoft (unexpandable) macros? How about 
> > > > > > `isFunctionLocalPredefinedMsMacro`?
> > > > > I don't think the Microsoft association is meaningful. Sure, some of 
> > > > > the predefined macros are only treated differently when compiling in 
> > > > > Microsoft mode, but some of those macros aren't Microsoft specific. 
> > > > > Also, there might be macros provided by other implementations that 
> > > > > we'll want to emulate some day.
> > > > I think it is, there is currently no way to link 
> > > > `isFunctionLocalPredefinedMacro` to the MS feature. "MSPredefinedMacro" 
> > > > is pretty self explanatory, same reason we most often - but not always 
> > > > - use GNU in the name of function related to GNU extensions.
> > > > There are enough similar-sounding features and extensions that we 
> > > > should try to make it clear which feature we are talking about.
> > > Perhaps we still haven't found the right name then. With the name that 
> > > I've been advocating, this function should also return true for 
> > > `tok::kw___PRETTY_FUNCTION__` even though that macro should not expand to 
> > > something that can be concatenated with other string literals (whether 
> > > compiling in Microsoft mode or not).
> > > 
> > > The Microsoft extension is the localized expansion to a string literal 
> > > that can be concatenated with other string literals. We probably should 
> > > isolate the conditions for that behavior to one place and if we do that, 
> > > then I guess naming this function as specific to Microsoft mode is ok; we 
> > > can always rename it later if it gets used for non-Microsoft extensions.
> > > 
> > > Here is my suggestion then:
> > >   // Return true if the token corresponds to a function local predefined
> > >   // macro that, in Microsoft modes, expands to a string literal (that can
> > >   // then be concatenated with other string literals).
> > >   inline bool isMsFunctionLocalStringLiteralMacro(TokenKind K, const 
> > > LangOptions ) {
> > > return langOpts.MicrosoftExt && (
> > > K == tok::kw___FUNCTION__ || K == tok::kw_L__FUNCTION__ ||
> > > K == tok::kw___FUNCSIG__ || K == tok::kw_L__FUNCSIG__ ||
> > > K == tok::kw___FUNCDNAME__);
> > >   }  
> > > 
> > > This will avoid the need for callers to check for `MicrosoftExt`; that is 
> > > what I really want to avoid since it is easy to forget to do that check.
> > 1. By requiring user to pass `LangOptions` I think we can remove MS 
> > association (implying that there could be other non-msft cases in the 
> > future)
> > 2. We would have to include a `LangOptions.h` in `TokenKinds.h`, are we ok 
> > with this? Alternatively while this function is for msft cases only, we 
> > could pass `bool MicrosoftExt`
> > 
> > Personally, I like version with `LangOptions` and removal of `MS`.
> > By requiring user to pass LangOptions I think we can remove MS association
> 
> I don't think there is any motivation to future proof against hypotheticals, 
> we can always refactor later 
> 
> > We would have to include a LangOptions.h in TokenKinds.h
> 
> This makes me uneasy, but i think we can move the function to 
> `LiteralSupport.h` and include that in `ParseExpr.cpp`.
Sounds good to me. Due to other changes I also moved `TokenIsLikeStringLiteral` 
there.

> I don't think there is any motivation to future proof against hypotheticals, 
> we can always refactor later
Yes, not a strong argument, but I'd like to keep current name if you are not 
against.



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

tahonermann wrote:
> cor3ntin wrote:
> > tahonermann wrote:
> > > RIscRIpt wrote:
> > > > tahonermann wrote:
> > > > > "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.
> 

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

[Richard Dzenis via Phabricator via cfe-commits]

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 helper-functions to `Lex/LiteralSupport`
4. Changed back to usage of `Lexer::Stringify` for function name expansion

Local lit clang/test succeed for both Debug and Release builds.


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
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/TokenKinds.h
  clang/include/clang/Lex/LiteralSupport.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Basic/TokenKinds.cpp
  clang/lib/Lex/LiteralSupport.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/ms_predefined_expr.cpp
  clang/test/Sema/ms_wide_predefined_expr.cpp

Index: clang/test/Sema/ms_wide_predefined_expr.cpp
===
--- clang/test/Sema/ms_wide_predefined_expr.cpp
+++ /dev/null
@@ -1,28 +0,0 @@
-// RUN: %clang_cc1 %s -fsyntax-only -Wno-unused-value -Wmicrosoft -verify -fms-extensions
-// expected-no-diagnostics
-
-// Wide character predefined identifiers
-#define _STR2WSTR(str) L##str
-#define STR2WSTR(str) _STR2WSTR(str)
-void abcdefghi12(void) {
- const wchar_t (*ss)[12] = (__FUNCTION__);
- static int arr[sizeof(STR2WSTR(__FUNCTION__))==12*sizeof(wchar_t) ? 1 : -1];
- const wchar_t (*ss2)[31] = (__FUNCSIG__);
- static int arr2[sizeof(STR2WSTR(__FUNCSIG__))==31*sizeof(wchar_t) ? 1 : -1];
-}
-
-namespace PR13206 {
-void foo(const wchar_t *);
-
-template class A {
-public:
- void method() {
-  foo(L__FUNCTION__);
- }
-};
-
-void bar() {
- A x;
- x.method();
-}
-}
Index: clang/test/Sema/ms_predefined_expr.cpp
===
--- clang/test/Sema/ms_predefined_expr.cpp
+++ clang/test/Sema/ms_predefined_expr.cpp
@@ -1,9 +1,170 @@
 // RUN: %clang_cc1 %s -fsyntax-only -Wmicrosoft -verify -fms-extensions
 
-void f() {
+using size_t = __SIZE_TYPE__;
+
+// Test array initialization
+void array_init() {
  const char a[] = __FUNCTION__; // expected-warning{{initializing an array from a '__FUNCTION__' predefined identifier is a Microsoft extension}}
  const char b[] = __FUNCDNAME__; // expected-warning{{initializing an array from a '__FUNCDNAME__' predefined identifier is a Microsoft extension}}
  const char c[] = __FUNCSIG__; // expected-warning{{initializing an array from a '__FUNCSIG__' predefined identifier is a Microsoft extension}}
  const char d[] = __func__; // expected-warning{{initializing an array from a '__func__' predefined identifier is a Microsoft extension}}
  const char e[] = __PRETTY_FUNCTION__; // expected-warning{{initializing an array from a '__PRETTY_FUNCTION__' predefined identifier is a Microsoft extension}}
+ const wchar_t f[] = L__FUNCTION__; // expected-warning{{initializing an array from a 'L__FUNCTION__' predefined identifier is a Microsoft extension}}
+ const wchar_t g[] = L__FUNCSIG__; // expected-warning{{initializing an array from a 'L__FUNCSIG__' predefined identifier is a Microsoft extension}}
+}
+
+// Test function local identifiers outside of a function
+const char* g_function = __FUNCTION__;// expected-warning{{predefined identifier is only valid inside function}}
+const char* g_function_lconcat = "" __FUNCTION__; // expected-warning{{predefined identifier is only valid inside function}} \
+  // expected-warning{{expansion of predefined identifier '__FUNCTION__' to a string literal is a Microsoft extension}}
+const char* g_function_rconcat = __FUNCTION__ ""; // expected-warning{{predefined identifier is only valid inside function}} \
+  // expected-warning{{expansion of predefined identifier '__FUNCTION__' to a string literal is a Microsoft extension}}
+
+namespace NS
+{
+  const char* function = __FUNCTION__;// expected-warning{{predefined identifier is only valid inside function}}
+  const char* function_lconcat = "" __FUNCTION__; // expected-warning{{predefined identifier is only valid inside function}} \
+  // expected-warning{{expansion of predefined identifier '__FUNCTION__' to a string literal is a Microsoft extension}}
+  const char* function_rconcat = __FUNCTION__ ""; // expected-warning{{predefined identifier is only valid inside function}} \
+  // expected-warning{{expansion of predefined identifier '__FUNCTION__' to a string literal is a Microsoft extension}}
+
+  struct S
+  {
+static constexpr 

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

[Tom Honermann via Phabricator via cfe-commits]

tahonermann added inline comments.



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

cor3ntin wrote:
> tahonermann wrote:
> > RIscRIpt wrote:
> > > tahonermann wrote:
> > > > "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.
> > > At first I thought you were hinting me to use non-ascii characters for 
> > > d-char-sequence. However, when I looked closely d-char-sequence is not as 
> > > flexible as r-chars that you referenced: 
> > > http://eel.is/c++draft/lex.string#nt:d-char and 
> > > http://eel.is/c++draft/lex.charset#2
> > > 
> > > Here're my thoughts:
> > > 1. I was afraid that there could be a possibility of a function name 
> > > contain either quote (`"`) or a backslash (`\`) despite it seemed 
> > > impossible.
> > > 2. At first I used `Lexer::Stringify` to make it bullet-proof. I didn't 
> > > like it, neither did you (reviewers).
> > > 3. I replaced `Lexer::Stringify` with raw-string-literal (d-char-sequence 
> > > was chosen as acronym of current function name).
> > > 4. Current `EFLPM` looks weird and cryptic. And we are limited to 
> > > [lex.charset.basic] with exceptions (space and earlier chars are not 
> > > allowed). I think, using a raw-string-literal without d-char-sequence 
> > > would be enough, because problem could be caused only by two consecutive 
> > > characters `)"`, neither of which can appear in a function name.
> > Sorry about that; you are absolutely right that `d-char-sequence` is (much) 
> > more constrained than `r-char-sequence`.
> > 
> > For `__FUNCSIG__`, the generated text can include arbitrary text. For 
> > example, given this declaration:
> >   void f(decltype(sizeof("()"))*)
> > the macro value is:
> >   void __cdecl f(decltype(sizeof ("()")) *)
> > which does contain `)"`.
> > 
> > I think we should do more to avoid any possibility of the resulting string 
> > being unparseable because the resulting diagnostics will be completely 
> > inscrutable. The best way to do that would be to avoid trying to produce a 
> > parseable string literal in the first place. Based on that and given that 
> > `Sema::ActOnStringLiteral()` immediately uses `StringLiteralParser` to 
> > produce an expanded string literal, I think we would be better off moving 
> > this expansion there such that these macros can be expanded to already 
> > cooked string literals that don't need to be parsed at all. This will mean 
> > having to modify the `StringLiteralParser` constructor to accept a `Decl*` 
> > pointer that will be passed `getCurLocalScopeDecl()` in 
> > `Sema::ActOnStringLiteral()`. `StringLiteralParser` can then assert if it 
> > ever encounters one of these function local predefined tokens when it 
> > hasn't been provided a corresponding `Decl*` to use with them.
> I didn't think about that scenario but in that case the original idea to use 
> Lexer::Stringify seems like a cleaner solution to me.
> Adding that kind of logic to `LiteralSupport` creates a lot of coupling for 
> not much gain that I can see.
> 
> 
> In that light, `Lexer::Stringify` seems like the least worse option.
Yeah, that might be the simpler solution. That matches how `__FILE__`, 
`__BASE_FILE__`, and `__FILE_NAME__` are handled in 
`Preprocessor::ExpandBuiltinMacro()`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153914

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added inline comments.



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

tahonermann wrote:
> RIscRIpt wrote:
> > tahonermann wrote:
> > > "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.
> > At first I thought you were hinting me to use non-ascii characters for 
> > d-char-sequence. However, when I looked closely d-char-sequence is not as 
> > flexible as r-chars that you referenced: 
> > http://eel.is/c++draft/lex.string#nt:d-char and 
> > http://eel.is/c++draft/lex.charset#2
> > 
> > Here're my thoughts:
> > 1. I was afraid that there could be a possibility of a function name 
> > contain either quote (`"`) or a backslash (`\`) despite it seemed 
> > impossible.
> > 2. At first I used `Lexer::Stringify` to make it bullet-proof. I didn't 
> > like it, neither did you (reviewers).
> > 3. I replaced `Lexer::Stringify` with raw-string-literal (d-char-sequence 
> > was chosen as acronym of current function name).
> > 4. Current `EFLPM` looks weird and cryptic. And we are limited to 
> > [lex.charset.basic] with exceptions (space and earlier chars are not 
> > allowed). I think, using a raw-string-literal without d-char-sequence would 
> > be enough, because problem could be caused only by two consecutive 
> > characters `)"`, neither of which can appear in a function name.
> Sorry about that; you are absolutely right that `d-char-sequence` is (much) 
> more constrained than `r-char-sequence`.
> 
> For `__FUNCSIG__`, the generated text can include arbitrary text. For 
> example, given this declaration:
>   void f(decltype(sizeof("()"))*)
> the macro value is:
>   void __cdecl f(decltype(sizeof ("()")) *)
> which does contain `)"`.
> 
> I think we should do more to avoid any possibility of the resulting string 
> being unparseable because the resulting diagnostics will be completely 
> inscrutable. The best way to do that would be to avoid trying to produce a 
> parseable string literal in the first place. Based on that and given that 
> `Sema::ActOnStringLiteral()` immediately uses `StringLiteralParser` to 
> produce an expanded string literal, I think we would be better off moving 
> this expansion there such that these macros can be expanded to already cooked 
> string literals that don't need to be parsed at all. This will mean having to 
> modify the `StringLiteralParser` constructor to accept a `Decl*` pointer that 
> will be passed `getCurLocalScopeDecl()` in `Sema::ActOnStringLiteral()`. 
> `StringLiteralParser` can then assert if it ever encounters one of these 
> function local predefined tokens when it hasn't been provided a corresponding 
> `Decl*` to use with them.
I didn't think about that scenario but in that case the original idea to use 
Lexer::Stringify seems like a cleaner solution to me.
Adding that kind of logic to `LiteralSupport` creates a lot of coupling for not 
much gain that I can see.


In that light, `Lexer::Stringify` seems like the least worse option.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153914

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Tom Honermann via Phabricator via cfe-commits]

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}}
 }

Here is another case to test. MSVC accepts this.
  void test_static_assert() {
static_assert(true, __FUNCTION__);
  }


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153914

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Tom Honermann via Phabricator via cfe-commits]

tahonermann added inline comments.



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

RIscRIpt wrote:
> tahonermann wrote:
> > "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.
> At first I thought you were hinting me to use non-ascii characters for 
> d-char-sequence. However, when I looked closely d-char-sequence is not as 
> flexible as r-chars that you referenced: 
> http://eel.is/c++draft/lex.string#nt:d-char and 
> http://eel.is/c++draft/lex.charset#2
> 
> Here're my thoughts:
> 1. I was afraid that there could be a possibility of a function name contain 
> either quote (`"`) or a backslash (`\`) despite it seemed impossible.
> 2. At first I used `Lexer::Stringify` to make it bullet-proof. I didn't like 
> it, neither did you (reviewers).
> 3. I replaced `Lexer::Stringify` with raw-string-literal (d-char-sequence was 
> chosen as acronym of current function name).
> 4. Current `EFLPM` looks weird and cryptic. And we are limited to 
> [lex.charset.basic] with exceptions (space and earlier chars are not 
> allowed). I think, using a raw-string-literal without d-char-sequence would 
> be enough, because problem could be caused only by two consecutive characters 
> `)"`, neither of which can appear in a function name.
Sorry about that; you are absolutely right that `d-char-sequence` is (much) 
more constrained than `r-char-sequence`.

For `__FUNCSIG__`, the generated text can include arbitrary text. For example, 
given this declaration:
  void f(decltype(sizeof("()"))*)
the macro value is:
  void __cdecl f(decltype(sizeof ("()")) *)
which does contain `)"`.

I think we should do more to avoid any possibility of the resulting string 
being unparseable because the resulting diagnostics will be completely 
inscrutable. The best way to do that would be to avoid trying to produce a 
parseable string literal in the first place. Based on that and given that 
`Sema::ActOnStringLiteral()` immediately uses `StringLiteralParser` to produce 
an expanded string literal, I think we would be better off moving this 
expansion there such that these macros can be expanded to already cooked string 
literals that don't need to be parsed at all. This will mean having to modify 
the `StringLiteralParser` constructor to accept a `Decl*` pointer that will be 
passed `getCurLocalScopeDecl()` in `Sema::ActOnStringLiteral()`. 
`StringLiteralParser` can then assert if it ever encounters one of these 
function local predefined tokens when it hasn't been provided a corresponding 
`Decl*` to use with them.



Comment at: clang/test/Sema/ms_predefined_expr.cpp:115
+   
// expected-warning{{string literal concatenation of 
predefined identifier 'L__FUNCSIG__' is a Microsoft extension}}
+}
+

RIscRIpt wrote:
> tahonermann wrote:
> > How about testing some other encoding prefix combinations?
> >   u"" __FUNCTION // Ok; UTF-16 string literal
> >   u"" L__FUNCTION__ // ill-formed.
> > 
> Good idea. I am not sure whether I should specify `-std` in the test command. 
> I'll leave it without, because current default standard is C++17, and I 
> believe it's not going to be decreased.
> 
> And I think there are enough tests of checking values of these macros. So, in 
> tests for encoding I am going to check types.
Thank you for adding those. I think we should check values for a couple of 
cases as well. Consider a function name that contains `` (U+1 LINEAR B 
SYLLABLE B008 A). In UTF-8, that character requires four code units (F0 90 80 
80) while in UTF-16 it requires two (D800 DC00). We'll want to ensure that such 
characters are properly converted between encodings.
  void test_encoding() {
ASSERT_EQ(u"" __FUNCTION__, u"test_encoding");
  }


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153914

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Corentin Jabot via Phabricator via cfe-commits]

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 == tok::kw___FUNCSIG__ ||
+ K == tok::kw_L__FUNCTION__ || K == tok::kw_L__FUNCSIG__ ||
+ K == tok::kw___FUNCDNAME__;
+}

RIscRIpt wrote:
> tahonermann wrote:
> > cor3ntin wrote:
> > > tahonermann wrote:
> > > > RIscRIpt wrote:
> > > > > tahonermann wrote:
> > > > > > 
> > > > > Thanks, I like the name. But shouldn't we reflect that we are 
> > > > > referring to only Microsoft (unexpandable) macros? How about 
> > > > > `isFunctionLocalPredefinedMsMacro`?
> > > > I don't think the Microsoft association is meaningful. Sure, some of 
> > > > the predefined macros are only treated differently when compiling in 
> > > > Microsoft mode, but some of those macros aren't Microsoft specific. 
> > > > Also, there might be macros provided by other implementations that 
> > > > we'll want to emulate some day.
> > > I think it is, there is currently no way to link 
> > > `isFunctionLocalPredefinedMacro` to the MS feature. "MSPredefinedMacro" 
> > > is pretty self explanatory, same reason we most often - but not always - 
> > > use GNU in the name of function related to GNU extensions.
> > > There are enough similar-sounding features and extensions that we should 
> > > try to make it clear which feature we are talking about.
> > Perhaps we still haven't found the right name then. With the name that I've 
> > been advocating, this function should also return true for 
> > `tok::kw___PRETTY_FUNCTION__` even though that macro should not expand to 
> > something that can be concatenated with other string literals (whether 
> > compiling in Microsoft mode or not).
> > 
> > The Microsoft extension is the localized expansion to a string literal that 
> > can be concatenated with other string literals. We probably should isolate 
> > the conditions for that behavior to one place and if we do that, then I 
> > guess naming this function as specific to Microsoft mode is ok; we can 
> > always rename it later if it gets used for non-Microsoft extensions.
> > 
> > Here is my suggestion then:
> >   // Return true if the token corresponds to a function local predefined
> >   // macro that, in Microsoft modes, expands to a string literal (that can
> >   // then be concatenated with other string literals).
> >   inline bool isMsFunctionLocalStringLiteralMacro(TokenKind K, const 
> > LangOptions ) {
> > return langOpts.MicrosoftExt && (
> > K == tok::kw___FUNCTION__ || K == tok::kw_L__FUNCTION__ ||
> > K == tok::kw___FUNCSIG__ || K == tok::kw_L__FUNCSIG__ ||
> > K == tok::kw___FUNCDNAME__);
> >   }  
> > 
> > This will avoid the need for callers to check for `MicrosoftExt`; that is 
> > what I really want to avoid since it is easy to forget to do that check.
> 1. By requiring user to pass `LangOptions` I think we can remove MS 
> association (implying that there could be other non-msft cases in the future)
> 2. We would have to include a `LangOptions.h` in `TokenKinds.h`, are we ok 
> with this? Alternatively while this function is for msft cases only, we could 
> pass `bool MicrosoftExt`
> 
> Personally, I like version with `LangOptions` and removal of `MS`.
> By requiring user to pass LangOptions I think we can remove MS association

I don't think there is any motivation to future proof against hypotheticals, we 
can always refactor later 

> We would have to include a LangOptions.h in TokenKinds.h

This makes me uneasy, but i think we can move the function to 
`LiteralSupport.h` and include that in `ParseExpr.cpp`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153914

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Richard Dzenis via Phabricator via cfe-commits]

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 token for MSVC, however it becomes 
valid if used via macro concatenation (see `#define WIDE`).
I think addition of support of new macros as well as bug fix (if possible) 
should be submitted separately. I'll open-up an issue, and submit a PR later.




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

tahonermann wrote:
> "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.
At first I thought you were hinting me to use non-ascii characters for 
d-char-sequence. However, when I looked closely d-char-sequence is not as 
flexible as r-chars that you referenced: 
http://eel.is/c++draft/lex.string#nt:d-char and 
http://eel.is/c++draft/lex.charset#2

Here're my thoughts:
1. I was afraid that there could be a possibility of a function name contain 
either quote (`"`) or a backslash (`\`) despite it seemed impossible.
2. At first I used `Lexer::Stringify` to make it bullet-proof. I didn't like 
it, neither did you (reviewers).
3. I replaced `Lexer::Stringify` with raw-string-literal (d-char-sequence was 
chosen as acronym of current function name).
4. Current `EFLPM` looks weird and cryptic. And we are limited to 
[lex.charset.basic] with exceptions (space and earlier chars are not allowed). 
I think, using a raw-string-literal without d-char-sequence would be enough, 
because problem could be caused only by two consecutive characters `)"`, 
neither of which can appear in a function name.



Comment at: clang/test/Sema/ms_predefined_expr.cpp:115
+   
// expected-warning{{string literal concatenation of 
predefined identifier 'L__FUNCSIG__' is a Microsoft extension}}
+}
+

tahonermann wrote:
> How about testing some other encoding prefix combinations?
>   u"" __FUNCTION // Ok; UTF-16 string literal
>   u"" L__FUNCTION__ // ill-formed.
> 
Good idea. I am not sure whether I should specify `-std` in the test command. 
I'll leave it without, because current default standard is C++17, and I believe 
it's not going to be decreased.

And I think there are enough tests of checking values of these macros. So, in 
tests for encoding I am going to check types.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153914

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Richard Dzenis via Phabricator via cfe-commits]

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/

https://reviews.llvm.org/D153914

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/TokenKinds.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Basic/TokenKinds.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/ms_predefined_expr.cpp
  clang/test/Sema/ms_wide_predefined_expr.cpp

Index: clang/test/Sema/ms_wide_predefined_expr.cpp
===
--- clang/test/Sema/ms_wide_predefined_expr.cpp
+++ /dev/null
@@ -1,28 +0,0 @@
-// RUN: %clang_cc1 %s -fsyntax-only -Wno-unused-value -Wmicrosoft -verify -fms-extensions
-// expected-no-diagnostics
-
-// Wide character predefined identifiers
-#define _STR2WSTR(str) L##str
-#define STR2WSTR(str) _STR2WSTR(str)
-void abcdefghi12(void) {
- const wchar_t (*ss)[12] = (__FUNCTION__);
- static int arr[sizeof(STR2WSTR(__FUNCTION__))==12*sizeof(wchar_t) ? 1 : -1];
- const wchar_t (*ss2)[31] = (__FUNCSIG__);
- static int arr2[sizeof(STR2WSTR(__FUNCSIG__))==31*sizeof(wchar_t) ? 1 : -1];
-}
-
-namespace PR13206 {
-void foo(const wchar_t *);
-
-template class A {
-public:
- void method() {
-  foo(L__FUNCTION__);
- }
-};
-
-void bar() {
- A x;
- x.method();
-}
-}
Index: clang/test/Sema/ms_predefined_expr.cpp
===
--- clang/test/Sema/ms_predefined_expr.cpp
+++ clang/test/Sema/ms_predefined_expr.cpp
@@ -1,9 +1,156 @@
 // RUN: %clang_cc1 %s -fsyntax-only -Wmicrosoft -verify -fms-extensions
 
-void f() {
+using size_t = __SIZE_TYPE__;
+
+// Test array initialization
+void array_init() {
  const char a[] = __FUNCTION__; // expected-warning{{initializing an array from a '__FUNCTION__' predefined identifier is a Microsoft extension}}
  const char b[] = __FUNCDNAME__; // expected-warning{{initializing an array from a '__FUNCDNAME__' predefined identifier is a Microsoft extension}}
  const char c[] = __FUNCSIG__; // expected-warning{{initializing an array from a '__FUNCSIG__' predefined identifier is a Microsoft extension}}
  const char d[] = __func__; // expected-warning{{initializing an array from a '__func__' predefined identifier is a Microsoft extension}}
  const char e[] = __PRETTY_FUNCTION__; // expected-warning{{initializing an array from a '__PRETTY_FUNCTION__' predefined identifier is a Microsoft extension}}
+ const wchar_t f[] = L__FUNCTION__; // expected-warning{{initializing an array from a 'L__FUNCTION__' predefined identifier is a Microsoft extension}}
+ const wchar_t g[] = L__FUNCSIG__; // expected-warning{{initializing an array from a 'L__FUNCSIG__' predefined identifier is a Microsoft extension}}
+}
+
+// Test function local identifiers outside of a function
+const char* g_function = __FUNCTION__;// expected-warning{{predefined identifier is only valid inside function}}
+const char* g_function_lconcat = "" __FUNCTION__; // expected-warning{{predefined identifier is only valid inside function}} \
+  // expected-warning{{string literal concatenation of predefined identifier '__FUNCTION__' is a Microsoft extension}}
+const char* g_function_rconcat = __FUNCTION__ ""; // expected-warning{{predefined identifier is only valid inside function}} \
+  // expected-warning{{string literal concatenation of predefined identifier '__FUNCTION__' is a Microsoft extension}}
+
+namespace NS
+{
+  const char* function = __FUNCTION__;// expected-warning{{predefined identifier is only valid inside function}}
+  const char* function_lconcat = "" __FUNCTION__; // expected-warning{{predefined identifier is only valid inside function}} \
+  // expected-warning{{string literal concatenation of predefined identifier '__FUNCTION__' is a Microsoft extension}}
+  const char* function_rconcat = __FUNCTION__ ""; // expected-warning{{predefined identifier is only valid inside function}} \
+  // expected-warning{{string literal concatenation of predefined identifier '__FUNCTION__' is a Microsoft extension}}
+
+  struct S
+  {
+static constexpr const char* function = __FUNCTION__;// expected-warning{{predefined identifier is only valid inside function}}
+static constexpr const char* function_lconcat = "" __FUNCTION__; // expected-warning{{predefined identifier is only valid inside function}} \
+ // expected-warning{{string literal concatenation of predefined identifier '__FUNCTION__' is a 

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

[Tom Honermann via Phabricator via cfe-commits]

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

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Richard Dzenis via Phabricator via cfe-commits]

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 = Actions.ActOnPredefinedExpr(Tok.getLocation(), SavedKind);

tahonermann wrote:
> `TokenIsLikeStringLiteral()` checks `MicrosoftExt`, so the check here is 
> redundant. This is an example of why I would like to see the `MicrosoftExt` 
> checking pushed down to `isFunctionLocalPredefinedMsMacro()`; that really 
> seems where that checking belongs to me.
Let's consider this code:
```
if (!TokenIsLikeStringLiteral(NextToken(), getLangOpts())) {
  Res = Actions.ActOnPredefinedExpr(Tok.getLocation(), SavedKind);
  ConsumeToken();
  break;
}
```

The condition can be expanded as follows: `!(isStringLiteral || (MsExt && 
NeededToken))`. Let's consider `MsExt` is `false`, then we have: 
`!isStringLiteral`. So, if next token is StringLiteral we will try to 
concatenate it even without MsExt, which is invalid. Thus this seemingly 
redundant check is needed.



Comment at: clang/lib/Parse/ParseExpr.cpp:3298-3299
+  assert(
+  (StringToks.size() != 1 ||
+   !tok::isFunctionLocalPredefinedMsMacro(StringToks[0].getKind())) &&
+  "single predefined identifiers shall be handled by ActOnPredefinedExpr");

tahonermann wrote:
> Is there a missing check for `MicrosoftExt` here? This is another example of 
> why I would prefer to see those checks pushed down to 
> `isFunctionLocalPredefinedMsMacro()`.
My intention for this `assert` is to ensure that we keep treating single 
predefined identifiers as `PredefinedExpr` (when we don't need to concat them) 
to preserve AST. I've adjusted the check to make it more clear.



Comment at: clang/lib/Parse/ParseExpr.cpp:1300-1307
+if (!getLangOpts().MicrosoftExt || !tok::isUnexpandableMsMacro(SavedKind) 
||
+!tok::isUnexpandableMsMacro(NextToken().getKind()) &&
+!tok::isStringLiteral(NextToken().getKind())) {
+  Res = Actions.ActOnPredefinedExpr(Tok.getLocation(), SavedKind);
+  ConsumeToken();
+  break;
+}

RIscRIpt wrote:
> tahonermann wrote:
> > Since the conditional code is only relevant for some of the tokens here, I 
> > would prefer to see the other token kinds (`kw___func__`, 
> > `kw___PRETTY_FUNCTION__`) handled separately to avoid the conditional 
> > fallthrough. That means duplicating the calls to 
> > `Actions.ActOnPredefinedExpr()` and `ConsumeToken()` between the blocks, 
> > but that bothers me less than having to understand the complicated 
> > condition.
> That's a valid point. And by handling two tokens separately allows 
> simplification of the condition. Adjusted code.
Having `TokenIsLikeStringLiteral` I think we can return to single case. I 
believe 3 values in condition is not difficult.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153914

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Richard Dzenis via Phabricator via cfe-commits]

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/

https://reviews.llvm.org/D153914

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/TokenKinds.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Basic/TokenKinds.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/ms_predefined_expr.cpp
  clang/test/Sema/ms_wide_predefined_expr.cpp

Index: clang/test/Sema/ms_wide_predefined_expr.cpp
===
--- clang/test/Sema/ms_wide_predefined_expr.cpp
+++ /dev/null
@@ -1,28 +0,0 @@
-// RUN: %clang_cc1 %s -fsyntax-only -Wno-unused-value -Wmicrosoft -verify -fms-extensions
-// expected-no-diagnostics
-
-// Wide character predefined identifiers
-#define _STR2WSTR(str) L##str
-#define STR2WSTR(str) _STR2WSTR(str)
-void abcdefghi12(void) {
- const wchar_t (*ss)[12] = (__FUNCTION__);
- static int arr[sizeof(STR2WSTR(__FUNCTION__))==12*sizeof(wchar_t) ? 1 : -1];
- const wchar_t (*ss2)[31] = (__FUNCSIG__);
- static int arr2[sizeof(STR2WSTR(__FUNCSIG__))==31*sizeof(wchar_t) ? 1 : -1];
-}
-
-namespace PR13206 {
-void foo(const wchar_t *);
-
-template class A {
-public:
- void method() {
-  foo(L__FUNCTION__);
- }
-};
-
-void bar() {
- A x;
- x.method();
-}
-}
Index: clang/test/Sema/ms_predefined_expr.cpp
===
--- clang/test/Sema/ms_predefined_expr.cpp
+++ clang/test/Sema/ms_predefined_expr.cpp
@@ -1,9 +1,123 @@
 // RUN: %clang_cc1 %s -fsyntax-only -Wmicrosoft -verify -fms-extensions
 
-void f() {
+using size_t = __SIZE_TYPE__;
+
+// Test array initialization
+void array_init() {
  const char a[] = __FUNCTION__; // expected-warning{{initializing an array from a '__FUNCTION__' predefined identifier is a Microsoft extension}}
  const char b[] = __FUNCDNAME__; // expected-warning{{initializing an array from a '__FUNCDNAME__' predefined identifier is a Microsoft extension}}
  const char c[] = __FUNCSIG__; // expected-warning{{initializing an array from a '__FUNCSIG__' predefined identifier is a Microsoft extension}}
  const char d[] = __func__; // expected-warning{{initializing an array from a '__func__' predefined identifier is a Microsoft extension}}
  const char e[] = __PRETTY_FUNCTION__; // expected-warning{{initializing an array from a '__PRETTY_FUNCTION__' predefined identifier is a Microsoft extension}}
+ const wchar_t f[] = L__FUNCTION__; // expected-warning{{initializing an array from a 'L__FUNCTION__' predefined identifier is a Microsoft extension}}
+ const wchar_t g[] = L__FUNCSIG__; // expected-warning{{initializing an array from a 'L__FUNCSIG__' predefined identifier is a Microsoft extension}}
+}
+
+// Test function local identifiers outside of a function
+const char* g_function = __FUNCTION__;// expected-warning{{predefined identifier is only valid inside function}}
+const char* g_function_lconcat = "" __FUNCTION__; // expected-warning{{predefined identifier is only valid inside function}} \
+  // expected-warning{{string literal concatenation of predefined identifier '__FUNCTION__' is a Microsoft extension}}
+const char* g_function_rconcat = __FUNCTION__ ""; // expected-warning{{predefined identifier is only valid inside function}} \
+  // expected-warning{{string literal concatenation of predefined identifier '__FUNCTION__' is a Microsoft extension}}
+
+namespace NS
+{
+  const char* function = __FUNCTION__;// expected-warning{{predefined identifier is only valid inside function}}
+  const char* function_lconcat = "" __FUNCTION__; // expected-warning{{predefined identifier is only valid inside function}} \
+  // expected-warning{{string literal concatenation of predefined identifier '__FUNCTION__' is a Microsoft extension}}
+  const char* function_rconcat = __FUNCTION__ ""; // expected-warning{{predefined identifier is only valid inside function}} \
+  // expected-warning{{string literal concatenation of predefined identifier '__FUNCTION__' is a Microsoft extension}}
+
+  struct S
+  {
+static constexpr const char* function = __FUNCTION__;// expected-warning{{predefined identifier is only valid inside function}}
+static constexpr const char* function_lconcat = "" __FUNCTION__; // expected-warning{{predefined identifier is only valid inside function}} \
+ // expected-warning{{string literal concatenation of predefined identifier '__FUNCTION__' is a 

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

[Richard Dzenis via Phabricator via cfe-commits]

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 == tok::kw___FUNCSIG__ ||
+ K == tok::kw_L__FUNCTION__ || K == tok::kw_L__FUNCSIG__ ||
+ K == tok::kw___FUNCDNAME__;
+}

tahonermann wrote:
> cor3ntin wrote:
> > tahonermann wrote:
> > > RIscRIpt wrote:
> > > > tahonermann wrote:
> > > > > 
> > > > Thanks, I like the name. But shouldn't we reflect that we are referring 
> > > > to only Microsoft (unexpandable) macros? How about 
> > > > `isFunctionLocalPredefinedMsMacro`?
> > > I don't think the Microsoft association is meaningful. Sure, some of the 
> > > predefined macros are only treated differently when compiling in 
> > > Microsoft mode, but some of those macros aren't Microsoft specific. Also, 
> > > there might be macros provided by other implementations that we'll want 
> > > to emulate some day.
> > I think it is, there is currently no way to link 
> > `isFunctionLocalPredefinedMacro` to the MS feature. "MSPredefinedMacro" is 
> > pretty self explanatory, same reason we most often - but not always - use 
> > GNU in the name of function related to GNU extensions.
> > There are enough similar-sounding features and extensions that we should 
> > try to make it clear which feature we are talking about.
> Perhaps we still haven't found the right name then. With the name that I've 
> been advocating, this function should also return true for 
> `tok::kw___PRETTY_FUNCTION__` even though that macro should not expand to 
> something that can be concatenated with other string literals (whether 
> compiling in Microsoft mode or not).
> 
> The Microsoft extension is the localized expansion to a string literal that 
> can be concatenated with other string literals. We probably should isolate 
> the conditions for that behavior to one place and if we do that, then I guess 
> naming this function as specific to Microsoft mode is ok; we can always 
> rename it later if it gets used for non-Microsoft extensions.
> 
> Here is my suggestion then:
>   // Return true if the token corresponds to a function local predefined
>   // macro that, in Microsoft modes, expands to a string literal (that can
>   // then be concatenated with other string literals).
>   inline bool isMsFunctionLocalStringLiteralMacro(TokenKind K, const 
> LangOptions ) {
> return langOpts.MicrosoftExt && (
> K == tok::kw___FUNCTION__ || K == tok::kw_L__FUNCTION__ ||
> K == tok::kw___FUNCSIG__ || K == tok::kw_L__FUNCSIG__ ||
> K == tok::kw___FUNCDNAME__);
>   }  
> 
> This will avoid the need for callers to check for `MicrosoftExt`; that is 
> what I really want to avoid since it is easy to forget to do that check.
1. By requiring user to pass `LangOptions` I think we can remove MS association 
(implying that there could be other non-msft cases in the future)
2. We would have to include a `LangOptions.h` in `TokenKinds.h`, are we ok with 
this? Alternatively while this function is for msft cases only, we could pass 
`bool MicrosoftExt`

Personally, I like version with `LangOptions` and removal of `MS`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153914

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Tom Honermann via Phabricator via cfe-commits]

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 == tok::kw___FUNCSIG__ ||
+ K == tok::kw_L__FUNCTION__ || K == tok::kw_L__FUNCSIG__ ||
+ K == tok::kw___FUNCDNAME__;
+}

cor3ntin wrote:
> tahonermann wrote:
> > RIscRIpt wrote:
> > > tahonermann wrote:
> > > > 
> > > Thanks, I like the name. But shouldn't we reflect that we are referring 
> > > to only Microsoft (unexpandable) macros? How about 
> > > `isFunctionLocalPredefinedMsMacro`?
> > I don't think the Microsoft association is meaningful. Sure, some of the 
> > predefined macros are only treated differently when compiling in Microsoft 
> > mode, but some of those macros aren't Microsoft specific. Also, there might 
> > be macros provided by other implementations that we'll want to emulate some 
> > day.
> I think it is, there is currently no way to link 
> `isFunctionLocalPredefinedMacro` to the MS feature. "MSPredefinedMacro" is 
> pretty self explanatory, same reason we most often - but not always - use GNU 
> in the name of function related to GNU extensions.
> There are enough similar-sounding features and extensions that we should try 
> to make it clear which feature we are talking about.
Perhaps we still haven't found the right name then. With the name that I've 
been advocating, this function should also return true for 
`tok::kw___PRETTY_FUNCTION__` even though that macro should not expand to 
something that can be concatenated with other string literals (whether 
compiling in Microsoft mode or not).

The Microsoft extension is the localized expansion to a string literal that can 
be concatenated with other string literals. We probably should isolate the 
conditions for that behavior to one place and if we do that, then I guess 
naming this function as specific to Microsoft mode is ok; we can always rename 
it later if it gets used for non-Microsoft extensions.

Here is my suggestion then:
  // Return true if the token corresponds to a function local predefined
  // macro that, in Microsoft modes, expands to a string literal (that can
  // then be concatenated with other string literals).
  inline bool isMsFunctionLocalStringLiteralMacro(TokenKind K, const 
LangOptions ) {
return langOpts.MicrosoftExt && (
K == tok::kw___FUNCTION__ || K == tok::kw_L__FUNCTION__ ||
K == tok::kw___FUNCSIG__ || K == tok::kw_L__FUNCSIG__ ||
K == tok::kw___FUNCDNAME__);
  }  

This will avoid the need for callers to check for `MicrosoftExt`; that is what 
I really want to avoid since it is easy to forget to do that check.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153914

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Corentin Jabot via Phabricator via cfe-commits]

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 == tok::kw___FUNCSIG__ ||
+ K == tok::kw_L__FUNCTION__ || K == tok::kw_L__FUNCSIG__ ||
+ K == tok::kw___FUNCDNAME__;
+}

tahonermann wrote:
> RIscRIpt wrote:
> > tahonermann wrote:
> > > 
> > Thanks, I like the name. But shouldn't we reflect that we are referring to 
> > only Microsoft (unexpandable) macros? How about 
> > `isFunctionLocalPredefinedMsMacro`?
> I don't think the Microsoft association is meaningful. Sure, some of the 
> predefined macros are only treated differently when compiling in Microsoft 
> mode, but some of those macros aren't Microsoft specific. Also, there might 
> be macros provided by other implementations that we'll want to emulate some 
> day.
I think it is, there is currently no way to link 
`isFunctionLocalPredefinedMacro` to the MS feature. "MSPredefinedMacro" is 
pretty self explanatory, same reason we most often - but not always - use GNU 
in the name of function related to GNU extensions.
There are enough similar-sounding features and extensions that we should try to 
make it clear which feature we are talking about.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153914

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Tom Honermann via Phabricator via cfe-commits]

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.
+inline bool isUnexpandableMsMacro(TokenKind K) {
+  return K == tok::kw___FUNCTION__ || K == tok::kw___FUNCSIG__ ||
+ K == tok::kw_L__FUNCTION__ || K == tok::kw_L__FUNCSIG__ ||
+ K == tok::kw___FUNCDNAME__;
+}

RIscRIpt wrote:
> tahonermann wrote:
> > 
> Thanks, I like the name. But shouldn't we reflect that we are referring to 
> only Microsoft (unexpandable) macros? How about 
> `isFunctionLocalPredefinedMsMacro`?
I don't think the Microsoft association is meaningful. Sure, some of the 
predefined macros are only treated differently when compiling in Microsoft 
mode, but some of those macros aren't Microsoft specific. Also, there might be 
macros provided by other implementations that we'll want to emulate some day.



Comment at: clang/include/clang/Sema/Sema.h:5681
 
+  std::vector ExpandUnexpandableMsMacros(ArrayRef Toks);
   ExprResult BuildPredefinedExpr(SourceLocation Loc,

RIscRIpt wrote:
> tahonermann wrote:
> > 
> Renamed to `ExpandFunctionLocalPredefinedMsMacros`. If you think my addition 
> of `Ms` is redundant, let me know.
I don't think the `Ms` is redundant, but I do think it is unnecessary and 
slightly confusing (`__FUNCTION__`) is not a Microsoft macro.



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 = Actions.ActOnPredefinedExpr(Tok.getLocation(), SavedKind);

`TokenIsLikeStringLiteral()` checks `MicrosoftExt`, so the check here is 
redundant. This is an example of why I would like to see the `MicrosoftExt` 
checking pushed down to `isFunctionLocalPredefinedMsMacro()`; that really seems 
where that checking belongs to me.



Comment at: clang/lib/Parse/ParseExpr.cpp:3298-3299
+  assert(
+  (StringToks.size() != 1 ||
+   !tok::isFunctionLocalPredefinedMsMacro(StringToks[0].getKind())) &&
+  "single predefined identifiers shall be handled by ActOnPredefinedExpr");

Is there a missing check for `MicrosoftExt` here? This is another example of 
why I would prefer to see those checks pushed down to 
`isFunctionLocalPredefinedMsMacro()`.



Comment at: clang/lib/Sema/SemaExpr.cpp:1979-1980
+  Decl *currentDecl = getCurLocalScopeDecl();
+  if (!currentDecl)
+currentDecl = Context.getTranslationUnitDecl();
+  std::vector ExpandedToks;

This could use a comment since it isn't obvious why the translation unit 
declaration is used when not in a local scope declaration of some kind.



Comment at: clang/lib/Sema/SemaExpr.cpp:2001-2004
+OS << '"'
+   << Lexer::Stringify(PredefinedExpr::ComputeName(
+  getPredefinedExprKind(Tok.getKind()), currentDecl))
+   << '"';

RIscRIpt wrote:
> tahonermann wrote:
> > A diagnostic is issued if the call to `getCurScopeDecl()` returned a 
> > translation unit declaration (at least in Microsoft mode). Does it make 
> > sense to pass a pointer to a `TranslationUnitDecl` here?
> Shortly, yes, kind of. `ComputeName` can handle `TranslationUnitDecl` in 
> which case it returns an empty string. This way we implicitly (without 
> additional code) create empty string-literal Tokens which can be handled in 
> `StringLiteralParser`. And we cannot pass non-string-literal tokens into 
> `StringLiteralParser`.
Ah, ok. And I see it even differentiates what name is produced for 
`__PRETTY_FUNCTION__`. That leaves me wondering what the right behavior should 
be at class and namespace scope, but maybe I'm better off not asking questions 
I don't want to know the answer to.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153914

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Richard Dzenis via Phabricator via cfe-commits]

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/

https://reviews.llvm.org/D153914

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/TokenKinds.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/ms_predefined_expr.cpp
  clang/test/Sema/ms_wide_predefined_expr.cpp

Index: clang/test/Sema/ms_wide_predefined_expr.cpp
===
--- clang/test/Sema/ms_wide_predefined_expr.cpp
+++ /dev/null
@@ -1,28 +0,0 @@
-// RUN: %clang_cc1 %s -fsyntax-only -Wno-unused-value -Wmicrosoft -verify -fms-extensions
-// expected-no-diagnostics
-
-// Wide character predefined identifiers
-#define _STR2WSTR(str) L##str
-#define STR2WSTR(str) _STR2WSTR(str)
-void abcdefghi12(void) {
- const wchar_t (*ss)[12] = (__FUNCTION__);
- static int arr[sizeof(STR2WSTR(__FUNCTION__))==12*sizeof(wchar_t) ? 1 : -1];
- const wchar_t (*ss2)[31] = (__FUNCSIG__);
- static int arr2[sizeof(STR2WSTR(__FUNCSIG__))==31*sizeof(wchar_t) ? 1 : -1];
-}
-
-namespace PR13206 {
-void foo(const wchar_t *);
-
-template class A {
-public:
- void method() {
-  foo(L__FUNCTION__);
- }
-};
-
-void bar() {
- A x;
- x.method();
-}
-}
Index: clang/test/Sema/ms_predefined_expr.cpp
===
--- clang/test/Sema/ms_predefined_expr.cpp
+++ clang/test/Sema/ms_predefined_expr.cpp
@@ -1,9 +1,123 @@
 // RUN: %clang_cc1 %s -fsyntax-only -Wmicrosoft -verify -fms-extensions
 
-void f() {
+using size_t = __SIZE_TYPE__;
+
+// Test array initialization
+void array_init() {
  const char a[] = __FUNCTION__; // expected-warning{{initializing an array from a '__FUNCTION__' predefined identifier is a Microsoft extension}}
  const char b[] = __FUNCDNAME__; // expected-warning{{initializing an array from a '__FUNCDNAME__' predefined identifier is a Microsoft extension}}
  const char c[] = __FUNCSIG__; // expected-warning{{initializing an array from a '__FUNCSIG__' predefined identifier is a Microsoft extension}}
  const char d[] = __func__; // expected-warning{{initializing an array from a '__func__' predefined identifier is a Microsoft extension}}
  const char e[] = __PRETTY_FUNCTION__; // expected-warning{{initializing an array from a '__PRETTY_FUNCTION__' predefined identifier is a Microsoft extension}}
+ const wchar_t f[] = L__FUNCTION__; // expected-warning{{initializing an array from a 'L__FUNCTION__' predefined identifier is a Microsoft extension}}
+ const wchar_t g[] = L__FUNCSIG__; // expected-warning{{initializing an array from a 'L__FUNCSIG__' predefined identifier is a Microsoft extension}}
+}
+
+// Test function local identifiers outside of a function
+const char* g_function = __FUNCTION__;// expected-warning{{predefined identifier is only valid inside function}}
+const char* g_function_lconcat = "" __FUNCTION__; // expected-warning{{predefined identifier is only valid inside function}} \
+  // expected-warning{{string literal concatenation of predefined identifier '__FUNCTION__' is a Microsoft extension}}
+const char* g_function_rconcat = __FUNCTION__ ""; // expected-warning{{predefined identifier is only valid inside function}} \
+  // expected-warning{{string literal concatenation of predefined identifier '__FUNCTION__' is a Microsoft extension}}
+
+namespace NS
+{
+  const char* function = __FUNCTION__;// expected-warning{{predefined identifier is only valid inside function}}
+  const char* function_lconcat = "" __FUNCTION__; // expected-warning{{predefined identifier is only valid inside function}} \
+  // expected-warning{{string literal concatenation of predefined identifier '__FUNCTION__' is a Microsoft extension}}
+  const char* function_rconcat = __FUNCTION__ ""; // expected-warning{{predefined identifier is only valid inside function}} \
+  // expected-warning{{string literal concatenation of predefined identifier '__FUNCTION__' is a Microsoft extension}}
+
+  struct S
+  {
+static constexpr const char* function = __FUNCTION__;// expected-warning{{predefined identifier is only valid inside function}}
+static constexpr const char* function_lconcat = "" __FUNCTION__; // expected-warning{{predefined identifier is only valid inside function}} \
+ // expected-warning{{string literal concatenation of predefined identifier '__FUNCTION__' is a Microsoft extension}}
+

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

[Richard Dzenis via Phabricator via cfe-commits]

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() == tok::kw_L__FUNCSIG__) {
+OS << 'L';

cor3ntin wrote:
> RIscRIpt wrote:
> > cor3ntin wrote:
> > > I think it might be easier to create a string_literal token directly 
> > > here. I'm also not sure we need to use `Lexer::Stringify`
> > > I think it might be easier to create a string_literal token directly here.
> > 
> > What do you mean? Is there a function which creates Token object from 
> > StringRef? Or is there a way to associate string literal value with a Token 
> > without PP? I would like to simplify it, but I haven't found other ways of 
> > achieving the same result.
> > 
> > > I'm also not sure we need to use Lexer::Stringify
> > 
> > Well, as far as I can see `StringLiteralParser` expands escape sequences. 
> > So, I am just being too careful here.
> > If not using `Lexer::Stringify` do we want to assert that function name 
> > does not contain neither `"` not `\` (which should not happen™)?
> Thanks! I really would get rid of `Stringify` here - I struggle to see how a 
> function could be produced that has characters that cannot appear in an 
> identifier. even asserting isn't very useful.
Replaced `Stringify` with enclosure into raw-string-literal. Despite 
raw-string-literals are available starting from C++11, as far as I can see 
`StringLiteralParser` does not check for the standard.
Let me know, if you believe we cannot abuse this fact, and we should keep using 
`"regular string-literal"` for this purpose.



Comment at: clang/test/Sema/ms_predefined_expr.cpp:9
  const char e[] = __PRETTY_FUNCTION__; // expected-warning{{initializing an 
array from a '__PRETTY_FUNCTION__' predefined identifier is a Microsoft 
extension}}
+ const wchar_t f[] = L__FUNCTION__; // expected-warning{{initializing an array 
from a 'L__FUNCTION__' predefined identifier is a Microsoft extension}}
+ const wchar_t g[] = L__FUNCSIG__; // expected-warning{{initializing an array 
from a 'L__FUNCSIG__' predefined identifier is a Microsoft extension}}

cor3ntin wrote:
> RIscRIpt wrote:
> > cor3ntin wrote:
> > > do we have any tests that look at the values of these things?
> > ```
> > clang/test/Analysis/eval-predefined-exprs.cpp
> > clang/test/AST/Interp/literals.cpp
> > clang/test/SemaCXX/source_location.cpp
> > clang/test/SemaCXX/predefined-expr.cpp
> > ```
> I think it's worth add a few more tests in 
> clang/test/SemaCXX/predefined-expr.cpp checking concatenations
Added tests to check values of these string literals, as well as moved 
`ms_wide_predefined_expr.cpp` tests into `ms_predefined_expr.cpp`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153914

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Corentin Jabot via Phabricator via cfe-commits]

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");

RIscRIpt wrote:
> tahonermann wrote:
> > What is the reason for requiring non-concatenated predefined function local 
> > macros to be handled by `ActOnPredefinedExpr()`?
> To ensure generation of the same AST as before my changes (with 
> `PredefinedExpr`):
> 
> ```
> |   | `-VarDecl  col:13 a 'const char[2]' cinit
> |   |   `-PredefinedExpr  'const char[2]' __FUNCTION__
> |   | `-StringLiteral  'const char[2]' "f"
> ```
> 
> This actually brings a question: do we want to wrap StringLiterals (into 
> PredefinedExpr) which were formed from concatenation of string-literals and 
> some predefined identifier(s)? But it does not really make any sense: 
> concatenated string does not represent any of PredefinedExpr.
Yes, this make sense. I think it's better than adding more complexity / memory 
footprint to `StringLiteral`.



Comment at: clang/lib/Parse/ParseExpr.cpp:3294-3298
 StringToks.push_back(Tok);
-ConsumeStringToken();
-  } while (isTokenStringLiteral());
+if (isTokenStringLiteral())
+  ConsumeStringToken();
+else
+  ConsumeToken();

RIscRIpt wrote:
> cor3ntin wrote:
> > I think I'd prefer `ConsumeAnyToken` with an assert that checks it's a 
> > stringLiteral or a predefined ms exppression
> Done. But do we really need an assertion here? We have one in the function 
> preamble and strict condition in `while`.
Looking at that again, i think you can remove the assert (the one at the top 
should stay). which you did, excellent. sorry about that!



Comment at: clang/lib/Sema/SemaExpr.cpp:1955-1991
+  // MSVC treats some of predefined identifiers (e.g. __FUNCTION__) as
+  // expandable predefined macros defined as string literals,
+  // which may be concatenated. Expand them here (in Sema),
+  // because StringLiteralParser (in Lex) doesn't have access to AST.
+  std::vector ExpandedToks;
+  if (getLangOpts().MicrosoftExt) {
+ExpandedToks = StringToks.vec();

RIscRIpt wrote:
> cor3ntin wrote:
> > Can we put that logic in a separate function?
> Done. Tho, I couldn't make it `const` (for the same reason I couldn't make 
> `getCurScopeDecl() const`). And I wasn't sure about the interface:
> ```
> std::vector ExpandMSPredefinedMacros(ArrayRef Toks);
> ```
> vs
> ```
> void ExpandMSPredefinedMacros(MutableArrayRef Toks);
> ```
I think the later let you use a small vector so it's probably more efficient. 
I'm find with either



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() == tok::kw_L__FUNCSIG__) {
+OS << 'L';

RIscRIpt wrote:
> cor3ntin wrote:
> > I think it might be easier to create a string_literal token directly here. 
> > I'm also not sure we need to use `Lexer::Stringify`
> > I think it might be easier to create a string_literal token directly here.
> 
> What do you mean? Is there a function which creates Token object from 
> StringRef? Or is there a way to associate string literal value with a Token 
> without PP? I would like to simplify it, but I haven't found other ways of 
> achieving the same result.
> 
> > I'm also not sure we need to use Lexer::Stringify
> 
> Well, as far as I can see `StringLiteralParser` expands escape sequences. So, 
> I am just being too careful here.
> If not using `Lexer::Stringify` do we want to assert that function name does 
> not contain neither `"` not `\` (which should not happen™)?
Thanks! I really would get rid of `Stringify` here - I struggle to see how a 
function could be produced that has characters that cannot appear in an 
identifier. even asserting isn't very useful.



Comment at: clang/test/Sema/ms_predefined_expr.cpp:9
  const char e[] = __PRETTY_FUNCTION__; // expected-warning{{initializing an 
array from a '__PRETTY_FUNCTION__' predefined identifier is a Microsoft 
extension}}
+ const wchar_t f[] = L__FUNCTION__; // expected-warning{{initializing an array 
from a 'L__FUNCTION__' predefined identifier is a Microsoft extension}}
+ const wchar_t g[] = L__FUNCSIG__; // expected-warning{{initializing an array 
from a 'L__FUNCSIG__' predefined identifier is a Microsoft extension}}

RIscRIpt wrote:
> cor3ntin wrote:
> > do we have any tests that look at the values of these things?
> ```
> clang/test/Analysis/eval-predefined-exprs.cpp
> clang/test/AST/Interp/literals.cpp
> clang/test/SemaCXX/source_location.cpp
> clang/test/SemaCXX/predefined-expr.cpp
> ```
I think it's worth add a few more tests in 

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

[Richard Dzenis via Phabricator via cfe-commits]

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 == tok::kw___FUNCSIG__ ||
+ K == tok::kw_L__FUNCTION__ || K == tok::kw_L__FUNCSIG__ ||
+ K == tok::kw___FUNCDNAME__;
+}

tahonermann wrote:
> 
Thanks, I like the name. But shouldn't we reflect that we are referring to only 
Microsoft (unexpandable) macros? How about `isFunctionLocalPredefinedMsMacro`?



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

tahonermann wrote:
> RIscRIpt wrote:
> > cor3ntin wrote:
> > > Unless you find a better name,  I think it's preferable to keep 
> > > `isTokenConcatenable` and `isTokenPredefinedMsMacro` as separate 
> > > functions.
> > > Also, this seems like a weird place to check for  
> > > `getLangOpts().MicrosoftExt`
> > Regarding `getLangOpts().MicrosoftExt`. If you are talking about it's 
> > presence in a function which name is meant to be used as a predicate, I 
> > agree. If you are talking about `class Parser`, then there're other places 
> > with references to `getLangOpts()`.
> > 
> > Without such function `ParseStringLiteralExpression` implementation would 
> > be too verbose.
> > Let's decide what we can do after I address other comments.
> > 
> > Meanwhile, I renamed it to `isTokenLikeStringLiteral()`.
> I suggest passing `getLangOpts()` to `isFunctionLocalPredefinedMacro` 
> (`isUnexpandableMsMacro`) and letting it determine whether the token is or is 
> not one of these special not-really-a-string-literal macro things. This will 
> facilitate additional such macros controlled by different options while also 
> colocating the option check with the related tokens.
@tahonermann, in theory it sounds good, but if I understood you correctly, the 
implementation seem weird to me:
```
inline bool isFunctionLocalPredefinedMsMacro(TokenKind K, const LangOptions& 
langOpts) {
  if (!langOpts.MicrosoftExt)
return false;
  return K == tok::kw___FUNCTION__ || K == tok::kw___FUNCSIG__ ||
 K == tok::kw_L__FUNCTION__ || K == tok::kw_L__FUNCSIG__ ||
 K == tok::kw___FUNCDNAME__;
}
```
And I would have to add `#include "LangOptions.h"` in `TokenKinds.h`.



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

RIscRIpt wrote:
> tahonermann wrote:
> > RIscRIpt wrote:
> > > cor3ntin wrote:
> > > > Unless you find a better name,  I think it's preferable to keep 
> > > > `isTokenConcatenable` and `isTokenPredefinedMsMacro` as separate 
> > > > functions.
> > > > Also, this seems like a weird place to check for  
> > > > `getLangOpts().MicrosoftExt`
> > > Regarding `getLangOpts().MicrosoftExt`. If you are talking about it's 
> > > presence in a function which name is meant to be used as a predicate, I 
> > > agree. If you are talking about `class Parser`, then there're other 
> > > places with references to `getLangOpts()`.
> > > 
> > > Without such function `ParseStringLiteralExpression` implementation would 
> > > be too verbose.
> > > Let's decide what we can do after I address other comments.
> > > 
> > > Meanwhile, I renamed it to `isTokenLikeStringLiteral()`.
> > I suggest passing `getLangOpts()` to `isFunctionLocalPredefinedMacro` 
> > (`isUnexpandableMsMacro`) and letting it determine whether the token is or 
> > is not one of these special not-really-a-string-literal macro things. This 
> > will facilitate additional such macros controlled by different options 
> > while also colocating the option check with the related tokens.
> @tahonermann, in theory it sounds good, but if I understood you correctly, 
> the implementation seem weird to me:
> ```
> inline bool isFunctionLocalPredefinedMsMacro(TokenKind K, const LangOptions& 
> langOpts) {
>   if (!langOpts.MicrosoftExt)
> return false;
>   return K == tok::kw___FUNCTION__ || K == tok::kw___FUNCSIG__ ||
>  K == tok::kw_L__FUNCTION__ || K == tok::kw_L__FUNCSIG__ ||
>  K == tok::kw___FUNCDNAME__;
> }
> ```
> And I would have to add `#include "LangOptions.h"` in `TokenKinds.h`.
If you are still concerned about usage of LangOptions in this context, and at 
the same time you are not against of having such function, I can offer the 
following alternatives:
1. Make it a local lambda function in `ParseStringLiteralExpression`
2. Make it a TU local function in `ParseExpr.cpp`

I went with (2) to do simplification of condition in switch statement 

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

[Richard Dzenis via Phabricator via cfe-commits]

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/

https://reviews.llvm.org/D153914

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/TokenKinds.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/ms_predefined_expr.cpp

Index: clang/test/Sema/ms_predefined_expr.cpp
===
--- clang/test/Sema/ms_predefined_expr.cpp
+++ clang/test/Sema/ms_predefined_expr.cpp
@@ -1,9 +1,86 @@
 // RUN: %clang_cc1 %s -fsyntax-only -Wmicrosoft -verify -fms-extensions
 
+using size_t = __SIZE_TYPE__;
+
 void f() {
  const char a[] = __FUNCTION__; // expected-warning{{initializing an array from a '__FUNCTION__' predefined identifier is a Microsoft extension}}
  const char b[] = __FUNCDNAME__; // expected-warning{{initializing an array from a '__FUNCDNAME__' predefined identifier is a Microsoft extension}}
  const char c[] = __FUNCSIG__; // expected-warning{{initializing an array from a '__FUNCSIG__' predefined identifier is a Microsoft extension}}
  const char d[] = __func__; // expected-warning{{initializing an array from a '__func__' predefined identifier is a Microsoft extension}}
  const char e[] = __PRETTY_FUNCTION__; // expected-warning{{initializing an array from a '__PRETTY_FUNCTION__' predefined identifier is a Microsoft extension}}
+ const wchar_t f[] = L__FUNCTION__; // expected-warning{{initializing an array from a 'L__FUNCTION__' predefined identifier is a Microsoft extension}}
+ const wchar_t g[] = L__FUNCSIG__; // expected-warning{{initializing an array from a 'L__FUNCSIG__' predefined identifier is a Microsoft extension}}
+}
+
+const char* g_function = __FUNCTION__;// expected-warning{{predefined identifier is only valid inside function}}
+const char* g_function_lconcat = "" __FUNCTION__; // expected-warning{{predefined identifier is only valid inside function}} \
+  // expected-warning{{string literal concatenation of predefined identifier '__FUNCTION__' is a Microsoft extension}}
+const char* g_function_rconcat = __FUNCTION__ ""; // expected-warning{{predefined identifier is only valid inside function}} \
+  // expected-warning{{string literal concatenation of predefined identifier '__FUNCTION__' is a Microsoft extension}}
+
+namespace NS
+{
+  const char* function = __FUNCTION__;// expected-warning{{predefined identifier is only valid inside function}}
+  const char* function_lconcat = "" __FUNCTION__; // expected-warning{{predefined identifier is only valid inside function}} \
+  // expected-warning{{string literal concatenation of predefined identifier '__FUNCTION__' is a Microsoft extension}}
+  const char* function_rconcat = __FUNCTION__ ""; // expected-warning{{predefined identifier is only valid inside function}} \
+  // expected-warning{{string literal concatenation of predefined identifier '__FUNCTION__' is a Microsoft extension}}
+
+  struct S
+  {
+static constexpr const char* function = __FUNCTION__;// expected-warning{{predefined identifier is only valid inside function}}
+static constexpr const char* function_lconcat = "" __FUNCTION__; // expected-warning{{predefined identifier is only valid inside function}} \
+ // expected-warning{{string literal concatenation of predefined identifier '__FUNCTION__' is a Microsoft extension}}
+static constexpr const char* function_rconcat = __FUNCTION__ ""; // expected-warning{{predefined identifier is only valid inside function}} \
+ // expected-warning{{string literal concatenation of predefined identifier '__FUNCTION__' is a Microsoft extension}}
+  };
+}
+
+// Test concatenation
+
+void eat_const_char_p(const char*);
+void eat_const_wchar_p(const wchar_t*);
+
+void test_concat() {
+  eat_const_char_p("s" __FUNCTION__); // expected-warning{{string literal concatenation of predefined identifier '__FUNCTION__' is a Microsoft extension}}
+  eat_const_char_p("s" __FUNCDNAME__); // expected-warning{{string literal concatenation of predefined identifier '__FUNCDNAME__' is a Microsoft extension}}
+  eat_const_char_p("s" __FUNCSIG__); // expected-warning{{string literal concatenation of predefined identifier '__FUNCSIG__' is a Microsoft extension}}
+
+  eat_const_char_p(__FUNCTION__ "s"); // expected-warning{{string literal concatenation of 

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

[Tom Honermann via Phabricator via cfe-commits]

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 '%0' is a Microsoft extension">,
+  InGroup;

I think it makes since to be more explicit that the concatenation is with 
regard to string literals.



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 == tok::kw___FUNCSIG__ ||
+ K == tok::kw_L__FUNCTION__ || K == tok::kw_L__FUNCSIG__ ||
+ K == tok::kw___FUNCDNAME__;
+}





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

RIscRIpt wrote:
> cor3ntin wrote:
> > Unless you find a better name,  I think it's preferable to keep 
> > `isTokenConcatenable` and `isTokenPredefinedMsMacro` as separate functions.
> > Also, this seems like a weird place to check for  
> > `getLangOpts().MicrosoftExt`
> Regarding `getLangOpts().MicrosoftExt`. If you are talking about it's 
> presence in a function which name is meant to be used as a predicate, I 
> agree. If you are talking about `class Parser`, then there're other places 
> with references to `getLangOpts()`.
> 
> Without such function `ParseStringLiteralExpression` implementation would be 
> too verbose.
> Let's decide what we can do after I address other comments.
> 
> Meanwhile, I renamed it to `isTokenLikeStringLiteral()`.
I suggest passing `getLangOpts()` to `isFunctionLocalPredefinedMacro` 
(`isUnexpandableMsMacro`) and letting it determine whether the token is or is 
not one of these special not-really-a-string-literal macro things. This will 
facilitate additional such macros controlled by different options while also 
colocating the option check with the related tokens.



Comment at: clang/include/clang/Sema/Sema.h:5681
 
+  std::vector ExpandUnexpandableMsMacros(ArrayRef Toks);
   ExprResult BuildPredefinedExpr(SourceLocation Loc,





Comment at: clang/lib/Parse/ParseExpr.cpp:1300-1307
+if (!getLangOpts().MicrosoftExt || !tok::isUnexpandableMsMacro(SavedKind) 
||
+!tok::isUnexpandableMsMacro(NextToken().getKind()) &&
+!tok::isStringLiteral(NextToken().getKind())) {
+  Res = Actions.ActOnPredefinedExpr(Tok.getLocation(), SavedKind);
+  ConsumeToken();
+  break;
+}

Since the conditional code is only relevant for some of the tokens here, I 
would prefer to see the other token kinds (`kw___func__`, 
`kw___PRETTY_FUNCTION__`) handled separately to avoid the conditional 
fallthrough. That means duplicating the calls to 
`Actions.ActOnPredefinedExpr()` and `ConsumeToken()` between the blocks, but 
that bothers me less than having to understand the complicated condition.



Comment at: clang/lib/Parse/ParseExpr.cpp:3277-3280
+  // String concatenation.
+  // Note that keywords like __func__ and __FUNCTION__
+  // are not considered to be strings for concatenation purposes,
+  // unless Microsoft extensions are enabled.

I think `__func__` is not considered a string literal in Microsoft mode, so I 
think this comment needs some adjusting.



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");

What is the reason for requiring non-concatenated predefined function local 
macros to be handled by `ActOnPredefinedExpr()`?



Comment at: clang/lib/Sema/Sema.cpp:1494
 
+Decl *Sema::getCurScopeDecl() {
+  if (const BlockScopeInfo *BSI = getCurBlock())

`getCurScopeDecl` isn't a great name since this doesn't handle class or 
namespace scopes. How about `getCurLocalScopeDecl`? That name isn't quite right 
either since this can return a `TranslationUnitDecl`, but I think it matches 
the intent fairly well.

None of the callers need the actual `TranslationUnitDecl` that is returned. I 
think this function can return `nullptr` is the current scope isn't function 
local and callers can issue the relevant diagnostic in that case (thus avoiding 
the need to check that a translation unit decl was returned).



Comment at: clang/lib/Sema/SemaExpr.cpp:1971-1974
+  // MSVC treats some of predefined identifiers (e.g. __FUNCTION__) as
+  // expandable macros defined as string 

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

[Richard Dzenis via Phabricator via cfe-commits]

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
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/TokenKinds.h
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/ms_predefined_expr.cpp

Index: clang/test/Sema/ms_predefined_expr.cpp
===
--- clang/test/Sema/ms_predefined_expr.cpp
+++ clang/test/Sema/ms_predefined_expr.cpp
@@ -1,9 +1,62 @@
 // RUN: %clang_cc1 %s -fsyntax-only -Wmicrosoft -verify -fms-extensions
 
+using size_t = __SIZE_TYPE__;
+
 void f() {
  const char a[] = __FUNCTION__; // expected-warning{{initializing an array from a '__FUNCTION__' predefined identifier is a Microsoft extension}}
  const char b[] = __FUNCDNAME__; // expected-warning{{initializing an array from a '__FUNCDNAME__' predefined identifier is a Microsoft extension}}
  const char c[] = __FUNCSIG__; // expected-warning{{initializing an array from a '__FUNCSIG__' predefined identifier is a Microsoft extension}}
  const char d[] = __func__; // expected-warning{{initializing an array from a '__func__' predefined identifier is a Microsoft extension}}
  const char e[] = __PRETTY_FUNCTION__; // expected-warning{{initializing an array from a '__PRETTY_FUNCTION__' predefined identifier is a Microsoft extension}}
+ const wchar_t f[] = L__FUNCTION__; // expected-warning{{initializing an array from a 'L__FUNCTION__' predefined identifier is a Microsoft extension}}
+ const wchar_t g[] = L__FUNCSIG__; // expected-warning{{initializing an array from a 'L__FUNCSIG__' predefined identifier is a Microsoft extension}}
+}
+
+// Test concatenation
+
+void eat_const_char_p(const char*);
+void eat_const_wchar_p(const wchar_t*);
+
+void test_concat() {
+  eat_const_char_p("s" __FUNCTION__); // expected-warning{{concatenation of predefined identifier '__FUNCTION__' is a Microsoft extension}}
+  eat_const_char_p("s" __FUNCDNAME__); // expected-warning{{concatenation of predefined identifier '__FUNCDNAME__' is a Microsoft extension}}
+  eat_const_char_p("s" __FUNCSIG__); // expected-warning{{concatenation of predefined identifier '__FUNCSIG__' is a Microsoft extension}}
+
+  eat_const_char_p(__FUNCTION__ "s"); // expected-warning{{concatenation of predefined identifier '__FUNCTION__' is a Microsoft extension}}
+  eat_const_char_p(__FUNCDNAME__ "s"); // expected-warning{{concatenation of predefined identifier '__FUNCDNAME__' is a Microsoft extension}}
+  eat_const_char_p(__FUNCSIG__ "s"); // expected-warning{{concatenation of predefined identifier '__FUNCSIG__' is a Microsoft extension}}
+
+  eat_const_char_p("s" __FUNCTION__ "s"); // expected-warning{{concatenation of predefined identifier '__FUNCTION__' is a Microsoft extension}}
+  eat_const_char_p("s" __FUNCDNAME__ "s"); // expected-warning{{concatenation of predefined identifier '__FUNCDNAME__' is a Microsoft extension}}
+  eat_const_char_p("s" __FUNCSIG__ "s"); // expected-warning{{concatenation of predefined identifier '__FUNCSIG__' is a Microsoft extension}}
+}
+
+void test_wide_concat() {
+  eat_const_wchar_p(L"s" L__FUNCTION__); // expected-warning{{concatenation of predefined identifier 'L__FUNCTION__' is a Microsoft extension}}
+  eat_const_wchar_p(L"s" L__FUNCSIG__); // expected-warning{{concatenation of predefined identifier 'L__FUNCSIG__' is a Microsoft extension}}
+
+  eat_const_wchar_p(L__FUNCTION__ L"s"); // expected-warning{{concatenation of predefined identifier 'L__FUNCTION__' is a Microsoft extension}}
+  eat_const_wchar_p(L__FUNCSIG__ L"s"); // expected-warning{{concatenation of predefined identifier 'L__FUNCSIG__' is a Microsoft extension}}
+
+  eat_const_wchar_p(L"s" L__FUNCTION__ L"s"); // expected-warning{{concatenation of predefined identifier 'L__FUNCTION__' is a Microsoft extension}}
+  eat_const_wchar_p(L"s" L__FUNCSIG__ L"s"); // expected-warning{{concatenation of predefined identifier 'L__FUNCSIG__' is a Microsoft extension}}
+}
+
+const char* test_return() {
+  return __FUNCTION__ "s" __FUNCSIG__; // expected-warning{{concatenation of predefined identifier '__FUNCTION__' is a Microsoft extension}} \
+   // expected-warning{{concatenation of predefined identifier '__FUNCSIG__' is a Microsoft extension}}
+}
+
+void test_struct_init_fn() {
+  struct test_struct_init {
+const char* str;
+  } struct_init = { "struct: " __FUNCSIG__ }; // expected-warning{{concatenation of predefined identifier '__FUNCSIG__' is a Microsoft extension}}
+}
+
+constexpr size_t operator""_len(const char*, size_t len) {
+return len;
+}
+
+void test_udliteral() {
+static_assert(__FUNCTION__ ""_len == 

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

[Richard Dzenis via Phabricator via cfe-commits]

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());
+  }

cor3ntin wrote:
> Unless you find a better name,  I think it's preferable to keep 
> `isTokenConcatenable` and `isTokenPredefinedMsMacro` as separate functions.
> Also, this seems like a weird place to check for  `getLangOpts().MicrosoftExt`
Regarding `getLangOpts().MicrosoftExt`. If you are talking about it's presence 
in a function which name is meant to be used as a predicate, I agree. If you 
are talking about `class Parser`, then there're other places with references to 
`getLangOpts()`.

Without such function `ParseStringLiteralExpression` implementation would be 
too verbose.
Let's decide what we can do after I address other comments.

Meanwhile, I renamed it to `isTokenLikeStringLiteral()`.



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 = ParsePredefinedExpression(true);
 break;

cor3ntin wrote:
> Can we instead, look at `NextToken()` and if next token is a StringLiteral or 
> a MS predefined extension we fallthrough?
> That would avoid having duplicated logic in `ParsePredefinedExpression` and 
> `ActOnStringLiteral` which would be a nice simplification
I thought of that, but I was afraid that playing with fallthrough wasn't 
appreciated.
Thanks, fixed.



Comment at: clang/lib/Parse/ParseExpr.cpp:3294-3298
 StringToks.push_back(Tok);
-ConsumeStringToken();
-  } while (isTokenStringLiteral());
+if (isTokenStringLiteral())
+  ConsumeStringToken();
+else
+  ConsumeToken();

cor3ntin wrote:
> I think I'd prefer `ConsumeAnyToken` with an assert that checks it's a 
> stringLiteral or a predefined ms exppression
Done. But do we really need an assertion here? We have one in the function 
preamble and strict condition in `while`.



Comment at: clang/lib/Sema/SemaExpr.cpp:1955-1991
+  // MSVC treats some of predefined identifiers (e.g. __FUNCTION__) as
+  // expandable predefined macros defined as string literals,
+  // which may be concatenated. Expand them here (in Sema),
+  // because StringLiteralParser (in Lex) doesn't have access to AST.
+  std::vector ExpandedToks;
+  if (getLangOpts().MicrosoftExt) {
+ExpandedToks = StringToks.vec();

cor3ntin wrote:
> Can we put that logic in a separate function?
Done. Tho, I couldn't make it `const` (for the same reason I couldn't make 
`getCurScopeDecl() const`). And I wasn't sure about the interface:
```
std::vector ExpandMSPredefinedMacros(ArrayRef Toks);
```
vs
```
void ExpandMSPredefinedMacros(MutableArrayRef Toks);
```



Comment at: clang/lib/Sema/SemaExpr.cpp:1964
+for (Token  : ExpandedToks) {
+  if (!tok::isMSPredefinedMacro(Tok.getKind())) {
+continue;

cor3ntin wrote:
> can you assert it's a string literal otherwise?
Done.



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() == tok::kw_L__FUNCSIG__) {
+OS << 'L';

cor3ntin wrote:
> I think it might be easier to create a string_literal token directly here. 
> I'm also not sure we need to use `Lexer::Stringify`
> I think it might be easier to create a string_literal token directly here.

What do you mean? Is there a function which creates Token object from 
StringRef? Or is there a way to associate string literal value with a Token 
without PP? I would like to simplify it, but I haven't found other ways of 
achieving the same result.

> I'm also not sure we need to use Lexer::Stringify

Well, as far as I can see `StringLiteralParser` expands escape sequences. So, I 
am just being too careful here.
If not using `Lexer::Stringify` do we want to assert that function name does 
not contain neither `"` not `\` (which should not happen™)?



Comment at: clang/test/Sema/ms_predefined_expr.cpp:9
  const char e[] = __PRETTY_FUNCTION__; // expected-warning{{initializing an 
array from a '__PRETTY_FUNCTION__' predefined identifier is a Microsoft 
extension}}
+ const wchar_t f[] = L__FUNCTION__; // expected-warning{{initializing an array 
from a 'L__FUNCTION__' predefined identifier is a Microsoft extension}}
+ const wchar_t g[] = L__FUNCSIG__; // expected-warning{{initializing an array 
from a 'L__FUNCSIG__' predefined identifier is a Microsoft extension}}

cor3ntin wrote:
> do we have any tests that 

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

[Richard Dzenis via Phabricator via cfe-commits]

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 are concatenated during phase 6...

I agree, but I couldn't come up with anything better (having limited knowledge 
about Clang).
Other possible //ugly// solution: make actual concatenation in 
`StringLiteralParser`, but this would require either of:

1. Pass `Decl*` into `StringLiteralParser` - this requires dependency (and 
linkage) of Lex with AST.
2. Pre-calculate function names for all possible types of tokens 
(`__FUNCTION__`, `__FUNCDNAME__`, etc.) in `ActOnStringLiteral` and pass into 
`StringLiteralParser`
3. Find other way of obtaining __current__ function name in Lex.

One may wish to expand these macros during phase (4), but these macros are 
predefined identifiers in non-MSVC compilers, and MSVC don't expand them as 
macros (`-E` compiler flag); and we don't have knowledge about AST (not to 
mention the function name) during phase (4).

In D153914#4497696 , @aaron.ballman 
wrote:

> I'm not certain it's actually observable.

I didn't understand this part.


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
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/TokenKinds.h
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/ms_predefined_expr.cpp

Index: clang/test/Sema/ms_predefined_expr.cpp
===
--- clang/test/Sema/ms_predefined_expr.cpp
+++ clang/test/Sema/ms_predefined_expr.cpp
@@ -1,9 +1,62 @@
 // RUN: %clang_cc1 %s -fsyntax-only -Wmicrosoft -verify -fms-extensions
 
+using size_t = __SIZE_TYPE__;
+
 void f() {
  const char a[] = __FUNCTION__; // expected-warning{{initializing an array from a '__FUNCTION__' predefined identifier is a Microsoft extension}}
  const char b[] = __FUNCDNAME__; // expected-warning{{initializing an array from a '__FUNCDNAME__' predefined identifier is a Microsoft extension}}
  const char c[] = __FUNCSIG__; // expected-warning{{initializing an array from a '__FUNCSIG__' predefined identifier is a Microsoft extension}}
  const char d[] = __func__; // expected-warning{{initializing an array from a '__func__' predefined identifier is a Microsoft extension}}
  const char e[] = __PRETTY_FUNCTION__; // expected-warning{{initializing an array from a '__PRETTY_FUNCTION__' predefined identifier is a Microsoft extension}}
+ const wchar_t f[] = L__FUNCTION__; // expected-warning{{initializing an array from a 'L__FUNCTION__' predefined identifier is a Microsoft extension}}
+ const wchar_t g[] = L__FUNCSIG__; // expected-warning{{initializing an array from a 'L__FUNCSIG__' predefined identifier is a Microsoft extension}}
+}
+
+// Test concatenation
+
+void eat_const_char_p(const char*);
+void eat_const_wchar_p(const wchar_t*);
+
+void test_concat() {
+  eat_const_char_p("s" __FUNCTION__); // expected-warning{{concatenation of predefined identifier '__FUNCTION__' is a Microsoft extension}}
+  eat_const_char_p("s" __FUNCDNAME__); // expected-warning{{concatenation of predefined identifier '__FUNCDNAME__' is a Microsoft extension}}
+  eat_const_char_p("s" __FUNCSIG__); // expected-warning{{concatenation of predefined identifier '__FUNCSIG__' is a Microsoft extension}}
+
+  eat_const_char_p(__FUNCTION__ "s"); // expected-warning{{concatenation of predefined identifier '__FUNCTION__' is a Microsoft extension}}
+  eat_const_char_p(__FUNCDNAME__ "s"); // expected-warning{{concatenation of predefined identifier '__FUNCDNAME__' is a Microsoft extension}}
+  eat_const_char_p(__FUNCSIG__ "s"); // expected-warning{{concatenation of predefined identifier '__FUNCSIG__' is a Microsoft extension}}
+
+  eat_const_char_p("s" __FUNCTION__ "s"); // expected-warning{{concatenation of predefined identifier '__FUNCTION__' is a Microsoft extension}}
+  eat_const_char_p("s" __FUNCDNAME__ "s"); // expected-warning{{concatenation of predefined identifier '__FUNCDNAME__' is a Microsoft extension}}
+  eat_const_char_p("s" __FUNCSIG__ "s"); // expected-warning{{concatenation of predefined identifier '__FUNCSIG__' is a Microsoft extension}}
+}
+
+void test_wide_concat() {
+  eat_const_wchar_p(L"s" L__FUNCTION__); // expected-warning{{concatenation of predefined identifier 'L__FUNCTION__' is a Microsoft extension}}
+  eat_const_wchar_p(L"s" L__FUNCSIG__); // expected-warning{{concatenation of predefined identifier 'L__FUNCSIG__' is a Microsoft extension}}
+
+  eat_const_wchar_p(L__FUNCTION__ L"s"); // 

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

[Corentin Jabot via Phabricator via cfe-commits]

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 = ParsePredefinedExpression(true);
 break;

Can we instead, look at `NextToken()` and if next token is a StringLiteral or a 
MS predefined extension we fallthrough?
That would avoid having duplicated logic in `ParsePredefinedExpression` and 
`ActOnStringLiteral` which would be a nice simplification


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153914

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Corentin Jabot via Phabricator via cfe-commits]

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());
+  }

Unless you find a better name,  I think it's preferable to keep 
`isTokenConcatenable` and `isTokenPredefinedMsMacro` as separate functions.
Also, this seems like a weird place to check for  `getLangOpts().MicrosoftExt`



Comment at: clang/lib/Parse/ParseExpr.cpp:3294-3298
 StringToks.push_back(Tok);
-ConsumeStringToken();
-  } while (isTokenStringLiteral());
+if (isTokenStringLiteral())
+  ConsumeStringToken();
+else
+  ConsumeToken();

I think I'd prefer `ConsumeAnyToken` with an assert that checks it's a 
stringLiteral or a predefined ms exppression



Comment at: clang/lib/Sema/SemaExpr.cpp:1955-1991
+  // MSVC treats some of predefined identifiers (e.g. __FUNCTION__) as
+  // expandable predefined macros defined as string literals,
+  // which may be concatenated. Expand them here (in Sema),
+  // because StringLiteralParser (in Lex) doesn't have access to AST.
+  std::vector ExpandedToks;
+  if (getLangOpts().MicrosoftExt) {
+ExpandedToks = StringToks.vec();

Can we put that logic in a separate function?



Comment at: clang/lib/Sema/SemaExpr.cpp:1964
+for (Token  : ExpandedToks) {
+  if (!tok::isMSPredefinedMacro(Tok.getKind())) {
+continue;

can you assert it's a string literal otherwise?



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() == tok::kw_L__FUNCSIG__) {
+OS << 'L';

I think it might be easier to create a string_literal token directly here. I'm 
also not sure we need to use `Lexer::Stringify`



Comment at: clang/test/Sema/ms_predefined_expr.cpp:9
  const char e[] = __PRETTY_FUNCTION__; // expected-warning{{initializing an 
array from a '__PRETTY_FUNCTION__' predefined identifier is a Microsoft 
extension}}
+ const wchar_t f[] = L__FUNCTION__; // expected-warning{{initializing an array 
from a 'L__FUNCTION__' predefined identifier is a Microsoft extension}}
+ const wchar_t g[] = L__FUNCSIG__; // expected-warning{{initializing an array 
from a 'L__FUNCSIG__' predefined identifier is a Microsoft extension}}

do we have any tests that look at the values of these things?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153914

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Aaron Ballman via Phabricator via cfe-commits]

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, I'm not certain it's 
actually observable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153914

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Richard Dzenis via Phabricator via cfe-commits]

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/

https://reviews.llvm.org/D153914

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Richard Dzenis via Phabricator via cfe-commits]

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
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/TokenKinds.h
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/ms_predefined_expr.cpp

Index: clang/test/Sema/ms_predefined_expr.cpp
===
--- clang/test/Sema/ms_predefined_expr.cpp
+++ clang/test/Sema/ms_predefined_expr.cpp
@@ -6,4 +6,47 @@
  const char c[] = __FUNCSIG__; // expected-warning{{initializing an array from a '__FUNCSIG__' predefined identifier is a Microsoft extension}}
  const char d[] = __func__; // expected-warning{{initializing an array from a '__func__' predefined identifier is a Microsoft extension}}
  const char e[] = __PRETTY_FUNCTION__; // expected-warning{{initializing an array from a '__PRETTY_FUNCTION__' predefined identifier is a Microsoft extension}}
+ const wchar_t f[] = L__FUNCTION__; // expected-warning{{initializing an array from a 'L__FUNCTION__' predefined identifier is a Microsoft extension}}
+ const wchar_t g[] = L__FUNCSIG__; // expected-warning{{initializing an array from a 'L__FUNCSIG__' predefined identifier is a Microsoft extension}}
+}
+
+// Test concatenation
+
+void eat_const_char_p(const char*);
+void eat_const_wchar_p(const wchar_t*);
+
+void test_concat() {
+  eat_const_char_p("s" __FUNCTION__); // expected-warning{{concatenation of predefined identifier '__FUNCTION__' is a Microsoft extension}}
+  eat_const_char_p("s" __FUNCDNAME__); // expected-warning{{concatenation of predefined identifier '__FUNCDNAME__' is a Microsoft extension}}
+  eat_const_char_p("s" __FUNCSIG__); // expected-warning{{concatenation of predefined identifier '__FUNCSIG__' is a Microsoft extension}}
+
+  eat_const_char_p(__FUNCTION__ "s"); // expected-warning{{concatenation of predefined identifier '__FUNCTION__' is a Microsoft extension}}
+  eat_const_char_p(__FUNCDNAME__ "s"); // expected-warning{{concatenation of predefined identifier '__FUNCDNAME__' is a Microsoft extension}}
+  eat_const_char_p(__FUNCSIG__ "s"); // expected-warning{{concatenation of predefined identifier '__FUNCSIG__' is a Microsoft extension}}
+
+  eat_const_char_p("s" __FUNCTION__ "s"); // expected-warning{{concatenation of predefined identifier '__FUNCTION__' is a Microsoft extension}}
+  eat_const_char_p("s" __FUNCDNAME__ "s"); // expected-warning{{concatenation of predefined identifier '__FUNCDNAME__' is a Microsoft extension}}
+  eat_const_char_p("s" __FUNCSIG__ "s"); // expected-warning{{concatenation of predefined identifier '__FUNCSIG__' is a Microsoft extension}}
+}
+
+void test_wide_concat() {
+  eat_const_wchar_p(L"s" L__FUNCTION__); // expected-warning{{concatenation of predefined identifier 'L__FUNCTION__' is a Microsoft extension}}
+  eat_const_wchar_p(L"s" L__FUNCSIG__); // expected-warning{{concatenation of predefined identifier 'L__FUNCSIG__' is a Microsoft extension}}
+
+  eat_const_wchar_p(L__FUNCTION__ L"s"); // expected-warning{{concatenation of predefined identifier 'L__FUNCTION__' is a Microsoft extension}}
+  eat_const_wchar_p(L__FUNCSIG__ L"s"); // expected-warning{{concatenation of predefined identifier 'L__FUNCSIG__' is a Microsoft extension}}
+
+  eat_const_wchar_p(L"s" L__FUNCTION__ L"s"); // expected-warning{{concatenation of predefined identifier 'L__FUNCTION__' is a Microsoft extension}}
+  eat_const_wchar_p(L"s" L__FUNCSIG__ L"s"); // expected-warning{{concatenation of predefined identifier 'L__FUNCSIG__' is a Microsoft extension}}
+}
+
+const char* test_return() {
+  return __FUNCTION__ "s" __FUNCSIG__; // expected-warning{{concatenation of predefined identifier '__FUNCTION__' is a Microsoft extension}} \
+   // expected-warning{{concatenation of predefined identifier '__FUNCSIG__' is a Microsoft extension}}
+}
+
+void test_struct_init_fn() {
+  struct test_struct_init {
+const char* str;
+  } struct_init = { "struct: " __FUNCSIG__ }; // expected-warning{{concatenation of predefined identifier '__FUNCSIG__' is a Microsoft extension}}
 }
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -1882,6 +1882,27 @@
   ContainsUnexpandedParameterPack, ResultIndex);
 }
 
+static PredefinedExpr::IdentKind getPredefinedExprKind(tok::TokenKind Kind) {
+  switch (Kind) {
+  default:
+llvm_unreachable("unexpected TokenKind");
+  case tok::kw___func__:
+return PredefinedExpr::Func; // [C99 6.4.2.2]
+  case tok::kw___FUNCTION__:
+return PredefinedExpr::Function;
+  case 

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

[Richard Dzenis via Phabricator via cfe-commits]

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 mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Richard Dzenis via Phabricator via cfe-commits]

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:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/TokenKinds.h
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/ms_predefined_expr.cpp

Index: clang/test/Sema/ms_predefined_expr.cpp
===
--- clang/test/Sema/ms_predefined_expr.cpp
+++ clang/test/Sema/ms_predefined_expr.cpp
@@ -6,4 +6,47 @@
  const char c[] = __FUNCSIG__; // expected-warning{{initializing an array from a '__FUNCSIG__' predefined identifier is a Microsoft extension}}
  const char d[] = __func__; // expected-warning{{initializing an array from a '__func__' predefined identifier is a Microsoft extension}}
  const char e[] = __PRETTY_FUNCTION__; // expected-warning{{initializing an array from a '__PRETTY_FUNCTION__' predefined identifier is a Microsoft extension}}
+ const wchar_t f[] = L__FUNCTION__; // expected-warning{{initializing an array from a 'L__FUNCTION__' predefined identifier is a Microsoft extension}}
+ const wchar_t g[] = L__FUNCSIG__; // expected-warning{{initializing an array from a 'L__FUNCSIG__' predefined identifier is a Microsoft extension}}
+}
+
+// Test concatenation
+
+void eat_const_char_p(const char*);
+void eat_const_wchar_p(const wchar_t*);
+
+void test_concat() {
+  eat_const_char_p("s" __FUNCTION__); // expected-warning{{concatenation of predefined identifier '__FUNCTION__' is a Microsoft extension}}
+  eat_const_char_p("s" __FUNCDNAME__); // expected-warning{{concatenation of predefined identifier '__FUNCDNAME__' is a Microsoft extension}}
+  eat_const_char_p("s" __FUNCSIG__); // expected-warning{{concatenation of predefined identifier '__FUNCSIG__' is a Microsoft extension}}
+
+  eat_const_char_p(__FUNCTION__ "s"); // expected-warning{{concatenation of predefined identifier '__FUNCTION__' is a Microsoft extension}}
+  eat_const_char_p(__FUNCDNAME__ "s"); // expected-warning{{concatenation of predefined identifier '__FUNCDNAME__' is a Microsoft extension}}
+  eat_const_char_p(__FUNCSIG__ "s"); // expected-warning{{concatenation of predefined identifier '__FUNCSIG__' is a Microsoft extension}}
+
+  eat_const_char_p("s" __FUNCTION__ "s"); // expected-warning{{concatenation of predefined identifier '__FUNCTION__' is a Microsoft extension}}
+  eat_const_char_p("s" __FUNCDNAME__ "s"); // expected-warning{{concatenation of predefined identifier '__FUNCDNAME__' is a Microsoft extension}}
+  eat_const_char_p("s" __FUNCSIG__ "s"); // expected-warning{{concatenation of predefined identifier '__FUNCSIG__' is a Microsoft extension}}
+}
+
+void test_wide_concat() {
+  eat_const_wchar_p(L"s" L__FUNCTION__); // expected-warning{{concatenation of predefined identifier 'L__FUNCTION__' is a Microsoft extension}}
+  eat_const_wchar_p(L"s" L__FUNCSIG__); // expected-warning{{concatenation of predefined identifier 'L__FUNCSIG__' is a Microsoft extension}}
+
+  eat_const_wchar_p(L__FUNCTION__ L"s"); // expected-warning{{concatenation of predefined identifier 'L__FUNCTION__' is a Microsoft extension}}
+  eat_const_wchar_p(L__FUNCSIG__ L"s"); // expected-warning{{concatenation of predefined identifier 'L__FUNCSIG__' is a Microsoft extension}}
+
+  eat_const_wchar_p(L"s" L__FUNCTION__ L"s"); // expected-warning{{concatenation of predefined identifier 'L__FUNCTION__' is a Microsoft extension}}
+  eat_const_wchar_p(L"s" L__FUNCSIG__ L"s"); // expected-warning{{concatenation of predefined identifier 'L__FUNCSIG__' is a Microsoft extension}}
+}
+
+const char* test_return() {
+  return __FUNCTION__ "s" __FUNCSIG__; // expected-warning{{concatenation of predefined identifier '__FUNCTION__' is a Microsoft extension}} \
+   // expected-warning{{concatenation of predefined identifier '__FUNCSIG__' is a Microsoft extension}}
+}
+
+void test_struct_init_fn() {
+  struct test_struct_init {
+const char* str;
+  } struct_init = { "struct: " __FUNCSIG__ }; // expected-warning{{concatenation of predefined identifier '__FUNCSIG__' is a Microsoft extension}}
 }
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -1882,6 +1882,27 @@
   ContainsUnexpandedParameterPack, ResultIndex);
 }
 
+static PredefinedExpr::IdentKind getPredefinedExprKind(tok::TokenKind Kind) {
+  switch (Kind) {
+  default:
+llvm_unreachable("unexpected TokenKind");
+  case tok::kw___func__:
+return PredefinedExpr::Func; // [C99 6.4.2.2]
+  case tok::kw___FUNCTION__:
+return PredefinedExpr::Function;
+