[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D153536#4577187 , @vitalybuka 
wrote:

> In D153536#4576683 , @aaron.ballman 
> wrote:
>
>> In D153536#4571435 , @vitalybuka 
>> wrote:
>>
 Thanks! We might need someone from clangd to help here. The logs do not 
 have any information explaining what's failing with the test, and looking 
 at the test code, I struggle to see how these changes would impact that 
 test. We've had some significant issues with flaky clangd tests 
 (https://github.com/clangd/clangd/issues/1712), so it's possible this is 
 happenstance.
>>>
>>> I tried to debug that, and it looks like false Asan report. Maybe a bug in 
>>> FakeStack::GC related code.
>>
>> Thank you for looking into it! I came to the same general conclusion; and it 
>> seems the impacted bot has gone back to green in the meantime: 
>> https://lab.llvm.org/buildbot/#/builders/168/builds/15031 so hopefully we're 
>> in good shape.
>
> Yes. Fixed by D157552 . It's amusing to see 
> this unrelated patch exposing ~10 years old bug, which was never reported 
> before.

Oh wow! Great fix, thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153536

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

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[Vitaly Buka via Phabricator via cfe-commits]

vitalybuka added a comment.

In D153536#4576683 , @aaron.ballman 
wrote:

> In D153536#4571435 , @vitalybuka 
> wrote:
>
>>> Thanks! We might need someone from clangd to help here. The logs do not 
>>> have any information explaining what's failing with the test, and looking 
>>> at the test code, I struggle to see how these changes would impact that 
>>> test. We've had some significant issues with flaky clangd tests 
>>> (https://github.com/clangd/clangd/issues/1712), so it's possible this is 
>>> happenstance.
>>
>> I tried to debug that, and it looks like false Asan report. Maybe a bug in 
>> FakeStack::GC related code.
>
> Thank you for looking into it! I came to the same general conclusion; and it 
> seems the impacted bot has gone back to green in the meantime: 
> https://lab.llvm.org/buildbot/#/builders/168/builds/15031 so hopefully we're 
> in good shape.

Yes. Fixed by D157552 . It's amusing to see 
this unrelated patch exposing ~10 years old bug, which was never reported 
before.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153536

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

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D153536#4571435 , @vitalybuka 
wrote:

>> Thanks! We might need someone from clangd to help here. The logs do not have 
>> any information explaining what's failing with the test, and looking at the 
>> test code, I struggle to see how these changes would impact that test. We've 
>> had some significant issues with flaky clangd tests 
>> (https://github.com/clangd/clangd/issues/1712), so it's possible this is 
>> happenstance.
>
> I tried to debug that, and it looks like false Asan report. Maybe a bug in 
> FakeStack::GC related code.

Thank you for looking into it! I came to the same general conclusion; and it 
seems the impacted bot has gone back to green in the meantime: 
https://lab.llvm.org/buildbot/#/builders/168/builds/15031 so hopefully we're in 
good shape.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153536

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

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[Vitaly Buka via Phabricator via cfe-commits]

vitalybuka added a comment.

> Thanks! We might need someone from clangd to help here. The logs do not have 
> any information explaining what's failing with the test, and looking at the 
> test code, I struggle to see how these changes would impact that test. We've 
> had some significant issues with flaky clangd tests 
> (https://github.com/clangd/clangd/issues/1712), so it's possible this is 
> happenstance.

I tried to debug that, and it looks like false Asan report. Maybe a bug in 
FakeStack::GC related code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153536

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

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D153536#4570561 , @vitalybuka 
wrote:

> In D153536#4570534 , @aaron.ballman 
> wrote:
>
>> In D153536#4570513 , @vitalybuka 
>> wrote:
>>
>>> This patch brakes https://lab.llvm.org/buildbot/#/builders/168/builds/14997
>>> Not sure what is wrong there, probably the test need to be updated. Can you 
>>> please take a look?
>>
>> I'm a bit confused -- the linked bot is green and the failures to either 
>> side of the linked run are both failing for the same reason (which seems to 
>> be unrelated to the changes in this patch). It looks like this bot went red 
>> here: https://lab.llvm.org/buildbot/#/builders/168/builds/14944 and hasn't 
>> reliably come back to green.
>
> Sorry copy-pasted wrong url. This green is manual request on revision before 
> this patch.
>
> Correct failure https://lab.llvm.org/buildbot/#/builders/168/builds/14944
> So it's persistently red after that.
>
> If anyone wondering, I've bisected to this patch on buildbot host.
> For whatever reason does not reproduce on my workstation.

Thanks! We might need someone from clangd to help here. The logs do not have 
any information explaining what's failing with the test, and looking at the 
test code, I struggle to see how these changes would impact that test. We've 
had some significant issues with flaky clangd tests 
(https://github.com/clangd/clangd/issues/1712), so it's possible this is 
happenstance.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153536

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

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[Vitaly Buka via Phabricator via cfe-commits]

vitalybuka added a comment.

Not a test update

The second line of the test triggers:

RUN: not --crash clangd -lit-test -sync=0 < %s 2> %t.async.err
==

  Signalled while building preamble
Filename: =
  ==2319884==ERROR: AddressSanitizer: stack-use-after-return on address 
0x7f310f6eb210 at pc 0x55a3ecaab9f5 bp 0x7f3115c49920 sp 0x7f3115c49918
  READ of size 1 at 0x7f310f6eb210 thread T3
  #0 0x55a3ecaab9f4 in __is_long 
/b/sanitizer-x86_64-linux-bootstrap-asan/build/libcxx_build_asan/include/c++/v1/string:1734:33
  #1 0x55a3ecaab9f4 in __get_pointer 
/b/sanitizer-x86_64-linux-bootstrap-asan/build/libcxx_build_asan/include/c++/v1/string:1869:17
  #2 0x55a3ecaab9f4 in data 
/b/sanitizer-x86_64-linux-bootstrap-asan/build/libcxx_build_asan/include/c++/v1/string:1559:73
  #3 0x55a3ecaab9f4 in operator<< 
/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/include/llvm/Support/raw_ostream.h:249:22
  #4 0x55a3ecaab9f4 in clang::clangd::(anonymous 
namespace)::crashDumpCompileCommand(llvm::raw_ostream&, 
clang::tooling::CompileCommand const&) 
/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/clang-tools-extra/clangd/TUScheduler.cpp:1012:24
  #5 0x55a3ecaab285 in clang::clangd::(anonymous 
namespace)::crashDumpParseInputs(llvm::raw_ostream&, clang::clangd::ParseInputs 
const&) 
/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/clang-tools-extra/clangd/TUScheduler.cpp:1038:3
  #6 0x55a3ecec1c9d in operator() 
/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/include/llvm/ADT/FunctionExtras.h:382:12
  #7 0x55a3ecec1c9d in 
clang::clangd::ThreadCrashReporter::runCrashHandlers() 
/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/clang-tools-extra/clangd/support/ThreadCrashReporter.cpp:30:5
  #8 0x55a3ebacfd78 in operator() 
/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/clang-tools-extra/clangd/tool/ClangdMain.cpp:717:9
  #9 0x55a3ebacfd78 in clang::clangd::clangdMain(int, 
char**)::$_0::__invoke(void*) 
/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/clang-tools-extra/clangd/tool/ClangdMain.cpp:716:7
  #10 0x55a3e87c27ba in llvm::sys::RunSignalHandlers() 
/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/lib/Support/Signals.cpp:103:5
  #11 0x55a3e87cbd01 in SignalHandler(int) 
/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/lib/Support/Unix/Signals.inc:403:3
  #12 0x7f3115a3bcef  (/lib/x86_64-linux-gnu/libc.so.6+0x3bcef) (BuildId: 
d1704d25fbbb72fa95d517b883131828c0883fe9)
  #13 0x7f3115a9226a in pthread_kill 
(/lib/x86_64-linux-gnu/libc.so.6+0x9226a) (BuildId: 
d1704d25fbbb72fa95d517b883131828c0883fe9)
  #14 0x7f3115a3bc45 in raise (/lib/x86_64-linux-gnu/libc.so.6+0x3bc45) 
(BuildId: d1704d25fbbb72fa95d517b883131828c0883fe9)
  #15 0x7f3115a227fb in abort (/lib/x86_64-linux-gnu/libc.so.6+0x227fb) 
(BuildId: d1704d25fbbb72fa95d517b883131828c0883fe9)
  #16 0x55a3e869c429 in llvm::report_fatal_error(llvm::Twine const&, bool) 
/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/lib/Support/ErrorHandling.cpp:123:5
  #17 0x55a3e869c014 in llvm::report_fatal_error(char const*, bool) 
/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/lib/Support/ErrorHandling.cpp:83:3
  #18 0x55a3e9a2bb12 in (anonymous 
namespace)::PragmaDebugHandler::HandlePragma(clang::Preprocessor&, 
clang::PragmaIntroducer, clang::Token&) 
/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/clang/lib/Lex/Pragma.cpp:1102:9
  #19 0x55a3e9a0f270 in 
clang::Preprocessor::HandlePragmaDirective(clang::PragmaIntroducer) 
/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/clang/lib/Lex/Pragma.cpp:178:19
  #20 0x55a3e98e92a1 in clang::Preprocessor::HandleDirective(clang::Token&) 
/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/clang/lib/Lex/PPDirectives.cpp:1265:14
  #21 0x55a3e98a427a in clang::Lexer::LexTokenInternal(clang::Token&, bool) 
/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/clang/lib/Lex/Lexer.cpp:4362:7
  #22 0x55a3e989754c in clang::Lexer::Lex(clang::Token&) 
/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/clang/lib/Lex/Lexer.cpp:3578:24
  #23 0x55a3e9a5fb72 in clang::Preprocessor::Lex(clang::Token&) 
/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/clang/lib/Lex/Preprocessor.cpp:886:33
  #24 0x55a3eee8d7ef in ConsumeToken 
/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/clang/include/clang/Parse/Parser.h:504:8
  #25 0x55a3eee8d7ef in clang::Parser::Initialize() 
/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/clang/lib/Parse/Parser.cpp:568:3
  #26 0x55a3eee7732f in clang::ParseAST(clang::Sema&, bool, bool) 

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added a subscriber: sammccall.
cor3ntin added a comment.

Looking at the failing test 
(`clang-tools-extra/clangd/test/crash-preamble.test`), I'd be very surprising 
if it is related to a front end change like this PR. Any idea @sammccall  ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153536

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

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[Vitaly Buka via Phabricator via cfe-commits]

vitalybuka added a comment.

In D153536#4570534 , @aaron.ballman 
wrote:

> In D153536#4570513 , @vitalybuka 
> wrote:
>
>> This patch brakes https://lab.llvm.org/buildbot/#/builders/168/builds/14997
>> Not sure what is wrong there, probably the test need to be updated. Can you 
>> please take a look?
>
> I'm a bit confused -- the linked bot is green and the failures to either side 
> of the linked run are both failing for the same reason (which seems to be 
> unrelated to the changes in this patch). It looks like this bot went red 
> here: https://lab.llvm.org/buildbot/#/builders/168/builds/14944 and hasn't 
> reliably come back to green.

Sorry copy-pasted wrong url. This green is manual request on revision before 
this patch.

Correct failure https://lab.llvm.org/buildbot/#/builders/168/builds/14944
So it's persistently red after that.

If anyone wondering, I've bisected to this patch on buildbot host.
For whatever reason does not reproduce on my workstation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153536

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

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D153536#4570513 , @vitalybuka 
wrote:

> This patch brakes https://lab.llvm.org/buildbot/#/builders/168/builds/14997
> Not sure what is wrong there, probably the test need to be updated. Can you 
> please take a look?

I'm a bit confused -- the linked bot is green and the failures to either side 
of the linked run are both failing for the same reason (which seems to be 
unrelated to the changes in this patch). It looks like this bot went red here: 
https://lab.llvm.org/buildbot/#/builders/168/builds/14944 and hasn't reliably 
come back to green.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153536

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

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[Vitaly Buka via Phabricator via cfe-commits]

vitalybuka added a comment.

This patch brakes https://lab.llvm.org/buildbot/#/builders/168/builds/14997
Not sure what is wrong there, probably the test need to be updated. Can you 
please take a look?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153536

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

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[Corentin Jabot via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa8bef8865e4a: [Clang] Implement P2169 A nice placeholder 
with no name (authored by cor3ntin).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153536

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/Decl.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/IdentifierTable.h
  clang/include/clang/Sema/Lookup.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/Decl.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseTemplate.cpp
  clang/lib/Parse/Parser.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaInit.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/test/Lexer/cxx-features.cpp
  clang/test/Lexer/unicode.c
  clang/test/SemaCXX/anonymous-union-export.cpp
  clang/test/SemaCXX/cxx2c-placeholder-vars.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -150,7 +150,7 @@
  
   Placeholder variables with no name
   https://wg21.link/P2169R4;>P2169R4
-  No
+  Clang 18
  
 
 
Index: clang/test/SemaCXX/cxx2c-placeholder-vars.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/cxx2c-placeholder-vars.cpp
@@ -0,0 +1,254 @@
+// RUN: %clang -cc1 -fsyntax-only -verify -std=c++2c -Wunused-parameter -Wunused -Wpre-c++26-compat %s
+
+void static_var() {
+static int _; // expected-note {{previous definition is here}} \
+  // expected-note {{candidate}}
+static int _; // expected-error {{redefinition of '_'}}
+int _;// expected-warning {{placeholder variables are incompatible with C++ standards before C++2c}} \
+  // expected-note {{candidate}}
+_++; // expected-error{{reference to '_' is ambiguous}}
+}
+
+void static_var_2() {
+int _; // expected-note {{previous definition is here}}
+static int _; // expected-error {{redefinition of '_'}}
+}
+
+void bindings() {
+int arr[4] = {0, 1, 2, 3};
+auto [_, _, _, _] = arr; // expected-warning 3{{placeholder variables are incompatible with C++ standards before C++2c}} \
+ // expected-note 4{{placeholder declared here}}
+_ == 42; // expected-error {{ambiguous reference to placeholder '_', which is defined multiple times}}
+{
+// no extension warning as we only introduce a single placeholder.
+auto [_, a, b, c] = arr; // expected-warning {{unused variable '[_, a, b, c]'}}
+}
+{
+auto [_, _, b, c] = arr; // expected-warning {{unused variable '[_, _, b, c]'}} \
+ // expected-warning {{placeholder variables are incompatible with C++ standards before C++2c}}
+}
+{
+// There are only 3 extension warnings because the first
+// introduction of `_` is valid in all C++ standards
+auto [_, _, _, _] = arr; // expected-warning 3{{placeholder variables are incompatible with C++ standards before C++2c}}
+}
+}
+
+namespace StaticBindings {
+
+int arr[2] = {0, 1};
+static auto [_, _] = arr; // expected-error {{redefinition of '_'}} \
+  // expected-note  {{previous definition is here}}
+
+void f() {
+int arr[2] = {0, 1};
+static auto [_, _] = arr; // expected-error {{redefinition of '_'}} \
+// expected-note  {{previous definition is here}}
+}
+
+}
+
+void lambda() {
+(void)[_ = 0, _ = 1] { // expected-warning {{placeholder variables are incompatible with C++ standards before C++2c}} \
+   // expected-note 4{{placeholder declared here}}
+(void)_++; // expected-error {{ambiguous reference to placeholder '_', which is defined multiple times}}
+};
+
+{
+int _ = 12;
+(void)[_ = 0]{}; // no warning (different scope)
+}
+}
+
+namespace global_var {
+int _; // expected-note {{previous definition is here}}
+int _; // expected-error {{redefinition of '_'}}
+}
+
+namespace {
+int _; // expected-note {{previous definition is here}}
+int _; // expected-error {{redefinition of '_'}}
+}
+
+
+namespace global_fun {
+void _();
+void _();
+
+void _() {} // expected-note {{previous definition is here}}
+void _() {} // expected-error {{redefinition of '_'}}
+void _(int){}
+}
+
+typedef int _;
+typedef int _; // Type redeclaration, nothing to do with placeholders
+
+void extern_test() {
+extern int _;
+extern int _; // expected-note {{candidate}}
+int _; //expected-note {{candidate}}
+_++; // expected-error 

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin updated this revision to Diff 547206.
cor3ntin marked an inline comment as done.
cor3ntin added a comment.

Fix nits


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153536

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/Decl.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/IdentifierTable.h
  clang/include/clang/Sema/Lookup.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/Decl.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseTemplate.cpp
  clang/lib/Parse/Parser.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaInit.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/test/Lexer/cxx-features.cpp
  clang/test/Lexer/unicode.c
  clang/test/SemaCXX/anonymous-union-export.cpp
  clang/test/SemaCXX/cxx2c-placeholder-vars.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -150,7 +150,7 @@
  
   Placeholder variables with no name
   https://wg21.link/P2169R4;>P2169R4
-  No
+  Clang 18
  
 
 
Index: clang/test/SemaCXX/cxx2c-placeholder-vars.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/cxx2c-placeholder-vars.cpp
@@ -0,0 +1,254 @@
+// RUN: %clang -cc1 -fsyntax-only -verify -std=c++2c -Wunused-parameter -Wunused -Wpre-c++26-compat %s
+
+void static_var() {
+static int _; // expected-note {{previous definition is here}} \
+  // expected-note {{candidate}}
+static int _; // expected-error {{redefinition of '_'}}
+int _;// expected-warning {{placeholder variables are incompatible with C++ standards before C++2c}} \
+  // expected-note {{candidate}}
+_++; // expected-error{{reference to '_' is ambiguous}}
+}
+
+void static_var_2() {
+int _; // expected-note {{previous definition is here}}
+static int _; // expected-error {{redefinition of '_'}}
+}
+
+void bindings() {
+int arr[4] = {0, 1, 2, 3};
+auto [_, _, _, _] = arr; // expected-warning 3{{placeholder variables are incompatible with C++ standards before C++2c}} \
+ // expected-note 4{{placeholder declared here}}
+_ == 42; // expected-error {{ambiguous reference to placeholder '_', which is defined multiple times}}
+{
+// no extension warning as we only introduce a single placeholder.
+auto [_, a, b, c] = arr; // expected-warning {{unused variable '[_, a, b, c]'}}
+}
+{
+auto [_, _, b, c] = arr; // expected-warning {{unused variable '[_, _, b, c]'}} \
+ // expected-warning {{placeholder variables are incompatible with C++ standards before C++2c}}
+}
+{
+// There are only 3 extension warnings because the first
+// introduction of `_` is valid in all C++ standards
+auto [_, _, _, _] = arr; // expected-warning 3{{placeholder variables are incompatible with C++ standards before C++2c}}
+}
+}
+
+namespace StaticBindings {
+
+int arr[2] = {0, 1};
+static auto [_, _] = arr; // expected-error {{redefinition of '_'}} \
+  // expected-note  {{previous definition is here}}
+
+void f() {
+int arr[2] = {0, 1};
+static auto [_, _] = arr; // expected-error {{redefinition of '_'}} \
+// expected-note  {{previous definition is here}}
+}
+
+}
+
+void lambda() {
+(void)[_ = 0, _ = 1] { // expected-warning {{placeholder variables are incompatible with C++ standards before C++2c}} \
+   // expected-note 4{{placeholder declared here}}
+(void)_++; // expected-error {{ambiguous reference to placeholder '_', which is defined multiple times}}
+};
+
+{
+int _ = 12;
+(void)[_ = 0]{}; // no warning (different scope)
+}
+}
+
+namespace global_var {
+int _; // expected-note {{previous definition is here}}
+int _; // expected-error {{redefinition of '_'}}
+}
+
+namespace {
+int _; // expected-note {{previous definition is here}}
+int _; // expected-error {{redefinition of '_'}}
+}
+
+
+namespace global_fun {
+void _();
+void _();
+
+void _() {} // expected-note {{previous definition is here}}
+void _() {} // expected-error {{redefinition of '_'}}
+void _(int){}
+}
+
+typedef int _;
+typedef int _; // Type redeclaration, nothing to do with placeholders
+
+void extern_test() {
+extern int _;
+extern int _; // expected-note {{candidate}}
+int _; //expected-note {{candidate}}
+_++; // expected-error {{reference to '_' is ambiguous}}
+}
+
+
+struct Members {
+int _; // expected-note 2{{placeholder declared here}}
+

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM aside from some minor tweaks (some extra parens to remove, a spurious 
comment to remove).




Comment at: clang/lib/Sema/SemaDeclCXX.cpp:4359
+  llvm::find_if(Result, [this](const NamedDecl *Elem) {
+return (isa(Elem)) &&
+   Elem->isPlaceholderVar(getLangOpts());





Comment at: clang/lib/Sema/SemaDeclCXX.cpp:4370
+  break;
+if ((isa(ND)) &&
+ND->isPlaceholderVar(getLangOpts()))





Comment at: clang/test/SemaCXX/cxx2c-placeholder-vars.cpp:1-2
+///
+// RUN: %clang -cc1 -fsyntax-only -verify -std=c++2c -Wunused-parameter 
-Wunused -Wpre-c++26-compat %s
+




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153536

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

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin updated this revision to Diff 546915.
cor3ntin added a comment.

Fix release notes formatting


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153536

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/Decl.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/IdentifierTable.h
  clang/include/clang/Sema/Lookup.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/Decl.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseTemplate.cpp
  clang/lib/Parse/Parser.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaInit.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/test/Lexer/cxx-features.cpp
  clang/test/Lexer/unicode.c
  clang/test/SemaCXX/anonymous-union-export.cpp
  clang/test/SemaCXX/cxx2c-placeholder-vars.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -150,7 +150,7 @@
  
   Placeholder variables with no name
   https://wg21.link/P2169R4;>P2169R4
-  No
+  Clang 18
  
 
 
Index: clang/test/SemaCXX/cxx2c-placeholder-vars.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/cxx2c-placeholder-vars.cpp
@@ -0,0 +1,255 @@
+///
+// RUN: %clang -cc1 -fsyntax-only -verify -std=c++2c -Wunused-parameter -Wunused -Wpre-c++26-compat %s
+
+void static_var() {
+static int _; // expected-note {{previous definition is here}} \
+  // expected-note {{candidate}}
+static int _; // expected-error {{redefinition of '_'}}
+int _;// expected-warning {{placeholder variables are incompatible with C++ standards before C++2c}} \
+  // expected-note {{candidate}}
+_++; // expected-error{{reference to '_' is ambiguous}}
+}
+
+void static_var_2() {
+int _; // expected-note {{previous definition is here}}
+static int _; // expected-error {{redefinition of '_'}}
+}
+
+void bindings() {
+int arr[4] = {0, 1, 2, 3};
+auto [_, _, _, _] = arr; // expected-warning 3{{placeholder variables are incompatible with C++ standards before C++2c}} \
+ // expected-note 4{{placeholder declared here}}
+_ == 42; // expected-error {{ambiguous reference to placeholder '_', which is defined multiple times}}
+{
+// no extension warning as we only introduce a single placeholder.
+auto [_, a, b, c] = arr; // expected-warning {{unused variable '[_, a, b, c]'}}
+}
+{
+auto [_, _, b, c] = arr; // expected-warning {{unused variable '[_, _, b, c]'}} \
+ // expected-warning {{placeholder variables are incompatible with C++ standards before C++2c}}
+}
+{
+// There are only 3 extension warnings because the first
+// introduction of `_` is valid in all C++ standards
+auto [_, _, _, _] = arr; // expected-warning 3{{placeholder variables are incompatible with C++ standards before C++2c}}
+}
+}
+
+namespace StaticBindings {
+
+int arr[2] = {0, 1};
+static auto [_, _] = arr; // expected-error {{redefinition of '_'}} \
+  // expected-note  {{previous definition is here}}
+
+void f() {
+int arr[2] = {0, 1};
+static auto [_, _] = arr; // expected-error {{redefinition of '_'}} \
+// expected-note  {{previous definition is here}}
+}
+
+}
+
+void lambda() {
+(void)[_ = 0, _ = 1] { // expected-warning {{placeholder variables are incompatible with C++ standards before C++2c}} \
+   // expected-note 4{{placeholder declared here}}
+(void)_++; // expected-error {{ambiguous reference to placeholder '_', which is defined multiple times}}
+};
+
+{
+int _ = 12;
+(void)[_ = 0]{}; // no warning (different scope)
+}
+}
+
+namespace global_var {
+int _; // expected-note {{previous definition is here}}
+int _; // expected-error {{redefinition of '_'}}
+}
+
+namespace {
+int _; // expected-note {{previous definition is here}}
+int _; // expected-error {{redefinition of '_'}}
+}
+
+
+namespace global_fun {
+void _();
+void _();
+
+void _() {} // expected-note {{previous definition is here}}
+void _() {} // expected-error {{redefinition of '_'}}
+void _(int){}
+}
+
+typedef int _;
+typedef int _; // Type redeclaration, nothing to do with placeholders
+
+void extern_test() {
+extern int _;
+extern int _; // expected-note {{candidate}}
+int _; //expected-note {{candidate}}
+_++; // expected-error {{reference to '_' is ambiguous}}
+}
+
+
+struct Members {
+int _; // expected-note 2{{placeholder declared here}}
+int _; 

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin updated this revision to Diff 546910.
cor3ntin marked an inline comment as done.
cor3ntin added a comment.

- Rebase
- Address Aaron's comments
- Mention the lack of debugging support in the release notes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153536

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/Decl.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/IdentifierTable.h
  clang/include/clang/Sema/Lookup.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/Decl.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseTemplate.cpp
  clang/lib/Parse/Parser.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaInit.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/test/Lexer/cxx-features.cpp
  clang/test/Lexer/unicode.c
  clang/test/SemaCXX/anonymous-union-export.cpp
  clang/test/SemaCXX/cxx2c-placeholder-vars.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -150,7 +150,7 @@
  
   Placeholder variables with no name
   https://wg21.link/P2169R4;>P2169R4
-  No
+  Clang 18
  
 
 
Index: clang/test/SemaCXX/cxx2c-placeholder-vars.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/cxx2c-placeholder-vars.cpp
@@ -0,0 +1,255 @@
+///
+// RUN: %clang -cc1 -fsyntax-only -verify -std=c++2c -Wunused-parameter -Wunused -Wpre-c++26-compat %s
+
+void static_var() {
+static int _; // expected-note {{previous definition is here}} \
+  // expected-note {{candidate}}
+static int _; // expected-error {{redefinition of '_'}}
+int _;// expected-warning {{placeholder variables are incompatible with C++ standards before C++2c}} \
+  // expected-note {{candidate}}
+_++; // expected-error{{reference to '_' is ambiguous}}
+}
+
+void static_var_2() {
+int _; // expected-note {{previous definition is here}}
+static int _; // expected-error {{redefinition of '_'}}
+}
+
+void bindings() {
+int arr[4] = {0, 1, 2, 3};
+auto [_, _, _, _] = arr; // expected-warning 3{{placeholder variables are incompatible with C++ standards before C++2c}} \
+ // expected-note 4{{placeholder declared here}}
+_ == 42; // expected-error {{ambiguous reference to placeholder '_', which is defined multiple times}}
+{
+// no extension warning as we only introduce a single placeholder.
+auto [_, a, b, c] = arr; // expected-warning {{unused variable '[_, a, b, c]'}}
+}
+{
+auto [_, _, b, c] = arr; // expected-warning {{unused variable '[_, _, b, c]'}} \
+ // expected-warning {{placeholder variables are incompatible with C++ standards before C++2c}}
+}
+{
+// There are only 3 extension warnings because the first
+// introduction of `_` is valid in all C++ standards
+auto [_, _, _, _] = arr; // expected-warning 3{{placeholder variables are incompatible with C++ standards before C++2c}}
+}
+}
+
+namespace StaticBindings {
+
+int arr[2] = {0, 1};
+static auto [_, _] = arr; // expected-error {{redefinition of '_'}} \
+  // expected-note  {{previous definition is here}}
+
+void f() {
+int arr[2] = {0, 1};
+static auto [_, _] = arr; // expected-error {{redefinition of '_'}} \
+// expected-note  {{previous definition is here}}
+}
+
+}
+
+void lambda() {
+(void)[_ = 0, _ = 1] { // expected-warning {{placeholder variables are incompatible with C++ standards before C++2c}} \
+   // expected-note 4{{placeholder declared here}}
+(void)_++; // expected-error {{ambiguous reference to placeholder '_', which is defined multiple times}}
+};
+
+{
+int _ = 12;
+(void)[_ = 0]{}; // no warning (different scope)
+}
+}
+
+namespace global_var {
+int _; // expected-note {{previous definition is here}}
+int _; // expected-error {{redefinition of '_'}}
+}
+
+namespace {
+int _; // expected-note {{previous definition is here}}
+int _; // expected-error {{redefinition of '_'}}
+}
+
+
+namespace global_fun {
+void _();
+void _();
+
+void _() {} // expected-note {{previous definition is here}}
+void _() {} // expected-error {{redefinition of '_'}}
+void _(int){}
+}
+
+typedef int _;
+typedef int _; // Type redeclaration, nothing to do with placeholders
+
+void extern_test() {
+extern int _;
+extern int _; // expected-note {{candidate}}
+int _; //expected-note {{candidate}}
+_++; // expected-error {{reference to '_' 

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D153536#4556485 , @cor3ntin wrote:

> From yesterday's C++ clang WG meeting
>
> - We can land this now
> - We need to track the debugger issue
> - We should mention the lack of debugging support in the release notes
> - We should strive to improve the situation in the 18 time frame but it would 
> not be blocker as few people may be impacted.
> - We should remember to modify the release notes when debugger support is 
> improved if that happens before clang 18

Agreed; there are some minor things that need to be tweaked before landing this 
now, but I think the patch is pretty close to finished.




Comment at: clang/lib/Sema/SemaDeclCXX.cpp:4332
+  llvm::find_if(Result, [this](const NamedDecl *Elem) {
+return (isa(Elem) || isa(Elem)) &&
+   Elem->isPlaceholderVar(getLangOpts());





Comment at: clang/lib/Sema/SemaDeclCXX.cpp:4343
+  break;
+if ((isa(ND) || isa(ND)) &&
+ND->isPlaceholderVar(getLangOpts()))





Comment at: clang/lib/Sema/SemaDeclCXX.cpp:4355
+  for (auto *D : ClassDecl->lookup(MemberOrBase)) {
+if (isa(D) || isa(D)) {
+  bool IsPlaceholder = D->isPlaceholderVar(getLangOpts());





Comment at: clang/www/cxx_status.html:148
   https://wg21.link/P2169R4;>P2169R4
-  No
+  Clang 17
  




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153536

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

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added a comment.

From yesterday's C++ clang WG meeting

- We can land this now
- We need to track the debugger issue
- We should mention the lack of debugging support in the release notes
- We should strive to improve the situation in the 18 time frame but it would 
not be blocker as few people may be impacted.
- We should remember to modify the release notes when debugger support is 
improved if that happens before clang 18


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153536

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

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D153536#4521156 , 
@hubert.reinterpretcast wrote:

> In D153536#4512897 , @dblaikie 
> wrote:
>
>> but at least at a first blush I can't reproduce the failures shown...
>
> @dblaikie, you //did// reproduce the issue with the members. Both entries 
> have DW_AT_decl_line 2 and DW_AT_data_member_location 0 (the second entry 
> should indicate DW_AT_decl_line 3 and DW_AT_data_member_location 4):
>
>>   0x002e:   DW_TAG_structure_type
>>   DW_AT_calling_convention(DW_CC_pass_by_value)
>>   DW_AT_name  ("t1")
>>   DW_AT_byte_size (0x08)
>>   DW_AT_decl_file 
>> ("/usr/local/google/home/blaikie/dev/scratch/unused_member.cpp")
>>   DW_AT_decl_line (1)
>>   
>>   0x0034: DW_TAG_member
>> DW_AT_name("_")
>> DW_AT_type(0x0047 "int")
>> DW_AT_decl_file   
>> ("/usr/local/google/home/blaikie/dev/scratch/unused_member.cpp")
>> DW_AT_decl_line   (2)
>> DW_AT_data_member_location(0x00)
>>   
>>   0x003d: DW_TAG_member
>> DW_AT_name("_")
>> DW_AT_type(0x0047 "int")
>> DW_AT_decl_file   
>> ("/usr/local/google/home/blaikie/dev/scratch/unused_member.cpp")
>> DW_AT_decl_line   (2)
>> DW_AT_data_member_location(0x00)
>>   
>>   0x0046: NULL
>
> As for the block-scope case, I am still able to reproduce the issue (and also 
> your result that does not exhibit the issue). The key seems to be having the 
> `_`s on the same line.

Ah, fair enough - probably easy enough for you to look for some kind of map 
with inadequate key material in CGDebugInfo somewhere, I'd guess...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153536

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

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

In D153536#4512897 , @dblaikie wrote:

> but at least at a first blush I can't reproduce the failures shown...

@dblaikie, you //did// reproduce the issue with the members. Both entries have 
DW_AT_decl_line 2 and DW_AT_data_member_location 0 (the second entry should 
indicate DW_AT_decl_line 3 and DW_AT_data_member_location 4):

>   0x002e:   DW_TAG_structure_type
>   DW_AT_calling_convention(DW_CC_pass_by_value)
>   DW_AT_name  ("t1")
>   DW_AT_byte_size (0x08)
>   DW_AT_decl_file 
> ("/usr/local/google/home/blaikie/dev/scratch/unused_member.cpp")
>   DW_AT_decl_line (1)
>   
>   0x0034: DW_TAG_member
> DW_AT_name("_")
> DW_AT_type(0x0047 "int")
> DW_AT_decl_file   
> ("/usr/local/google/home/blaikie/dev/scratch/unused_member.cpp")
> DW_AT_decl_line   (2)
> DW_AT_data_member_location(0x00)
>   
>   0x003d: DW_TAG_member
> DW_AT_name("_")
> DW_AT_type(0x0047 "int")
> DW_AT_decl_file   
> ("/usr/local/google/home/blaikie/dev/scratch/unused_member.cpp")
> DW_AT_decl_line   (2)
> DW_AT_data_member_location(0x00)
>   
>   0x0046: NULL

As for the block-scope case, I am still able to reproduce the issue (and also 
your result that does not exhibit the issue). The key seems to be having the 
`_`s on the same line.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153536

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

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added a comment.

FYI, my current aim is to land that early in the Clang 18 cycle, that should 
give us time to find any remaining issue and maybe improve debugger support!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153536

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

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D153536#4490918 , @cor3ntin wrote:

> @dblaikie Would you be willing to look at the debugger side of things in a 
> subsequent patch? I'm not familiar with debug symbol code gen so I'm not sure 
> I'd be able to improve thing the right way.

Not sure I've got the time to do the fix myself, but might be able to provide 
pointers.

but at least at a first blush I can't reproduce the failures shown...

  struct t1 {
int _;
int _;
  };
  t1 v1;
  int main() {
int _;
int _;
  }

  0x002e:   DW_TAG_structure_type
  DW_AT_calling_convention(DW_CC_pass_by_value)
  DW_AT_name  ("t1")
  DW_AT_byte_size (0x08)
  DW_AT_decl_file 
("/usr/local/google/home/blaikie/dev/scratch/unused_member.cpp")
  DW_AT_decl_line (1)
  
  0x0034: DW_TAG_member
DW_AT_name("_")
DW_AT_type(0x0047 "int")
DW_AT_decl_file   
("/usr/local/google/home/blaikie/dev/scratch/unused_member.cpp")
DW_AT_decl_line   (2)
DW_AT_data_member_location(0x00)
  
  0x003d: DW_TAG_member
DW_AT_name("_")
DW_AT_type(0x0047 "int")
DW_AT_decl_file   
("/usr/local/google/home/blaikie/dev/scratch/unused_member.cpp")
DW_AT_decl_line   (2)
DW_AT_data_member_location(0x00)
  
  0x0046: NULL
  
  0x0047:   DW_TAG_base_type
  DW_AT_name  ("int")
  DW_AT_encoding  (DW_ATE_signed)
  DW_AT_byte_size (0x04)
  
  0x004b:   DW_TAG_subprogram
  DW_AT_low_pc(0x)
  DW_AT_high_pc   (0x0008)
  DW_AT_frame_base(DW_OP_reg6 RBP)
  DW_AT_name  ("main")
  DW_AT_decl_file 
("/usr/local/google/home/blaikie/dev/scratch/unused_member.cpp")
  DW_AT_decl_line (6)
  DW_AT_type  (0x0047 "int")
  DW_AT_external  (true)
  
  0x005a: DW_TAG_variable
DW_AT_location(DW_OP_fbreg -4)
DW_AT_name("_")
DW_AT_decl_file   
("/usr/local/google/home/blaikie/dev/scratch/unused_member.cpp")
DW_AT_decl_line   (7)
DW_AT_type(0x0047 "int")
  
  0x0065: DW_TAG_variable
DW_AT_location(DW_OP_fbreg -8)
DW_AT_name("_")
DW_AT_decl_file   
("/usr/local/google/home/blaikie/dev/scratch/unused_member.cpp")
DW_AT_decl_line   (8)
DW_AT_type(0x0047 "int")

Looks OK to me - two local variables with the same name, two member variables 
with the same name.

so probably at least one bug in lldb because it does seem to think `t1` has 
only one member. But the DWARF I see for the local variables doesn't seem to 
match the dump shown in https://reviews.llvm.org/D153536#4483191


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153536

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

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:136-139
+- Implemented `P2169R4: A nice placeholder with no name 
`_. This allows using `_`
+  as a variable name multiple times in the same scope and is supported in all 
C++ language modes as an extension.
+  An extension warning is produced when multiple variables are introduced by 
`_` in the same scope.
+  Unused warnings are no longer produced for variables named `_`.

Use double backticks to start/end inline code in RST.



Comment at: clang/lib/Sema/SemaInit.cpp:2607
+  ValueDecl *VD =
+  SemaRef.tryLookupUnambiguousFieldDecl(RT->getDecl(), FieldName);
+  if (auto *FD = dyn_cast_if_present(VD)) {

Needs rebase on top of 632dd6a4ca00.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153536

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

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added a comment.

@dblaikie Would you be willing to look at the debugger side of things in a 
subsequent patch? I'm not familiar with debug symbol code gen so I'm not sure 
I'd be able to improve thing the right way.




Comment at: clang/test/SemaCXX/cxx2c-placeholder-vars.cpp:2
+// RUN: %clang -cc1 -fsyntax-only -verify -std=c++2c -Wunused-parameter 
-Wunused %s
+
+void static_var() {

hubert.reinterpretcast wrote:
> hubert.reinterpretcast wrote:
> > hubert.reinterpretcast wrote:
> > > cor3ntin wrote:
> > > > cor3ntin wrote:
> > > > > cor3ntin wrote:
> > > > > > hubert.reinterpretcast wrote:
> > > > > > > hubert.reinterpretcast wrote:
> > > > > > > > hubert.reinterpretcast wrote:
> > > > > > > > > hubert.reinterpretcast wrote:
> > > > > > > > > > Can we have tests for:
> > > > > > > > > > ```
> > > > > > > > > > struct { int _, _; } a = { ._ = 0 };
> > > > > > > > > > ```
> > > > > > > > > > 
> > > > > > > > > > and
> > > > > > > > > > 
> > > > > > > > > > ```
> > > > > > > > > > struct A {
> > > > > > > > > >   A();
> > > > > > > > > >   int _, _;
> > > > > > > > > > };
> > > > > > > > > > 
> > > > > > > > > > A::A() : _(0) {}
> > > > > > > > > > ```
> > > > > > > > > Codegen test for
> > > > > > > > > ```
> > > > > > > > > static union { int _ = 42; };
> > > > > > > > > int  = _;
> > > > > > > > > int foo() { return 13; }
> > > > > > > > > static union { int _ = foo(); };
> > > > > > > > > int main(void) { return ref; }
> > > > > > > > > ```
> > > > > > > > > might be interesting.
> > > > > > > > > 
> > > > > > > > > I suspect that this case was missed in the committee 
> > > > > > > > > discussion of the paper @cor3ntin.
> > > > > > > > Less controversial tests to consider:
> > > > > > > > ```
> > > > > > > > struct A {
> > > > > > > >   int _;
> > > > > > > >   union { int _; };
> > > > > > > > };
> > > > > > > > struct B { union { int _, _; }; };
> > > > > > > > ```
> > > > > > > > 
> > > > > > > In a similar vein, a codegen test for:
> > > > > > > ```
> > > > > > > struct A { A(); };
> > > > > > > inline void f [[gnu::used]]() {
> > > > > > >   static union { A _{}; };
> > > > > > >   static union { A _{}; };
> > > > > > > }
> > > > > > > ```
> > > > > > > 
> > > > > > > Perhaps not intended to be allowed though (premise was no symbols 
> > > > > > > with "linkage"?)
> > > > > > What's interesting about 
> > > > > > 
> > > > > > ```
> > > > > > static union { int _ = 42; };
> > > > > > int  = _;
> > > > > > int foo() { return 13; }
> > > > > > static union { int _ = foo(); };
> > > > > > int main(void) { return ref; }
> > > > > > ```
> > > > > > ?
> > > > > > It's already supported by clang https://godbolt.org/z/6j89EdnEo
> > > > > > 
> > > > > > 
> > > > > > I'm adding the other tests (and fixing the associated bugs, of 
> > > > > > which there were a few...)
> > > > > > 
> > > > > > Perhaps not intended to be allowed though (premise was no symbols 
> > > > > > with "linkage"?)
> > > > > 
> > > > > 
> > > > > Yes, this should be ill-formed, anything where we would have to 
> > > > > mangle  multiple `_` should be ill-formed.
> > > > > I do believe that's covered though, `_` does not have storage 
> > > > > duration.
> > > > > What's interesting about 
> > > > > 
> > > > > ```
> > > > > static union { int _ = 42; };
> > > > > int  = _;
> > > > > int foo() { return 13; }
> > > > > static union { int _ = foo(); };
> > > > > int main(void) { return ref; }
> > > > > ```
> > > > > ?
> > > > > It's already supported by clang https://godbolt.org/z/6j89EdnEo
> > > > > 
> > > > > 
> > > > > I'm adding the other tests (and fixing the associated bugs, of which 
> > > > > there were a few...)
> > > > > 
> > > > 
> > > > I see it now. Thanks, I hate it. There is apparently a preexisting bug.
> > > > And yes, i think we should say something about members of anonymous 
> > > > union declared at namespace scope in the standard I realize now 
> > > > this is missing
> > > > Thanks for catching that.
> > > > Thanks for catching that.
> > > 
> > > Glad to be of help!
> > > I do believe that's covered though, `_` does not have storage duration.
> > 
> > It's not covered by the wording: `_` //is// a non-static data member.
> > 
> > 
> @cor3ntin, it seems the designated initializer case has not been added.
Thanks for noticing!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153536

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

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

In D153536#4483191 , 
@hubert.reinterpretcast wrote:

> Similarly, only one local variable out of two in the same line reported:

I can confirm that adding a lexical block scope causes both variables to be 
recorded into the debug info (thus this is a debug info generation issue unique 
to the new feature).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153536

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

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin updated this revision to Diff 538549.
cor3ntin added a comment.

Add missing designated initializer test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153536

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/Decl.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/IdentifierTable.h
  clang/include/clang/Sema/Lookup.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/Decl.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseTemplate.cpp
  clang/lib/Parse/Parser.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaInit.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/test/Lexer/cxx-features.cpp
  clang/test/Lexer/unicode.c
  clang/test/SemaCXX/anonymous-union-export.cpp
  clang/test/SemaCXX/cxx2c-placeholder-vars.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -145,7 +145,7 @@
  
   Placeholder variables with no name
   https://wg21.link/P2169R4;>P2169R4
-  No
+  Clang 17
  
 
 
Index: clang/test/SemaCXX/cxx2c-placeholder-vars.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/cxx2c-placeholder-vars.cpp
@@ -0,0 +1,255 @@
+///
+// RUN: %clang -cc1 -fsyntax-only -verify -std=c++2c -Wunused-parameter -Wunused -Wpre-c++26-compat %s
+
+void static_var() {
+static int _; // expected-note {{previous definition is here}} \
+  // expected-note {{candidate}}
+static int _; // expected-error {{redefinition of '_'}}
+int _;// expected-warning {{placeholder variables are incompatible with C++ standards before C++2c}} \
+  // expected-note {{candidate}}
+_++; // expected-error{{reference to '_' is ambiguous}}
+}
+
+void static_var_2() {
+int _; // expected-note {{previous definition is here}}
+static int _; // expected-error {{redefinition of '_'}}
+}
+
+void bindings() {
+int arr[4] = {0, 1, 2, 3};
+auto [_, _, _, _] = arr; // expected-warning 3{{placeholder variables are incompatible with C++ standards before C++2c}} \
+ // expected-note 4{{placeholder declared here}}
+_ == 42; // expected-error {{ambiguous reference to placeholder '_', which is defined multiple times}}
+{
+// no extension warning as we only introduce a single placeholder.
+auto [_, a, b, c] = arr; // expected-warning {{unused variable '[_, a, b, c]'}}
+}
+{
+auto [_, _, b, c] = arr; // expected-warning {{unused variable '[_, _, b, c]'}} \
+ // expected-warning {{placeholder variables are incompatible with C++ standards before C++2c}}
+}
+{
+// There are only 3 extension warnings because the first
+// introduction of `_` is valid in all C++ standards
+auto [_, _, _, _] = arr; // expected-warning 3{{placeholder variables are incompatible with C++ standards before C++2c}}
+}
+}
+
+namespace StaticBindings {
+
+int arr[2] = {0, 1};
+static auto [_, _] = arr; // expected-error {{redefinition of '_'}} \
+  // expected-note  {{previous definition is here}}
+
+void f() {
+int arr[2] = {0, 1};
+static auto [_, _] = arr; // expected-error {{redefinition of '_'}} \
+// expected-note  {{previous definition is here}}
+}
+
+}
+
+void lambda() {
+(void)[_ = 0, _ = 1] { // expected-warning {{placeholder variables are incompatible with C++ standards before C++2c}} \
+   // expected-note 4{{placeholder declared here}}
+(void)_++; // expected-error {{ambiguous reference to placeholder '_', which is defined multiple times}}
+};
+
+{
+int _ = 12;
+(void)[_ = 0]{}; // no warning (different scope)
+}
+}
+
+namespace global_var {
+int _; // expected-note {{previous definition is here}}
+int _; // expected-error {{redefinition of '_'}}
+}
+
+namespace {
+int _; // expected-note {{previous definition is here}}
+int _; // expected-error {{redefinition of '_'}}
+}
+
+
+namespace global_fun {
+void _();
+void _();
+
+void _() {} // expected-note {{previous definition is here}}
+void _() {} // expected-error {{redefinition of '_'}}
+void _(int){}
+}
+
+typedef int _;
+typedef int _; // Type redeclaration, nothing to do with placeholders
+
+void extern_test() {
+extern int _;
+extern int _; // expected-note {{candidate}}
+int _; //expected-note {{candidate}}
+_++; // expected-error {{reference to '_' is ambiguous}}
+}
+
+
+struct Members {
+int _; // expected-note 2{{placeholder declared here}}
+  

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin updated this revision to Diff 538545.
cor3ntin added a comment.

Add more lookup tests, including designated initializers


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153536

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/Decl.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/IdentifierTable.h
  clang/include/clang/Sema/Lookup.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/Decl.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseTemplate.cpp
  clang/lib/Parse/Parser.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaInit.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/test/Lexer/cxx-features.cpp
  clang/test/Lexer/unicode.c
  clang/test/SemaCXX/anonymous-union-export.cpp
  clang/test/SemaCXX/cxx2c-placeholder-vars.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -145,7 +145,7 @@
  
   Placeholder variables with no name
   https://wg21.link/P2169R4;>P2169R4
-  No
+  Clang 17
  
 
 
Index: clang/test/SemaCXX/cxx2c-placeholder-vars.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/cxx2c-placeholder-vars.cpp
@@ -0,0 +1,258 @@
+///
+// RUN: %clang -cc1 -fsyntax-only -verify -std=c++2c -Wunused-parameter -Wunused -Wpre-c++26-compat %s
+
+void static_var() {
+static int _; // expected-note {{previous definition is here}} \
+  // expected-note {{candidate}}
+static int _; // expected-error {{redefinition of '_'}}
+int _;// expected-warning {{placeholder variables are incompatible with C++ standards before C++2c}} \
+  // expected-note {{candidate}}
+_++; // expected-error{{reference to '_' is ambiguous}}
+}
+
+void static_var_2() {
+int _; // expected-note {{previous definition is here}}
+static int _; // expected-error {{redefinition of '_'}}
+}
+
+void bindings() {
+int arr[4] = {0, 1, 2, 3};
+auto [_, _, _, _] = arr; // expected-warning 3{{placeholder variables are incompatible with C++ standards before C++2c}} \
+ // expected-note 4{{placeholder declared here}}
+_ == 42; // expected-error {{ambiguous reference to placeholder '_', which is defined multiple times}}
+{
+// no extension warning as we only introduce a single placeholder.
+auto [_, a, b, c] = arr; // expected-warning {{unused variable '[_, a, b, c]'}}
+}
+{
+auto [_, _, b, c] = arr; // expected-warning {{unused variable '[_, _, b, c]'}} \
+ // expected-warning {{placeholder variables are incompatible with C++ standards before C++2c}}
+}
+{
+// There are only 3 extension warnings because the first
+// introduction of `_` is valid in all C++ standards
+auto [_, _, _, _] = arr; // expected-warning 3{{placeholder variables are incompatible with C++ standards before C++2c}}
+}
+}
+
+namespace StaticBindings {
+
+int arr[2] = {0, 1};
+static auto [_, _] = arr; // expected-error {{redefinition of '_'}} \
+  // expected-note  {{previous definition is here}}
+
+void f() {
+int arr[2] = {0, 1};
+static auto [_, _] = arr; // expected-error {{redefinition of '_'}} \
+// expected-note  {{previous definition is here}}
+}
+
+}
+
+void lambda() {
+(void)[_ = 0, _ = 1] { // expected-warning {{placeholder variables are incompatible with C++ standards before C++2c}} \
+   // expected-note 4{{placeholder declared here}}
+(void)_++; // expected-error {{ambiguous reference to placeholder '_', which is defined multiple times}}
+};
+
+{
+int _ = 12;
+(void)[_ = 0]{}; // no warning (different scope)
+}
+}
+
+namespace global_var {
+int _; // expected-note {{previous definition is here}}
+int _; // expected-error {{redefinition of '_'}}
+}
+
+namespace {
+int _; // expected-note {{previous definition is here}}
+int _; // expected-error {{redefinition of '_'}}
+}
+
+
+namespace global_fun {
+void _();
+void _();
+
+void _() {} // expected-note {{previous definition is here}}
+void _() {} // expected-error {{redefinition of '_'}}
+void _(int){}
+}
+
+typedef int _;
+typedef int _; // Type redeclaration, nothing to do with placeholders
+
+void extern_test() {
+extern int _;
+extern int _; // expected-note {{candidate}}
+int _; //expected-note {{candidate}}
+_++; // expected-error {{reference to '_' is ambiguous}}
+}
+
+
+struct Members {
+int _; // expected-note 2{{placeholder 

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

In D153536#4483182 , 
@hubert.reinterpretcast wrote:

> If `llvm-dwarfdump` is to be believed, it looks like a compiler bug (two 
> members reported at the same offset).

Similarly, only one local variable out of two in the same line reported:

  $ cat placeholderDbg2.cc
  extern "C" int printf(const char *, ...);
  struct A {
A(int x) : x(x) { }
~A() { printf("%d\n", x); }
int x;
  };
  int main() {
A _ = 0, _ = 1;
return 0;
  }
  Return:  0x00:0   Sat Jul  8 23:52:46 2023 EDT



  (lldb) target create "a.out"
  Current executable set to '/terrannew/hstong/.Lpcoral03/llvmbld/a.out' 
(powerpc64le).
  (lldb) b 9
  Breakpoint 1: where = a.out`main + 68 at placeholderDbg2.cc:9:3, address = 
0x00010ab4
  (lldb) r
  Process 2951717 launched: '/terrannew/hstong/.Lpcoral03/llvmbld/a.out' 
(powerpc64le)
  Process 2951717 stopped
  * thread #1, name = 'a.out', stop reason = breakpoint 1.1
  frame #0: 0x000100010ab4 a.out`main at placeholderDbg2.cc:9:3
 6};
 7int main() {
 8  A _ = 0, _ = 1;
  -> 9  return 0;
 10   }
  (lldb) var
  (A) _ = (x = 0)
  (lldb) c
  Process 2951717 resuming
  1
  0
  Process 2951717 exited with status = 0 (0x)
  (lldb) q



  0x009b:   DW_TAG_subprogram [14] * (0x000b)
  DW_AT_low_pc [DW_FORM_addr] (0x00010a70)
  DW_AT_high_pc [DW_FORM_data4]   (0x00b4)
  DW_AT_frame_base [DW_FORM_exprloc]  (DW_OP_reg31 X31)
  DW_AT_name [DW_FORM_strp]   ( .debug_str[0x0086] = 
"main")
  DW_AT_decl_file [DW_FORM_data1] 
("/terrannew/hstong/.Lpcoral03/llvmbld/placeholderDbg2.cc")
  DW_AT_decl_line [DW_FORM_data1] (7)
  DW_AT_type [DW_FORM_ref4]   (cu + 0x008f => {0x008f} 
"int")
  DW_AT_external [DW_FORM_flag_present]   (true)
  
  0x00b4: DW_TAG_variable [15]   (0x009b)
DW_AT_location [DW_FORM_exprloc]  (DW_OP_fbreg +128)
DW_AT_name [DW_FORM_strp] ( .debug_str[0x008b] = 
"_")
DW_AT_decl_file [DW_FORM_data1]   
("/terrannew/hstong/.Lpcoral03/llvmbld/placeholderDbg2.cc")
DW_AT_decl_line [DW_FORM_data1]   (8)
DW_AT_type [DW_FORM_ref4] (cu + 0x005a => {0x005a} 
"A")
  
  0x00c3: NULL


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153536

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

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/test/SemaCXX/cxx2c-placeholder-vars.cpp:46
+int arr[2] = {0, 1};
+static auto [_, _] = arr; // expected-error {{redefinition of '_'}} \
+// expected-note  {{previous definition is here}}

Just noting: This case is allowed by the wording, and I suspect it requires 
extended discussion.




Comment at: clang/test/SemaCXX/cxx2c-placeholder-vars.cpp:2
+// RUN: %clang -cc1 -fsyntax-only -verify -std=c++2c -Wunused-parameter 
-Wunused %s
+
+void static_var() {

hubert.reinterpretcast wrote:
> hubert.reinterpretcast wrote:
> > cor3ntin wrote:
> > > cor3ntin wrote:
> > > > cor3ntin wrote:
> > > > > hubert.reinterpretcast wrote:
> > > > > > hubert.reinterpretcast wrote:
> > > > > > > hubert.reinterpretcast wrote:
> > > > > > > > hubert.reinterpretcast wrote:
> > > > > > > > > Can we have tests for:
> > > > > > > > > ```
> > > > > > > > > struct { int _, _; } a = { ._ = 0 };
> > > > > > > > > ```
> > > > > > > > > 
> > > > > > > > > and
> > > > > > > > > 
> > > > > > > > > ```
> > > > > > > > > struct A {
> > > > > > > > >   A();
> > > > > > > > >   int _, _;
> > > > > > > > > };
> > > > > > > > > 
> > > > > > > > > A::A() : _(0) {}
> > > > > > > > > ```
> > > > > > > > Codegen test for
> > > > > > > > ```
> > > > > > > > static union { int _ = 42; };
> > > > > > > > int  = _;
> > > > > > > > int foo() { return 13; }
> > > > > > > > static union { int _ = foo(); };
> > > > > > > > int main(void) { return ref; }
> > > > > > > > ```
> > > > > > > > might be interesting.
> > > > > > > > 
> > > > > > > > I suspect that this case was missed in the committee discussion 
> > > > > > > > of the paper @cor3ntin.
> > > > > > > Less controversial tests to consider:
> > > > > > > ```
> > > > > > > struct A {
> > > > > > >   int _;
> > > > > > >   union { int _; };
> > > > > > > };
> > > > > > > struct B { union { int _, _; }; };
> > > > > > > ```
> > > > > > > 
> > > > > > In a similar vein, a codegen test for:
> > > > > > ```
> > > > > > struct A { A(); };
> > > > > > inline void f [[gnu::used]]() {
> > > > > >   static union { A _{}; };
> > > > > >   static union { A _{}; };
> > > > > > }
> > > > > > ```
> > > > > > 
> > > > > > Perhaps not intended to be allowed though (premise was no symbols 
> > > > > > with "linkage"?)
> > > > > What's interesting about 
> > > > > 
> > > > > ```
> > > > > static union { int _ = 42; };
> > > > > int  = _;
> > > > > int foo() { return 13; }
> > > > > static union { int _ = foo(); };
> > > > > int main(void) { return ref; }
> > > > > ```
> > > > > ?
> > > > > It's already supported by clang https://godbolt.org/z/6j89EdnEo
> > > > > 
> > > > > 
> > > > > I'm adding the other tests (and fixing the associated bugs, of which 
> > > > > there were a few...)
> > > > > 
> > > > > Perhaps not intended to be allowed though (premise was no symbols 
> > > > > with "linkage"?)
> > > > 
> > > > 
> > > > Yes, this should be ill-formed, anything where we would have to mangle  
> > > > multiple `_` should be ill-formed.
> > > > I do believe that's covered though, `_` does not have storage duration.
> > > > What's interesting about 
> > > > 
> > > > ```
> > > > static union { int _ = 42; };
> > > > int  = _;
> > > > int foo() { return 13; }
> > > > static union { int _ = foo(); };
> > > > int main(void) { return ref; }
> > > > ```
> > > > ?
> > > > It's already supported by clang https://godbolt.org/z/6j89EdnEo
> > > > 
> > > > 
> > > > I'm adding the other tests (and fixing the associated bugs, of which 
> > > > there were a few...)
> > > > 
> > > 
> > > I see it now. Thanks, I hate it. There is apparently a preexisting bug.
> > > And yes, i think we should say something about members of anonymous union 
> > > declared at namespace scope in the standard I realize now this is 
> > > missing
> > > Thanks for catching that.
> > > Thanks for catching that.
> > 
> > Glad to be of help!
> > I do believe that's covered though, `_` does not have storage duration.
> 
> It's not covered by the wording: `_` //is// a non-static data member.
> 
> 
@cor3ntin, it seems the designated initializer case has not been added.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153536

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

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

In D153536#4479275 , 
@hubert.reinterpretcast wrote:

> It seems the class member case trips up debuggers.

If `llvm-dwarfdump` is to be believed, it looks like a compiler bug (two 
members reported at the same offset).

  0x00a3:   DW_TAG_member [4]   (0x009d)
  DW_AT_name [DW_FORM_string] ("_")
  DW_AT_type [DW_FORM_ref4]   (cu + 0x00de => {0x00de} 
"int")
  DW_AT_decl_file [DW_FORM_data1] 
("/terrannew/hstong/.Lpcoral03/llvmbld/placeholderDbg.cc")
  DW_AT_decl_line [DW_FORM_data1] (2)
  DW_AT_data_member_location [DW_FORM_data1]  (0x00)
  
  0x00ad:   DW_TAG_member [4]   (0x009d)
  DW_AT_name [DW_FORM_string] ("_")
  DW_AT_type [DW_FORM_ref4]   (cu + 0x00de => {0x00de} 
"int")
  DW_AT_decl_file [DW_FORM_data1] 
("/terrannew/hstong/.Lpcoral03/llvmbld/placeholderDbg.cc")
  DW_AT_decl_line [DW_FORM_data1] (2)
  DW_AT_data_member_location [DW_FORM_data1]  (0x00)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153536

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

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

In D153536#4474066 , @cor3ntin wrote:

> Seems to work well enough @hubert.reinterpretcast

It seems the class member case trips up debuggers.

  union U {
struct A { int _, _; } a;
struct B { int x, y; } b;
  };
  U u = {{1, 2}};
  
  int main(void) { return u.b.x; }



  (lldb) b main
  Breakpoint 1: where = a.out`main + 28 at placeholderDbg.cc:7:18, address = 
0x0001080c
  (lldb) r
  Process 2339034 launched: '/terrannew/hstong/.Lpcoral03/llvmbld/a.out' 
(powerpc64le)
  Process 2339034 stopped
  * thread #1, name = 'a.out', stop reason = breakpoint 1.1
  frame #0: 0x00010001080c a.out`main at placeholderDbg.cc:7:18
 4};
 5U u = {{1, 2}};
 6
  -> 7int main(void) { return u.b.x; }
  (lldb) print u
  (U) {
a = (_ = 1)
b = (x = 1, y = 2)
  }
  (lldb)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153536

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

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin updated this revision to Diff 537745.
cor3ntin added a comment.

Add tests for static structured bindings


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153536

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/Decl.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/IdentifierTable.h
  clang/include/clang/Sema/Lookup.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/Decl.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseTemplate.cpp
  clang/lib/Parse/Parser.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaInit.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/test/Lexer/cxx-features.cpp
  clang/test/Lexer/unicode.c
  clang/test/SemaCXX/anonymous-union-export.cpp
  clang/test/SemaCXX/cxx2c-placeholder-vars.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -145,7 +145,7 @@
  
   Placeholder variables with no name
   https://wg21.link/P2169R4;>P2169R4
-  No
+  Clang 17
  
 
 
Index: clang/test/SemaCXX/cxx2c-placeholder-vars.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/cxx2c-placeholder-vars.cpp
@@ -0,0 +1,214 @@
+///
+// RUN: %clang -cc1 -fsyntax-only -verify -std=c++2c -Wunused-parameter -Wunused -Wpre-c++26-compat %s
+
+void static_var() {
+static int _; // expected-note {{previous definition is here}} \
+  // expected-note {{candidate}}
+static int _; // expected-error {{redefinition of '_'}}
+int _;// expected-warning {{placeholder variables are incompatible with C++ standards before C++2c}} \
+  // expected-note {{candidate}}
+_++; // expected-error{{reference to '_' is ambiguous}}
+}
+
+void static_var_2() {
+int _; // expected-note {{previous definition is here}}
+static int _; // expected-error {{redefinition of '_'}}
+}
+
+void bindings() {
+int arr[4] = {0, 1, 2, 3};
+auto [_, _, _, _] = arr; // expected-warning 3{{placeholder variables are incompatible with C++ standards before C++2c}} \
+ // expected-note 4{{placeholder declared here}}
+_ == 42; // expected-error {{ambiguous reference to placeholder '_', which is defined multiple times}}
+{
+// no extension warning as we only introduce a single placeholder.
+auto [_, a, b, c] = arr; // expected-warning {{unused variable '[_, a, b, c]'}}
+}
+{
+auto [_, _, b, c] = arr; // expected-warning {{unused variable '[_, _, b, c]'}} \
+ // expected-warning {{placeholder variables are incompatible with C++ standards before C++2c}}
+}
+{
+// There are only 3 extension warnings because the first
+// introduction of `_` is valid in all C++ standards
+auto [_, _, _, _] = arr; // expected-warning 3{{placeholder variables are incompatible with C++ standards before C++2c}}
+}
+}
+
+namespace StaticBindings {
+
+int arr[2] = {0, 1};
+static auto [_, _] = arr; // expected-error {{redefinition of '_'}} \
+  // expected-note  {{previous definition is here}}
+
+void f() {
+int arr[2] = {0, 1};
+static auto [_, _] = arr; // expected-error {{redefinition of '_'}} \
+// expected-note  {{previous definition is here}}
+}
+
+}
+
+void lambda() {
+(void)[_ = 0, _ = 1] { // expected-warning {{placeholder variables are incompatible with C++ standards before C++2c}} \
+   // expected-note 4{{placeholder declared here}}
+(void)_++; // expected-error {{ambiguous reference to placeholder '_', which is defined multiple times}}
+};
+
+{
+int _ = 12;
+(void)[_ = 0]{}; // no warning (different scope)
+}
+}
+
+namespace global_var {
+int _; // expected-note {{previous definition is here}}
+int _; // expected-error {{redefinition of '_'}}
+}
+
+namespace {
+int _; // expected-note {{previous definition is here}}
+int _; // expected-error {{redefinition of '_'}}
+}
+
+
+namespace global_fun {
+void _();
+void _();
+
+void _() {} // expected-note {{previous definition is here}}
+void _() {} // expected-error {{redefinition of '_'}}
+void _(int){}
+}
+
+typedef int _;
+typedef int _; // Type redeclaration, nothing to do with placeholders
+
+void extern_test() {
+extern int _;
+extern int _; // expected-note {{candidate}}
+int _; //expected-note {{candidate}}
+_++; // expected-error {{reference to '_' is ambiguous}}
+}
+
+
+struct Members {
+int _; // expected-note 2{{placeholder declared here}}
+ 

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin updated this revision to Diff 537742.
cor3ntin added a comment.

Address Hubert's feedback

- Add code and tests to properly support member initializer
- Add code and tests to support designated initializers
- Correctly hamdle members of anonymous union
- Make placeholder redeclaration in anonymous static union ill-formed, even if 
the wording fails to handle that case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153536

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/Decl.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/IdentifierTable.h
  clang/include/clang/Sema/Lookup.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/Decl.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseTemplate.cpp
  clang/lib/Parse/Parser.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaInit.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/test/Lexer/cxx-features.cpp
  clang/test/Lexer/unicode.c
  clang/test/SemaCXX/anonymous-union-export.cpp
  clang/test/SemaCXX/cxx2c-placeholder-vars.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -145,7 +145,7 @@
  
   Placeholder variables with no name
   https://wg21.link/P2169R4;>P2169R4
-  No
+  Clang 17
  
 
 
Index: clang/test/SemaCXX/cxx2c-placeholder-vars.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/cxx2c-placeholder-vars.cpp
@@ -0,0 +1,200 @@
+///
+// RUN: %clang -cc1 -fsyntax-only -verify -std=c++2c -Wunused-parameter -Wunused -Wpre-c++26-compat %s
+
+void static_var() {
+static int _; // expected-note {{previous definition is here}} \
+  // expected-note {{candidate}}
+static int _; // expected-error {{redefinition of '_'}}
+int _;// expected-warning {{placeholder variables are incompatible with C++ standards before C++2c}} \
+  // expected-note {{candidate}}
+_++; // expected-error{{reference to '_' is ambiguous}}
+}
+
+void static_var_2() {
+int _; // expected-note {{previous definition is here}}
+static int _; // expected-error {{redefinition of '_'}}
+}
+
+void bindings() {
+int arr[4] = {0, 1, 2, 3};
+auto [_, _, _, _] = arr; // expected-warning 3{{placeholder variables are incompatible with C++ standards before C++2c}} \
+ // expected-note 4{{placeholder declared here}}
+_ == 42; // expected-error {{ambiguous reference to placeholder '_', which is defined multiple times}}
+{
+// no extension warning as we only introduce a single placeholder.
+auto [_, a, b, c] = arr; // expected-warning {{unused variable '[_, a, b, c]'}}
+}
+{
+auto [_, _, b, c] = arr; // expected-warning {{unused variable '[_, _, b, c]'}} \
+ // expected-warning {{placeholder variables are incompatible with C++ standards before C++2c}}
+}
+{
+// There are only 3 extension warnings because the first
+// introduction of `_` is valid in all C++ standards
+auto [_, _, _, _] = arr; // expected-warning 3{{placeholder variables are incompatible with C++ standards before C++2c}}
+}
+}
+
+void lambda() {
+(void)[_ = 0, _ = 1] { // expected-warning {{placeholder variables are incompatible with C++ standards before C++2c}} \
+   // expected-note 4{{placeholder declared here}}
+(void)_++; // expected-error {{ambiguous reference to placeholder '_', which is defined multiple times}}
+};
+
+{
+int _ = 12;
+(void)[_ = 0]{}; // no warning (different scope)
+}
+}
+
+namespace global_var {
+int _; // expected-note {{previous definition is here}}
+int _; // expected-error {{redefinition of '_'}}
+}
+
+namespace {
+int _; // expected-note {{previous definition is here}}
+int _; // expected-error {{redefinition of '_'}}
+}
+
+
+namespace global_fun {
+void _();
+void _();
+
+void _() {} // expected-note {{previous definition is here}}
+void _() {} // expected-error {{redefinition of '_'}}
+void _(int){}
+}
+
+typedef int _;
+typedef int _; // Type redeclaration, nothing to do with placeholders
+
+void extern_test() {
+extern int _;
+extern int _; // expected-note {{candidate}}
+int _; //expected-note {{candidate}}
+_++; // expected-error {{reference to '_' is ambiguous}}
+}
+
+
+struct Members {
+int _; // expected-note 2{{placeholder declared here}}
+int _; // expected-warning{{placeholder variables are incompatible with C++ standards before C++2c}} \
+   // expected-note 

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D153536#4475308 , @cor3ntin wrote:

> In D153536#4475240 , @dblaikie 
> wrote:
>
>> In D153536#4475199 , @cor3ntin 
>> wrote:
>>
>>> In D153536#4474733 , @dblaikie 
>>> wrote:
>>>
 Maybe try testing with more different values, to demonstrate which 
 entities the debugger is finding?

   int _ = 1;
   int _ = 2;
   {
 int _ = 3;
 int _ = 7;
   }

 Or something like that. But, honestly, if the point is that these 
 variables should be unnameable - perhaps we shouldn't generate any DWARF 
 for them?
>>>
>>> The variables should still be inspectable, ideally. It's true it makes no 
>>> sense to be able to use them in expressions, and maybe if lldb use clang to 
>>> evaluate expressions that would just work?
>>
>> I think that's going to be tricky - because we don't emit DWARF to say where 
>> one variable's lifetime starts compared to another in the same scope - so 
>> likely a debugger would just always show the first or last variable with the 
>> same name in a given scope (so in the above example you could probably only 
>> print {1,3} or {2,7}) - and getting it wrong is more likely and more 
>> problematic than existing cases of shadowed variables. If we can't get this 
>> right, which I don't think we can, practically speaking, at the moment (the 
>> DWARF feature we'd have to support to do this better would probably be 
>> `DW_AT_start_scope` to say at what instruction the lifetime of a variable 
>> starts) - it may be best not to include it at all?
>
> It doesn't seems to be confused on simple examples
>
>   cpp
>   * thread #1, name = 'placeholder', stop reason = step in
>   frame #0: 0x5157 placeholder`main at 
> debug_placeholder.cpp:8:13
>  5{
>  6int _ = 4;
>  7int _ = 5;
>   -> 8int _ = 6;
>  9}
>  10   }
>   (lldb) frame variable
>   (int) _ = 1
>   (int) _ = 2
>   (int) _ = 3
>   (int) _ = 4
>   (int) _ = 5
>   (int) _ = -9432
>
> That being said, if you think it's more prudent, I would not mind

Huh, I guess maybe it's got some heuristics based on line number. Could 
probably thwart those by stamping this out with a macro (which would cause all 
code in the macro to be attributed to the line where the macro is named) - add 
some function calls in, so you can step back up from the call to get to more 
precise locations and you'd probably see some problems. That's probably a bit 
contrived (but maybe not - perhaps `_` would be more common in macros).

*shrug* maybe it's OK enough.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153536

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

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/test/SemaCXX/cxx2c-placeholder-vars.cpp:2
+// RUN: %clang -cc1 -fsyntax-only -verify -std=c++2c -Wunused-parameter 
-Wunused %s
+
+void static_var() {

hubert.reinterpretcast wrote:
> cor3ntin wrote:
> > cor3ntin wrote:
> > > cor3ntin wrote:
> > > > hubert.reinterpretcast wrote:
> > > > > hubert.reinterpretcast wrote:
> > > > > > hubert.reinterpretcast wrote:
> > > > > > > hubert.reinterpretcast wrote:
> > > > > > > > Can we have tests for:
> > > > > > > > ```
> > > > > > > > struct { int _, _; } a = { ._ = 0 };
> > > > > > > > ```
> > > > > > > > 
> > > > > > > > and
> > > > > > > > 
> > > > > > > > ```
> > > > > > > > struct A {
> > > > > > > >   A();
> > > > > > > >   int _, _;
> > > > > > > > };
> > > > > > > > 
> > > > > > > > A::A() : _(0) {}
> > > > > > > > ```
> > > > > > > Codegen test for
> > > > > > > ```
> > > > > > > static union { int _ = 42; };
> > > > > > > int  = _;
> > > > > > > int foo() { return 13; }
> > > > > > > static union { int _ = foo(); };
> > > > > > > int main(void) { return ref; }
> > > > > > > ```
> > > > > > > might be interesting.
> > > > > > > 
> > > > > > > I suspect that this case was missed in the committee discussion 
> > > > > > > of the paper @cor3ntin.
> > > > > > Less controversial tests to consider:
> > > > > > ```
> > > > > > struct A {
> > > > > >   int _;
> > > > > >   union { int _; };
> > > > > > };
> > > > > > struct B { union { int _, _; }; };
> > > > > > ```
> > > > > > 
> > > > > In a similar vein, a codegen test for:
> > > > > ```
> > > > > struct A { A(); };
> > > > > inline void f [[gnu::used]]() {
> > > > >   static union { A _{}; };
> > > > >   static union { A _{}; };
> > > > > }
> > > > > ```
> > > > > 
> > > > > Perhaps not intended to be allowed though (premise was no symbols 
> > > > > with "linkage"?)
> > > > What's interesting about 
> > > > 
> > > > ```
> > > > static union { int _ = 42; };
> > > > int  = _;
> > > > int foo() { return 13; }
> > > > static union { int _ = foo(); };
> > > > int main(void) { return ref; }
> > > > ```
> > > > ?
> > > > It's already supported by clang https://godbolt.org/z/6j89EdnEo
> > > > 
> > > > 
> > > > I'm adding the other tests (and fixing the associated bugs, of which 
> > > > there were a few...)
> > > > 
> > > > Perhaps not intended to be allowed though (premise was no symbols with 
> > > > "linkage"?)
> > > 
> > > 
> > > Yes, this should be ill-formed, anything where we would have to mangle  
> > > multiple `_` should be ill-formed.
> > > I do believe that's covered though, `_` does not have storage duration.
> > > What's interesting about 
> > > 
> > > ```
> > > static union { int _ = 42; };
> > > int  = _;
> > > int foo() { return 13; }
> > > static union { int _ = foo(); };
> > > int main(void) { return ref; }
> > > ```
> > > ?
> > > It's already supported by clang https://godbolt.org/z/6j89EdnEo
> > > 
> > > 
> > > I'm adding the other tests (and fixing the associated bugs, of which 
> > > there were a few...)
> > > 
> > 
> > I see it now. Thanks, I hate it. There is apparently a preexisting bug.
> > And yes, i think we should say something about members of anonymous union 
> > declared at namespace scope in the standard I realize now this is 
> > missing
> > Thanks for catching that.
> > Thanks for catching that.
> 
> Glad to be of help!
> I do believe that's covered though, `_` does not have storage duration.

It's not covered by the wording: `_` //is// a non-static data member.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153536

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

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/test/SemaCXX/cxx2c-placeholder-vars.cpp:2
+// RUN: %clang -cc1 -fsyntax-only -verify -std=c++2c -Wunused-parameter 
-Wunused %s
+
+void static_var() {

cor3ntin wrote:
> cor3ntin wrote:
> > cor3ntin wrote:
> > > hubert.reinterpretcast wrote:
> > > > hubert.reinterpretcast wrote:
> > > > > hubert.reinterpretcast wrote:
> > > > > > hubert.reinterpretcast wrote:
> > > > > > > Can we have tests for:
> > > > > > > ```
> > > > > > > struct { int _, _; } a = { ._ = 0 };
> > > > > > > ```
> > > > > > > 
> > > > > > > and
> > > > > > > 
> > > > > > > ```
> > > > > > > struct A {
> > > > > > >   A();
> > > > > > >   int _, _;
> > > > > > > };
> > > > > > > 
> > > > > > > A::A() : _(0) {}
> > > > > > > ```
> > > > > > Codegen test for
> > > > > > ```
> > > > > > static union { int _ = 42; };
> > > > > > int  = _;
> > > > > > int foo() { return 13; }
> > > > > > static union { int _ = foo(); };
> > > > > > int main(void) { return ref; }
> > > > > > ```
> > > > > > might be interesting.
> > > > > > 
> > > > > > I suspect that this case was missed in the committee discussion of 
> > > > > > the paper @cor3ntin.
> > > > > Less controversial tests to consider:
> > > > > ```
> > > > > struct A {
> > > > >   int _;
> > > > >   union { int _; };
> > > > > };
> > > > > struct B { union { int _, _; }; };
> > > > > ```
> > > > > 
> > > > In a similar vein, a codegen test for:
> > > > ```
> > > > struct A { A(); };
> > > > inline void f [[gnu::used]]() {
> > > >   static union { A _{}; };
> > > >   static union { A _{}; };
> > > > }
> > > > ```
> > > > 
> > > > Perhaps not intended to be allowed though (premise was no symbols with 
> > > > "linkage"?)
> > > What's interesting about 
> > > 
> > > ```
> > > static union { int _ = 42; };
> > > int  = _;
> > > int foo() { return 13; }
> > > static union { int _ = foo(); };
> > > int main(void) { return ref; }
> > > ```
> > > ?
> > > It's already supported by clang https://godbolt.org/z/6j89EdnEo
> > > 
> > > 
> > > I'm adding the other tests (and fixing the associated bugs, of which 
> > > there were a few...)
> > > 
> > > Perhaps not intended to be allowed though (premise was no symbols with 
> > > "linkage"?)
> > 
> > 
> > Yes, this should be ill-formed, anything where we would have to mangle  
> > multiple `_` should be ill-formed.
> > I do believe that's covered though, `_` does not have storage duration.
> > What's interesting about 
> > 
> > ```
> > static union { int _ = 42; };
> > int  = _;
> > int foo() { return 13; }
> > static union { int _ = foo(); };
> > int main(void) { return ref; }
> > ```
> > ?
> > It's already supported by clang https://godbolt.org/z/6j89EdnEo
> > 
> > 
> > I'm adding the other tests (and fixing the associated bugs, of which there 
> > were a few...)
> > 
> 
> I see it now. Thanks, I hate it. There is apparently a preexisting bug.
> And yes, i think we should say something about members of anonymous union 
> declared at namespace scope in the standard I realize now this is missing
> Thanks for catching that.
> Thanks for catching that.

Glad to be of help!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153536

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

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added inline comments.



Comment at: clang/test/SemaCXX/cxx2c-placeholder-vars.cpp:2
+// RUN: %clang -cc1 -fsyntax-only -verify -std=c++2c -Wunused-parameter 
-Wunused %s
+
+void static_var() {

cor3ntin wrote:
> cor3ntin wrote:
> > hubert.reinterpretcast wrote:
> > > hubert.reinterpretcast wrote:
> > > > hubert.reinterpretcast wrote:
> > > > > hubert.reinterpretcast wrote:
> > > > > > Can we have tests for:
> > > > > > ```
> > > > > > struct { int _, _; } a = { ._ = 0 };
> > > > > > ```
> > > > > > 
> > > > > > and
> > > > > > 
> > > > > > ```
> > > > > > struct A {
> > > > > >   A();
> > > > > >   int _, _;
> > > > > > };
> > > > > > 
> > > > > > A::A() : _(0) {}
> > > > > > ```
> > > > > Codegen test for
> > > > > ```
> > > > > static union { int _ = 42; };
> > > > > int  = _;
> > > > > int foo() { return 13; }
> > > > > static union { int _ = foo(); };
> > > > > int main(void) { return ref; }
> > > > > ```
> > > > > might be interesting.
> > > > > 
> > > > > I suspect that this case was missed in the committee discussion of 
> > > > > the paper @cor3ntin.
> > > > Less controversial tests to consider:
> > > > ```
> > > > struct A {
> > > >   int _;
> > > >   union { int _; };
> > > > };
> > > > struct B { union { int _, _; }; };
> > > > ```
> > > > 
> > > In a similar vein, a codegen test for:
> > > ```
> > > struct A { A(); };
> > > inline void f [[gnu::used]]() {
> > >   static union { A _{}; };
> > >   static union { A _{}; };
> > > }
> > > ```
> > > 
> > > Perhaps not intended to be allowed though (premise was no symbols with 
> > > "linkage"?)
> > What's interesting about 
> > 
> > ```
> > static union { int _ = 42; };
> > int  = _;
> > int foo() { return 13; }
> > static union { int _ = foo(); };
> > int main(void) { return ref; }
> > ```
> > ?
> > It's already supported by clang https://godbolt.org/z/6j89EdnEo
> > 
> > 
> > I'm adding the other tests (and fixing the associated bugs, of which there 
> > were a few...)
> > 
> > Perhaps not intended to be allowed though (premise was no symbols with 
> > "linkage"?)
> 
> 
> Yes, this should be ill-formed, anything where we would have to mangle  
> multiple `_` should be ill-formed.
> I do believe that's covered though, `_` does not have storage duration.
> What's interesting about 
> 
> ```
> static union { int _ = 42; };
> int  = _;
> int foo() { return 13; }
> static union { int _ = foo(); };
> int main(void) { return ref; }
> ```
> ?
> It's already supported by clang https://godbolt.org/z/6j89EdnEo
> 
> 
> I'm adding the other tests (and fixing the associated bugs, of which there 
> were a few...)
> 

I see it now. Thanks, I hate it. There is apparently a preexisting bug.
And yes, i think we should say something about members of anonymous union 
declared at namespace scope in the standard I realize now this is missing
Thanks for catching that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153536

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

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added a comment.

In D153536#4475215 , @philnik wrote:

> Would it be possible to make `__` also a placeholder, so standard libraries 
> can use this before C++26? Or is `_` already a reserved identifier? (I don't 
> think so)

I don't see an issue allowing that in the std namespace. If we allow it 
everywhere, I'm afraid people will rely on it, which would hurt portability/be 
confusing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153536

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

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added a comment.

In D153536#4475240 , @dblaikie wrote:

> In D153536#4475199 , @cor3ntin 
> wrote:
>
>> In D153536#4474733 , @dblaikie 
>> wrote:
>>
>>> Maybe try testing with more different values, to demonstrate which entities 
>>> the debugger is finding?
>>>
>>>   int _ = 1;
>>>   int _ = 2;
>>>   {
>>> int _ = 3;
>>> int _ = 7;
>>>   }
>>>
>>> Or something like that. But, honestly, if the point is that these variables 
>>> should be unnameable - perhaps we shouldn't generate any DWARF for them?
>>
>> The variables should still be inspectable, ideally. It's true it makes no 
>> sense to be able to use them in expressions, and maybe if lldb use clang to 
>> evaluate expressions that would just work?
>
> I think that's going to be tricky - because we don't emit DWARF to say where 
> one variable's lifetime starts compared to another in the same scope - so 
> likely a debugger would just always show the first or last variable with the 
> same name in a given scope (so in the above example you could probably only 
> print {1,3} or {2,7}) - and getting it wrong is more likely and more 
> problematic than existing cases of shadowed variables. If we can't get this 
> right, which I don't think we can, practically speaking, at the moment (the 
> DWARF feature we'd have to support to do this better would probably be 
> `DW_AT_start_scope` to say at what instruction the lifetime of a variable 
> starts) - it may be best not to include it at all?

It doesn't seems to be confused on simple examples

  cpp
  * thread #1, name = 'placeholder', stop reason = step in
  frame #0: 0x5157 placeholder`main at 
debug_placeholder.cpp:8:13
 5{
 6int _ = 4;
 7int _ = 5;
  -> 8int _ = 6;
 9}
 10   }
  (lldb) frame variable
  (int) _ = 1
  (int) _ = 2
  (int) _ = 3
  (int) _ = 4
  (int) _ = 5
  (int) _ = -9432

That being said, if you think it's more prudent, I would not mind


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153536

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

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D153536#4475199 , @cor3ntin wrote:

> In D153536#4474733 , @dblaikie 
> wrote:
>
>> Maybe try testing with more different values, to demonstrate which entities 
>> the debugger is finding?
>>
>>   int _ = 1;
>>   int _ = 2;
>>   {
>> int _ = 3;
>> int _ = 7;
>>   }
>>
>> Or something like that. But, honestly, if the point is that these variables 
>> should be unnameable - perhaps we shouldn't generate any DWARF for them?
>
> The variables should still be inspectable, ideally. It's true it makes no 
> sense to be able to use them in expressions, and maybe if lldb use clang to 
> evaluate expressions that would just work?

I think that's going to be tricky - because we don't emit DWARF to say where 
one variable's lifetime starts compared to another in the same scope - so 
likely a debugger would just always show the first or last variable with the 
same name in a given scope (so in the above example you could probably only 
print {1,3} or {2,7}) - and getting it wrong is more likely and more 
problematic than existing cases of shadowed variables. If we can't get this 
right, which I don't think we can, practically speaking, at the moment (the 
DWARF feature we'd have to support to do this better would probably be 
`DW_AT_start_scope` to say at what instruction the lifetime of a variable 
starts) - it may be best not to include it at all?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153536

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

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added a comment.

Would it be possible to make `__` also a placeholder, so standard libraries can 
use this before C++26? Or is `_` already a reserved identifier? (I don't think 
so)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153536

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

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added a comment.

In D153536#4474733 , @dblaikie wrote:

> Maybe try testing with more different values, to demonstrate which entities 
> the debugger is finding?
>
>   int _ = 1;
>   int _ = 2;
>   {
> int _ = 3;
> int _ = 7;
>   }
>
> Or something like that. But, honestly, if the point is that these variables 
> should be unnameable - perhaps we shouldn't generate any DWARF for them?

The variables should still be inspectable, ideally. It's true it makes no sense 
to be able to use them in expressions, and maybe if lldb use clang to evaluate 
expressions that would just work?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153536

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

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added inline comments.



Comment at: clang/test/SemaCXX/cxx2c-placeholder-vars.cpp:2
+// RUN: %clang -cc1 -fsyntax-only -verify -std=c++2c -Wunused-parameter 
-Wunused %s
+
+void static_var() {

hubert.reinterpretcast wrote:
> hubert.reinterpretcast wrote:
> > hubert.reinterpretcast wrote:
> > > hubert.reinterpretcast wrote:
> > > > Can we have tests for:
> > > > ```
> > > > struct { int _, _; } a = { ._ = 0 };
> > > > ```
> > > > 
> > > > and
> > > > 
> > > > ```
> > > > struct A {
> > > >   A();
> > > >   int _, _;
> > > > };
> > > > 
> > > > A::A() : _(0) {}
> > > > ```
> > > Codegen test for
> > > ```
> > > static union { int _ = 42; };
> > > int  = _;
> > > int foo() { return 13; }
> > > static union { int _ = foo(); };
> > > int main(void) { return ref; }
> > > ```
> > > might be interesting.
> > > 
> > > I suspect that this case was missed in the committee discussion of the 
> > > paper @cor3ntin.
> > Less controversial tests to consider:
> > ```
> > struct A {
> >   int _;
> >   union { int _; };
> > };
> > struct B { union { int _, _; }; };
> > ```
> > 
> In a similar vein, a codegen test for:
> ```
> struct A { A(); };
> inline void f [[gnu::used]]() {
>   static union { A _{}; };
>   static union { A _{}; };
> }
> ```
> 
> Perhaps not intended to be allowed though (premise was no symbols with 
> "linkage"?)
What's interesting about 

```
static union { int _ = 42; };
int  = _;
int foo() { return 13; }
static union { int _ = foo(); };
int main(void) { return ref; }
```
?
It's already supported by clang https://godbolt.org/z/6j89EdnEo


I'm adding the other tests (and fixing the associated bugs, of which there were 
a few...)




Comment at: clang/test/SemaCXX/cxx2c-placeholder-vars.cpp:2
+// RUN: %clang -cc1 -fsyntax-only -verify -std=c++2c -Wunused-parameter 
-Wunused %s
+
+void static_var() {

cor3ntin wrote:
> hubert.reinterpretcast wrote:
> > hubert.reinterpretcast wrote:
> > > hubert.reinterpretcast wrote:
> > > > hubert.reinterpretcast wrote:
> > > > > Can we have tests for:
> > > > > ```
> > > > > struct { int _, _; } a = { ._ = 0 };
> > > > > ```
> > > > > 
> > > > > and
> > > > > 
> > > > > ```
> > > > > struct A {
> > > > >   A();
> > > > >   int _, _;
> > > > > };
> > > > > 
> > > > > A::A() : _(0) {}
> > > > > ```
> > > > Codegen test for
> > > > ```
> > > > static union { int _ = 42; };
> > > > int  = _;
> > > > int foo() { return 13; }
> > > > static union { int _ = foo(); };
> > > > int main(void) { return ref; }
> > > > ```
> > > > might be interesting.
> > > > 
> > > > I suspect that this case was missed in the committee discussion of the 
> > > > paper @cor3ntin.
> > > Less controversial tests to consider:
> > > ```
> > > struct A {
> > >   int _;
> > >   union { int _; };
> > > };
> > > struct B { union { int _, _; }; };
> > > ```
> > > 
> > In a similar vein, a codegen test for:
> > ```
> > struct A { A(); };
> > inline void f [[gnu::used]]() {
> >   static union { A _{}; };
> >   static union { A _{}; };
> > }
> > ```
> > 
> > Perhaps not intended to be allowed though (premise was no symbols with 
> > "linkage"?)
> What's interesting about 
> 
> ```
> static union { int _ = 42; };
> int  = _;
> int foo() { return 13; }
> static union { int _ = foo(); };
> int main(void) { return ref; }
> ```
> ?
> It's already supported by clang https://godbolt.org/z/6j89EdnEo
> 
> 
> I'm adding the other tests (and fixing the associated bugs, of which there 
> were a few...)
> 
> Perhaps not intended to be allowed though (premise was no symbols with 
> "linkage"?)


Yes, this should be ill-formed, anything where we would have to mangle  
multiple `_` should be ill-formed.
I do believe that's covered though, `_` does not have storage duration.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153536

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

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/test/SemaCXX/cxx2c-placeholder-vars.cpp:2
+// RUN: %clang -cc1 -fsyntax-only -verify -std=c++2c -Wunused-parameter 
-Wunused %s
+
+void static_var() {

hubert.reinterpretcast wrote:
> hubert.reinterpretcast wrote:
> > hubert.reinterpretcast wrote:
> > > Can we have tests for:
> > > ```
> > > struct { int _, _; } a = { ._ = 0 };
> > > ```
> > > 
> > > and
> > > 
> > > ```
> > > struct A {
> > >   A();
> > >   int _, _;
> > > };
> > > 
> > > A::A() : _(0) {}
> > > ```
> > Codegen test for
> > ```
> > static union { int _ = 42; };
> > int  = _;
> > int foo() { return 13; }
> > static union { int _ = foo(); };
> > int main(void) { return ref; }
> > ```
> > might be interesting.
> > 
> > I suspect that this case was missed in the committee discussion of the 
> > paper @cor3ntin.
> Less controversial tests to consider:
> ```
> struct A {
>   int _;
>   union { int _; };
> };
> struct B { union { int _, _; }; };
> ```
> 
In a similar vein, a codegen test for:
```
struct A { A(); };
inline void f [[gnu::used]]() {
  static union { A _{}; };
  static union { A _{}; };
}
```

Perhaps not intended to be allowed though (premise was no symbols with 
"linkage"?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153536

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

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/test/SemaCXX/cxx2c-placeholder-vars.cpp:2
+// RUN: %clang -cc1 -fsyntax-only -verify -std=c++2c -Wunused-parameter 
-Wunused %s
+
+void static_var() {

hubert.reinterpretcast wrote:
> Can we have tests for:
> ```
> struct { int _, _; } a = { ._ = 0 };
> ```
> 
> and
> 
> ```
> struct A {
>   A();
>   int _, _;
> };
> 
> A::A() : _(0) {}
> ```
Codegen test for
```
static union { int _ = 42; };
int  = _;
int foo() { return 13; }
static union { int _ = foo(); };
int main(void) { return ref; }
```
might be interesting.

I suspect that this case was missed in the committee discussion of the paper 
@cor3ntin.



Comment at: clang/test/SemaCXX/cxx2c-placeholder-vars.cpp:2
+// RUN: %clang -cc1 -fsyntax-only -verify -std=c++2c -Wunused-parameter 
-Wunused %s
+
+void static_var() {

hubert.reinterpretcast wrote:
> hubert.reinterpretcast wrote:
> > Can we have tests for:
> > ```
> > struct { int _, _; } a = { ._ = 0 };
> > ```
> > 
> > and
> > 
> > ```
> > struct A {
> >   A();
> >   int _, _;
> > };
> > 
> > A::A() : _(0) {}
> > ```
> Codegen test for
> ```
> static union { int _ = 42; };
> int  = _;
> int foo() { return 13; }
> static union { int _ = foo(); };
> int main(void) { return ref; }
> ```
> might be interesting.
> 
> I suspect that this case was missed in the committee discussion of the paper 
> @cor3ntin.
Less controversial tests to consider:
```
struct A {
  int _;
  union { int _; };
};
struct B { union { int _, _; }; };
```



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153536

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

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

Maybe try testing with more different values, to demonstrate which entities the 
debugger is finding?

  int _ = 1;
  int _ = 2;
  {
int _ = 3;
int _ = 7;
  }

Or something like that. But, honestly, if the point is that these variables 
should be unnameable - perhaps we shouldn't generate any DWARF for them?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153536

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

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/test/SemaCXX/cxx2c-placeholder-vars.cpp:2
+// RUN: %clang -cc1 -fsyntax-only -verify -std=c++2c -Wunused-parameter 
-Wunused %s
+
+void static_var() {

Can we have tests for:
```
struct { int _, _; } a = { ._ = 0 };
```

and

```
struct A {
  A();
  int _, _;
};

A::A() : _(0) {}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153536

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

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added a subscriber: hubert.reinterpretcast.
cor3ntin added a comment.

Some tests with lldb

  (lldb) b main
  Breakpoint 1: where = placeholder`main + 4 at debug_placeholder.cpp:2:9, 
address = 0x5134
  (lldb) run
  Process 4044833 launched: 
'/home/cor3ntin/dev/compilers/LLVM/build-release/placeholder' (x86_64)
  Process 4044833 stopped
  * thread #1, name = 'placeholder', stop reason = breakpoint 1.1
  frame #0: 0x5134 placeholder`main at debug_placeholder.cpp:2:9
 1int main() {
  -> 2int _ = 0;
 3int _ = 0;
 4int _ = 0;
 5{
 6int _ = 0;
 7int _ = 0;
  (lldb) step
  Process 4044833 stopped
  * thread #1, name = 'placeholder', stop reason = step in
  frame #0: 0x513b placeholder`main at debug_placeholder.cpp:3:9
 1int main() {
 2int _ = 0;
  -> 3int _ = 0;
 4int _ = 0;
 5{
 6int _ = 0;
 7int _ = 0;
  (lldb) step
  Process 4044833 stopped
  * thread #1, name = 'placeholder', stop reason = step in
  frame #0: 0x5142 placeholder`main at debug_placeholder.cpp:4:9
 1int main() {
 2int _ = 0;
 3int _ = 0;
  -> 4int _ = 0;
 5{
 6int _ = 0;
 7int _ = 0;
  (lldb) frame variable
  (int) _ = 0
  (int) _ = 0
  (int) _ = 0
  (lldb) step
  Process 4044833 stopped
  * thread #1, name = 'placeholder', stop reason = step in
  frame #0: 0x5149 placeholder`main at 
debug_placeholder.cpp:6:13
 3int _ = 0;
 4int _ = 0;
 5{
  -> 6int _ = 0;
 7int _ = 0;
 8}
 9}
  (lldb) step
  Process 4044833 stopped
  * thread #1, name = 'placeholder', stop reason = step in
  frame #0: 0x5150 placeholder`main at 
debug_placeholder.cpp:7:13
 4int _ = 0;
 5{
 6int _ = 0;
  -> 7int _ = 0;
 8}
 9}
  (lldb) frame variable
  (int) _ = 0
  (int) _ = 0
  (int) _ = 0
  (int) _ = 0
  (int) _ = 32767
  (lldb) 

Seems to work well enough @hubert.reinterpretcast


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153536

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

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added reviewers: dblaikie, echristo.
aaron.ballman added a comment.

CCing David and Eric -- do you see any concerns regarding debug information for 
these changes?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153536

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

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin updated this revision to Diff 535846.
cor3ntin marked 5 inline comments as done.
cor3ntin added a comment.

- Address Aaron's feedback
  - Add more tests
  - Add a longer description of the feature in the release notes
  - address nitpicks

- Improve error message when using __builtin_offsetof


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153536

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/Decl.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/IdentifierTable.h
  clang/include/clang/Sema/Lookup.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/Decl.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/test/Lexer/cxx-features.cpp
  clang/test/Lexer/unicode.c
  clang/test/SemaCXX/cxx2c-placeholder-vars.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -145,7 +145,7 @@
  
   Placeholder variables with no name
   https://wg21.link/P2169R4;>P2169R4
-  No
+  Clang 17
  
 
 
Index: clang/test/SemaCXX/cxx2c-placeholder-vars.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/cxx2c-placeholder-vars.cpp
@@ -0,0 +1,143 @@
+// RUN: %clang -cc1 -fsyntax-only -verify -std=c++2c -Wunused-parameter -Wunused %s
+
+void static_var() {
+static int _; // expected-note {{previous definition is here}} \
+  // expected-note {{candidate}}
+static int _; // expected-error {{redefinition of '_'}}
+int _;// expected-warning {{placeholder variables are a C++2c extension}} \
+  // expected-note {{candidate}}
+_++; // expected-error{{reference to '_' is ambiguous}}
+}
+
+void static_var_2() {
+int _; // expected-note {{previous definition is here}}
+static int _; // expected-error {{redefinition of '_'}}
+}
+
+void bindings() {
+int arr[4] = {0, 1, 2, 3};
+auto [_, _, _, _] = arr; // expected-warning 3{{placeholder variables are a C++2c extension}} \
+ // expected-note 4{{placeholder declared here}}
+_ == 42; // expected-error {{ambiguous reference to placeholder '_', which is defined multiple times}}
+{
+// no extension warning as we only introduce a single placeholder.
+auto [_, a, b, c] = arr; // expected-warning {{unused variable '[_, a, b, c]'}}
+}
+{
+auto [_, _, b, c] = arr; // expected-warning {{unused variable '[_, _, b, c]'}} \
+ // expected-warning {{placeholder variables are a C++2c extension}}
+}
+{
+// There are only 3 extension warnings because the first
+// introduction of `_` is valid in all C++ standards
+auto [_, _, _, _] = arr; // expected-warning 3{{placeholder variables are a C++2c extension}}
+}
+}
+
+void lambda() {
+(void)[_ = 0, _ = 1] { // expected-warning {{placeholder variables are a C++2c extension}} \
+   // expected-note 4{{placeholder declared here}}
+(void)_++; // expected-error {{ambiguous reference to placeholder '_', which is defined multiple times}}
+};
+
+{
+int _ = 12;
+(void)[_ = 0]{}; // no warning (different scope)
+}
+}
+
+namespace global_var {
+int _; // expected-note {{previous definition is here}}
+int _; // expected-error {{redefinition of '_'}}
+}
+
+namespace {
+int _; // expected-note {{previous definition is here}}
+int _; // expected-error {{redefinition of '_'}}
+}
+
+
+namespace global_fun {
+void _();
+void _();
+
+void _() {} // expected-note {{previous definition is here}}
+void _() {} // expected-error {{redefinition of '_'}}
+void _(int){}
+}
+
+typedef int _;
+typedef int _; // Type redeclaration, nothing to do with placeholders
+
+void extern_test() {
+extern int _;
+extern int _; // expected-note {{candidate}}
+int _; //expected-note {{candidate}}
+_++; // expected-error {{reference to '_' is ambiguous}}
+}
+
+
+struct Members {
+int _; // expected-note 2{{placeholder declared here}}
+int _; // expected-warning{{placeholder variables are a C++2c extension}} \
+   // expected-note 2{{placeholder declared here}}
+void f() {
+_++; // expected-error {{ambiguous reference to placeholder '_', which is defined multiple times}}
+}
+void attributes() __attribute__((diagnose_if(_ != 0, "oh no!", "warning"))); // expected-error{{ambiguous reference to placeholder '_', which is defined multiple times}}
+};
+
+namespace using_ {
+int _; // expected-note {{target of using declaration}}
+void f() {
+int _; // expected-note {{conflicting 

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

I've not spotted anything major, but I would like some additional test 
coverage. Btw, you need to add this to the table in 
https://github.com/llvm/llvm-project/blob/189033e6bede96de0d74e61715fcd1b48d95e247/clang/docs/LanguageExtensions.rst?plain=1#L1429
 because this is an extension we intentionally support in older language modes 
(we want to let folks like libc++ rely on this behavior in Clang).




Comment at: clang/lib/AST/Decl.cpp:1115-1122
+  if (const VarDecl *VD = dyn_cast(this)) {
+if (isa(VD))
+  return false;
+if (VD->isInitCapture())
+  return true;
+return VD->getStorageDuration() == StorageDuration::SD_Automatic;
+  }





Comment at: clang/lib/Sema/SemaDecl.cpp:1979
 
-static bool ShouldDiagnoseUnusedDecl(const NamedDecl *D) {
+static bool ShouldDiagnoseUnusedDecl(const Sema , const NamedDecl *D) {
   if (D->isInvalidDecl())

This could take `const LangOptions &` instead of `const Sema &` as we don't 
actually use `Sema` here.



Comment at: clang/test/SemaCXX/cxx2c-placeholder-vars.cpp:3
+
+void static_var() {
+static int _; // expected-note {{previous definition is here}} \

Can you also add tests for global namespace and anonymous namespaces?

Can you add coverage for:
```
int _ = 12;
auto lam = [_ = 0]{}; // gets a placeholder extension warning (I don't think we 
issue a shadow warning currently, but probably should?)
```
and
```
void func() {
  // Is a placeholder variable and so it does not get a -Wunused-but-set 
warning despite
  int _ = 12;
  int x = 12; // Gets -Wunused-but-set warning
}
```




Comment at: clang/test/SemaCXX/cxx2c-placeholder-vars.cpp:19
+int arr[4] = {0, 1, 2, 3};
+auto [_, _, _, _] = arr; // expected-warning 3{{placeholder variables are 
a C++2c extension}} \
+ // expected-note 4{{placeholder declared here}}

I think we want additional comments explaining why there's only three instances 
of this warning instead of four because this situation is kind of weird. The 
first declaration *is* a placeholder, so we suppress unused variable warnings 
for it. However, because a single variable named `_` is valid code in earlier 
language modes, it's not really an *extension* until we introduce the second 
variable named `_` in the same scope in terms of conformance. It doesn't help 
users to issue an extension warning on the first declaration because the 
diagnostic behavior change is more likely appreciated than a surprising change.

I think this information should be documented somewhere (perhaps just in the 
release notes with an example since we don't have much to document about the 
extension itself).



Comment at: clang/test/SemaCXX/cxx2c-placeholder-vars.cpp:52
+void _() {} // expected-error {{redefinition of '_'}}
+void _(int){};
+}





Comment at: clang/test/SemaCXX/cxx2c-placeholder-vars.cpp:63-70
+struct Members {
+int _; // expected-note {{placeholder declared here}}
+int _; // expected-warning{{placeholder variables are a C++2c extension}} \
+   // expected-note {{placeholder declared here}}
+void f() {
+_++; // expected-error {{ambiguous reference to placeholder '_', which 
is defined multiple times}}
+}

Can you add a test for this case:
```
struct WhoWritesCodeLikeThis {
  int _;
  void f() {
int _;
_ = 12; // Not ambiguous reference to placeholder, accesses the local 
variable which shadows the field
(void)({ int _ = 12; _;}); // Also not an ambiguous reference to a 
placeholder, accesses the GNU statement expression variable which shadows the 
local variable
  }
  // None of the _ declarations should get the extension warning
};
```
and for:
```
typedef int _;
typedef int _; // Not a placeholder, but also not a redefinition, so 
this_is_fine.gif
```
and for:
```
struct S {
  int _, _;
};
constexpr int oh_no = __builtin_offsetof(S, _); // Ambiguous reference error
int S::* ref = ::_; // Ambiguous reference error
```
and for:
```
struct S {
  int _, _;
  void func() __attribute__((diagnose_if(_ != 0, "oh no!", "warning")));
};
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153536

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

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin marked 14 inline comments as done.
cor3ntin added inline comments.



Comment at: clang/include/clang/AST/DeclBase.h:340-342
+  /// Whether this declaration denotes a placeholder that can be
+  /// redefined in the same scope.
+  unsigned IsPlaceholder : 1;

aaron.ballman wrote:
> cor3ntin wrote:
> > aaron.ballman wrote:
> > > This seems a bit heavy to put on every `Decl`, it only matters for 
> > > declarations with names, right? Should this be on `NamedDecl` instead? 
> > > And should it be using terminology like "name independent"?
> > > 
> > > Actually, do we need this bit at all? Would it work for `NamedDecl` to 
> > > have a function that returns whether the declaration name is `_` or not? 
> > > Oh, I suppose we can't get away with that because `void _(int x);` does 
> > > not introduce a placeholder identifier but a regular one, right?
> > I think you made me realize we can probably compute that completely without 
> > using bits. Oups.
> > It would certainly simplify things. I'll investigate!
> Thank you!
Removed!



Comment at: clang/test/SemaCXX/cxx2c-placeholder-vars.cpp:31
+   // expected-note 4{{placeholder declared here}} \\
+   // expected-warning 2{{placeholder variable has no 
side effect}}
+(void)_++; // expected-error {{referring to placeholder '_' is not 
allowed}}

aaron.ballman wrote:
> cor3ntin wrote:
> > aaron.ballman wrote:
> > > I don't understand what this diagnostic is trying to tell me, can you 
> > > explain a bit more?
> > The no side effect one?
> > You are assigning something that has no side effect to something that you 
> > may never be able to name, so the whole thing is likely dead code.
> > I welcome improvements suggestions
> Yeah, the no side effect one.
> 
> Corentin and I discussed this off-list and I understand the situation a bit 
> better now. Basically, an init capture named `_` either has a side effect on 
> construction and/or destruction (like an RAII object) or it cannot be used 
> within the lambda. However, it does still have effects (for example, it 
> impacts whether the lambda has a function conversion operator).
> 
> That makes this kind of tricky to word. Given that it's a QoI warning, I'd 
> recommend we leave this bit for a follow-up so that we can land the majority 
> of the functionality and we can add the safety rails later.
Removed



Comment at: clang/test/SemaCXX/cxx2c-placeholder-vars.cpp:79
+void test_param(int _) {}
+void test_params(int _, int _); // expected-error {{redefinition of parameter 
'_'}} \
+// expected-note {{previous declaration is 
here}}

aaron.ballman wrote:
> cor3ntin wrote:
> > aaron.ballman wrote:
> > > Weird.. is this expected? You can have two variables named `_` in block 
> > > scope, but you can't have two parameters named `_` despite them 
> > > inhabiting the same block scope?
> > Yes, CWG decided against having it in parameter declarations, because 
> > otherwise we'd need them in template parameters which we didn't want to do.
> > And it would be useless, you can simply not name them.
> I guess I see the same amount of utility in allowing an init capture named 
> `_` as I do a function parameter named `_` as a local variable named `_` (if 
> it is an RAII type, it can do useful things in theory; otherwise, it's not a 
> useful declaration).
side effects are the main motivation for local variables beside structured 
bindings (and pattern matching later)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153536

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

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin updated this revision to Diff 535116.
cor3ntin added a comment.

- Address more feedback
- reword diag
- remove diag for unused init capture
- Remove the IsPlaceholder bit, as placeholder variables can be determined 
without data


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153536

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/Decl.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/IdentifierTable.h
  clang/include/clang/Sema/Lookup.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/Decl.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/test/Lexer/cxx-features.cpp
  clang/test/Lexer/unicode.c
  clang/test/SemaCXX/cxx2c-placeholder-vars.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -145,7 +145,7 @@
  
   Placeholder variables with no name
   https://wg21.link/P2169R4;>P2169R4
-  No
+  Clang 17
  
 
 
Index: clang/test/SemaCXX/cxx2c-placeholder-vars.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/cxx2c-placeholder-vars.cpp
@@ -0,0 +1,103 @@
+// RUN: %clang -cc1 -fsyntax-only -verify -std=c++2c -Wunused-parameter -Wunused %s
+
+void static_var() {
+static int _; // expected-note {{previous definition is here}} \
+  // expected-note {{candidate}}
+static int _; // expected-error {{redefinition of '_'}}
+int _;// expected-warning {{placeholder variables are a C++2c extension}} \
+  // expected-note {{candidate}}
+_++; // expected-error{{reference to '_' is ambiguous}}
+}
+
+void static_var_2() {
+int _; // expected-note {{previous definition is here}}
+static int _; // expected-error {{redefinition of '_'}}
+}
+
+void bindings() {
+int arr[4] = {0, 1, 2, 3};
+auto [_, _, _, _] = arr; // expected-warning 3{{placeholder variables are a C++2c extension}} \
+ // expected-note 4{{placeholder declared here}}
+_ == 42; // expected-error {{ambiguous reference to placeholder '_', which is defined multiple times}}
+{
+auto [_, a, b, c] = arr; // expected-warning {{unused variable '[_, a, b, c]'}}
+}
+{
+auto [_, _, b, c] = arr; // expected-warning {{unused variable '[_, _, b, c]'}} \
+ // expected-warning {{placeholder variables are a C++2c extension}}
+}
+{
+auto [_, _, _, _] = arr; // expected-warning 3{{placeholder variables are a C++2c extension}}
+}
+}
+
+void lambda() {
+(void)[_ = 0, _ = 1] { // expected-warning {{placeholder variables are a C++2c extension}} \
+   // expected-note 4{{placeholder declared here}}
+(void)_++; // expected-error {{ambiguous reference to placeholder '_', which is defined multiple times}}
+};
+}
+
+namespace global_var {
+int _; // expected-note {{previous definition is here}}
+int _; // expected-error {{redefinition of '_'}}
+}
+
+namespace global_fun {
+void _();
+void _();
+
+void _() {} // expected-note {{previous definition is here}}
+void _() {} // expected-error {{redefinition of '_'}}
+void _(int){};
+}
+
+void extern_test() {
+extern int _;
+extern int _; // expected-note {{candidate}}
+int _; //expected-note {{candidate}}
+_++; // expected-error {{reference to '_' is ambiguous}}
+}
+
+
+struct Members {
+int _; // expected-note {{placeholder declared here}}
+int _; // expected-warning{{placeholder variables are a C++2c extension}} \
+   // expected-note {{placeholder declared here}}
+void f() {
+_++; // expected-error {{ambiguous reference to placeholder '_', which is defined multiple times}}
+}
+};
+
+namespace using_ {
+int _; // expected-note {{target of using declaration}}
+void f() {
+int _; // expected-note {{conflicting declaration}}
+_ = 0;
+using using_::_; // expected-error {{target of using declaration conflicts with declaration already in scope}}
+}
+}
+
+
+void call(int);
+void test_param(int _) {}
+void test_params(int _, int _); // expected-error {{redefinition of parameter '_'}} \
+// expected-note {{previous declaration is here}}
+
+template  // expected-error {{declaration of '_' shadows template parameter}} \
+  // expected-note  {{template parameter is declared here}}
+auto i = 0;
+
+template 
+concept C = requires(T _, T _) {  // expected-error {{redefinition of parameter '_'}} \
+// expected-note {{previous declaration is here}}
+T{};
+};
+
+struct S {
+int a;
+};
+
+void f(S a, S _) { // 

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:135-136
 - Compiler flags ``-std=c++2c`` and ``-std=gnu++2c`` have been added for 
experimental C++2c implementation work.
+- Implemented `P2169R4: A nice placeholder with no name 
`_. This allows to use `_`
+  as a variable name multiple times in the same scope and is supported in all 
C++ language modes as an extension.
 

cor3ntin wrote:
> aaron.ballman wrote:
> > Should we consider adding this as an extension to C? It would be conforming 
> > there as well because it wouldn't change the behavior of any correct C 
> > code, right?
> Maybe we should consult WG14 first?
> It's of very questionable usefulness without structured bindings and 
> destructors.
We certainly could ask WG14 for their opinion, but the scenario I was thinking 
this would help with is for complex macros where variables are sometimes 
defined but the names are not important as they shouldn't escape. However, I 
think those scenarios generally introduce different scopes (often with GNU 
statement expressions) so there's not a naming conflict that we'd need to 
avoid. So I'm fine skipping C for now until we hit a concrete use case.



Comment at: clang/include/clang/AST/DeclBase.h:340-342
+  /// Whether this declaration denotes a placeholder that can be
+  /// redefined in the same scope.
+  unsigned IsPlaceholder : 1;

cor3ntin wrote:
> aaron.ballman wrote:
> > This seems a bit heavy to put on every `Decl`, it only matters for 
> > declarations with names, right? Should this be on `NamedDecl` instead? And 
> > should it be using terminology like "name independent"?
> > 
> > Actually, do we need this bit at all? Would it work for `NamedDecl` to have 
> > a function that returns whether the declaration name is `_` or not? Oh, I 
> > suppose we can't get away with that because `void _(int x);` does not 
> > introduce a placeholder identifier but a regular one, right?
> I think you made me realize we can probably compute that completely without 
> using bits. Oups.
> It would certainly simplify things. I'll investigate!
Thank you!



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:6589
+def err_using_placeholder_variable : Error<
+  "referring to placeholder '_' is not allowed">;
+def note_reference_placeholder : Note<

cor3ntin wrote:
> cjdb wrote:
> > aaron.ballman wrote:
> > > I don't think this helps the user understand what's wrong with their 
> > > code, especially given the somewhat odd language rules around the 
> > > feature. How about: `ambiguous reference to multiply-defined placeholder 
> > > '_'` or something along those lines? Then the note can show the previous 
> > > declarations of the placeholders that are in scope? e.g.,
> > > ```
> > > void g() {
> > > int _; // note: placeholder declared here
> > > _ = 0;
> > > int _; // note: placeholder declared here
> > > _ = 0; // error: `ambiguous reference to multiply-defined placeholder '_'`
> > > }
> > > ```
> > > CC @cjdb 
> > Agreed. I'd suggest a rewording though: I took "multiply" to mean the maths 
> > term until completing the sentence, rather than its alternative meaning of 
> > "multiple instances" (which is more or less the same meaning, but 
> > "multiply" maps to the `x * y` operation for me).
> > 
> > Perhaps `ambiguous reference to placeholder '_', which has multiple 
> > definitions`? Not sold on that being the best wording, but it does avoid 
> > the hardcoded-word-at-8yo problem :)
> @cjdb I like your suggestion. maybe "which is defined multiple times?"  - 
> pedantically a bunch of things  have been defined once each, it's not a 
> redefinition of the same entity in the c++ sense
I like either of your suggestions better than my use of "multiply-defined." :-)



Comment at: clang/lib/Sema/SemaDecl.cpp:2171-2172
 
+  if (VD->isPlaceholderVar())
+return;
+

cor3ntin wrote:
> aaron.ballman wrote:
> > What's the rationale here? I know the standard has a recommended practice, 
> > but I don't see why that should apply to the case where the user assigns a 
> > value to the placeholder but never references it otherwise.
> The intent is that this behave like `[[maybe_unused]]`
Ah, thank you! I think we should make a slightly wider change here then. Let's 
move `VD->hasAttr()` out of the first if statement and into this 
one, then add a comment about the relationship between the attribute and this 
feautre. WDYT?



Comment at: clang/test/SemaCXX/cxx2c-placeholder-vars.cpp:31
+   // expected-note 4{{placeholder declared here}} \\
+   // expected-warning 2{{placeholder variable has no 
side effect}}
+(void)_++; // expected-error {{referring to placeholder '_' is not 
allowed}}

cor3ntin wrote:
> aaron.ballman wrote:
> > I don't 

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin updated this revision to Diff 535089.
cor3ntin marked 2 inline comments as done.
cor3ntin added a comment.

Address some of Aarons comments before getting rid of the bit in Decl


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153536

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/Decl.h
  clang/include/clang/AST/DeclBase.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/IdentifierTable.h
  clang/include/clang/Sema/Lookup.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/Lexer/cxx-features.cpp
  clang/test/Lexer/unicode.c
  clang/test/SemaCXX/cxx2c-placeholder-vars.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -145,7 +145,7 @@
  
   Placeholder variables with no name
   https://wg21.link/P2169R4;>P2169R4
-  No
+  Clang 17
  
 
 
Index: clang/test/SemaCXX/cxx2c-placeholder-vars.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/cxx2c-placeholder-vars.cpp
@@ -0,0 +1,98 @@
+// RUN: %clang -cc1 -fsyntax-only -verify -std=c++2c -Wunused-parameter -Wunused %s
+
+void static_var() {
+static int _; // expected-note {{previous definition is here}} \
+  // expected-note {{candidate}}
+static int _; // expected-error {{redefinition of '_'}}
+int _;// expected-warning {{placeholder variables are a C++2c extension}} \
+  // expected-note {{candidate}}
+_++; // expected-error{{reference to '_' is ambiguous}}
+}
+
+void static_var_2() {
+int _; // expected-note {{previous definition is here}}
+static int _; // expected-error {{redefinition of '_'}}
+}
+
+void bindings() {
+int arr[4] = {0, 1, 2, 3};
+auto [_, _, _, _] = arr; // expected-warning 3{{placeholder variables are a C++2c extension}} \
+ // expected-note 4{{placeholder declared here}}
+_ == 42; // expected-error {{referring to placeholder '_' is not allowed}}
+{
+auto [_, a, b, c] = arr;
+auto [_, _, _, _] = arr; // expected-warning 4{{placeholder variables are a C++2c extension}}
+}
+}
+
+void lambda() {
+(void)[_ = 0, _ = 1] { // expected-warning {{placeholder variables are a C++2c extension}} \
+   // expected-note 4{{placeholder declared here}} \\
+   // expected-warning 2{{placeholder variable has no side effect}}
+(void)_++; // expected-error {{referring to placeholder '_' is not allowed}}
+};
+}
+
+namespace global_var {
+int _; // expected-note {{previous definition is here}}
+int _; // expected-error {{redefinition of '_'}}
+}
+
+namespace global_fun {
+void _();
+void _();
+
+void _() {} // expected-note {{previous definition is here}}
+void _() {} // expected-error {{redefinition of '_'}}
+void _(int){};
+}
+
+void extern_test() {
+extern int _;
+extern int _; // expected-note {{candidate}}
+int _; //expected-note {{candidate}}
+_++; // expected-error {{reference to '_' is ambiguous}}
+}
+
+
+struct Members {
+int _; // expected-note {{placeholder declared here}}
+int _; // expected-warning{{placeholder variables are a C++2c extension}} \
+   // expected-note {{placeholder declared here}}
+void f() {
+_++; // expected-error {{referring to placeholder '_' is not allowed}}
+}
+};
+
+namespace using_ {
+int _; // expected-note {{target of using declaration}}
+void f() {
+int _; // expected-note {{conflicting declaration}}
+_ = 0;
+using using_::_; // expected-error {{target of using declaration conflicts with declaration already in scope}}
+}
+}
+
+
+void call(int);
+void test_param(int _) {}
+void test_params(int _, int _); // expected-error {{redefinition of parameter '_'}} \
+// expected-note {{previous declaration is here}}
+
+template  // expected-error {{declaration of '_' shadows template parameter}} \
+  // expected-note  {{template parameter is declared here}}
+auto i = 0;
+
+template 
+concept C = requires(T _, T _) {  // expected-error {{redefinition of parameter '_'}} \
+// expected-note {{previous declaration is here}}
+T{};
+};
+
+struct S {
+int a;
+};
+
+void f(S a, S _) { // expected-warning {{unused parameter 'a'}}
+
+}
Index: clang/test/Lexer/unicode.c
===
--- 

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin marked 14 inline comments as done.
cor3ntin added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:135-136
 - Compiler flags ``-std=c++2c`` and ``-std=gnu++2c`` have been added for 
experimental C++2c implementation work.
+- Implemented `P2169R4: A nice placeholder with no name 
`_. This allows to use `_`
+  as a variable name multiple times in the same scope and is supported in all 
C++ language modes as an extension.
 

aaron.ballman wrote:
> Should we consider adding this as an extension to C? It would be conforming 
> there as well because it wouldn't change the behavior of any correct C code, 
> right?
Maybe we should consult WG14 first?
It's of very questionable usefulness without structured bindings and 
destructors.



Comment at: clang/include/clang/AST/DeclBase.h:340-342
+  /// Whether this declaration denotes a placeholder that can be
+  /// redefined in the same scope.
+  unsigned IsPlaceholder : 1;

aaron.ballman wrote:
> This seems a bit heavy to put on every `Decl`, it only matters for 
> declarations with names, right? Should this be on `NamedDecl` instead? And 
> should it be using terminology like "name independent"?
> 
> Actually, do we need this bit at all? Would it work for `NamedDecl` to have a 
> function that returns whether the declaration name is `_` or not? Oh, I 
> suppose we can't get away with that because `void _(int x);` does not 
> introduce a placeholder identifier but a regular one, right?
I think you made me realize we can probably compute that completely without 
using bits. Oups.
It would certainly simplify things. I'll investigate!



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:6589
+def err_using_placeholder_variable : Error<
+  "referring to placeholder '_' is not allowed">;
+def note_reference_placeholder : Note<

cjdb wrote:
> aaron.ballman wrote:
> > I don't think this helps the user understand what's wrong with their code, 
> > especially given the somewhat odd language rules around the feature. How 
> > about: `ambiguous reference to multiply-defined placeholder '_'` or 
> > something along those lines? Then the note can show the previous 
> > declarations of the placeholders that are in scope? e.g.,
> > ```
> > void g() {
> > int _; // note: placeholder declared here
> > _ = 0;
> > int _; // note: placeholder declared here
> > _ = 0; // error: `ambiguous reference to multiply-defined placeholder '_'`
> > }
> > ```
> > CC @cjdb 
> Agreed. I'd suggest a rewording though: I took "multiply" to mean the maths 
> term until completing the sentence, rather than its alternative meaning of 
> "multiple instances" (which is more or less the same meaning, but "multiply" 
> maps to the `x * y` operation for me).
> 
> Perhaps `ambiguous reference to placeholder '_', which has multiple 
> definitions`? Not sold on that being the best wording, but it does avoid the 
> hardcoded-word-at-8yo problem :)
@cjdb I like your suggestion. maybe "which is defined multiple times?"  - 
pedantically a bunch of things  have been defined once each, it's not a 
redefinition of the same entity in the c++ sense



Comment at: clang/lib/Sema/SemaDecl.cpp:2171-2172
 
+  if (VD->isPlaceholderVar())
+return;
+

aaron.ballman wrote:
> What's the rationale here? I know the standard has a recommended practice, 
> but I don't see why that should apply to the case where the user assigns a 
> value to the placeholder but never references it otherwise.
The intent is that this behave like `[[maybe_unused]]`



Comment at: clang/test/SemaCXX/cxx2c-placeholder-vars.cpp:31
+   // expected-note 4{{placeholder declared here}} \\
+   // expected-warning 2{{placeholder variable has no 
side effect}}
+(void)_++; // expected-error {{referring to placeholder '_' is not 
allowed}}

aaron.ballman wrote:
> I don't understand what this diagnostic is trying to tell me, can you explain 
> a bit more?
The no side effect one?
You are assigning something that has no side effect to something that you may 
never be able to name, so the whole thing is likely dead code.
I welcome improvements suggestions



Comment at: clang/test/SemaCXX/cxx2c-placeholder-vars.cpp:79
+void test_param(int _) {}
+void test_params(int _, int _); // expected-error {{redefinition of parameter 
'_'}} \
+// expected-note {{previous declaration is 
here}}

aaron.ballman wrote:
> Weird.. is this expected? You can have two variables named `_` in block 
> scope, but you can't have two parameters named `_` despite them inhabiting 
> the same block scope?
Yes, CWG decided against having it in parameter declarations, because otherwise 
we'd need them in template parameters which we didn't want to do.
And it would 

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:6589
+def err_using_placeholder_variable : Error<
+  "referring to placeholder '_' is not allowed">;
+def note_reference_placeholder : Note<

aaron.ballman wrote:
> I don't think this helps the user understand what's wrong with their code, 
> especially given the somewhat odd language rules around the feature. How 
> about: `ambiguous reference to multiply-defined placeholder '_'` or something 
> along those lines? Then the note can show the previous declarations of the 
> placeholders that are in scope? e.g.,
> ```
> void g() {
> int _; // note: placeholder declared here
> _ = 0;
> int _; // note: placeholder declared here
> _ = 0; // error: `ambiguous reference to multiply-defined placeholder '_'`
> }
> ```
> CC @cjdb 
Agreed. I'd suggest a rewording though: I took "multiply" to mean the maths 
term until completing the sentence, rather than its alternative meaning of 
"multiple instances" (which is more or less the same meaning, but "multiply" 
maps to the `x * y` operation for me).

Perhaps `ambiguous reference to placeholder '_', which has multiple 
definitions`? Not sold on that being the best wording, but it does avoid the 
hardcoded-word-at-8yo problem :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153536

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

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added subscribers: cjdb, aaron.ballman.
aaron.ballman added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:135-136
 - Compiler flags ``-std=c++2c`` and ``-std=gnu++2c`` have been added for 
experimental C++2c implementation work.
+- Implemented `P2169R4: A nice placeholder with no name 
`_. This allows to use `_`
+  as a variable name multiple times in the same scope and is supported in all 
C++ language modes as an extension.
 

Should we consider adding this as an extension to C? It would be conforming 
there as well because it wouldn't change the behavior of any correct C code, 
right?



Comment at: clang/include/clang/AST/DeclBase.h:340-342
+  /// Whether this declaration denotes a placeholder that can be
+  /// redefined in the same scope.
+  unsigned IsPlaceholder : 1;

This seems a bit heavy to put on every `Decl`, it only matters for declarations 
with names, right? Should this be on `NamedDecl` instead? And should it be 
using terminology like "name independent"?

Actually, do we need this bit at all? Would it work for `NamedDecl` to have a 
function that returns whether the declaration name is `_` or not? Oh, I suppose 
we can't get away with that because `void _(int x);` does not introduce a 
placeholder identifier but a regular one, right?



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:6589
+def err_using_placeholder_variable : Error<
+  "referring to placeholder '_' is not allowed">;
+def note_reference_placeholder : Note<

I don't think this helps the user understand what's wrong with their code, 
especially given the somewhat odd language rules around the feature. How about: 
`ambiguous reference to multiply-defined placeholder '_'` or something along 
those lines? Then the note can show the previous declarations of the 
placeholders that are in scope? e.g.,
```
void g() {
int _; // note: placeholder declared here
_ = 0;
int _; // note: placeholder declared here
_ = 0; // error: `ambiguous reference to multiply-defined placeholder '_'`
}
```
CC @cjdb 



Comment at: clang/include/clang/Sema/Lookup.h:121
+/// Name lookup results in an ambiguity because multiple placeholder
+/// variables were found in the same scope
+/// @code





Comment at: clang/lib/Sema/SemaDecl.cpp:2171-2172
 
+  if (VD->isPlaceholderVar())
+return;
+

What's the rationale here? I know the standard has a recommended practice, but 
I don't see why that should apply to the case where the user assigns a value to 
the placeholder but never references it otherwise.



Comment at: clang/lib/Sema/SemaDecl.cpp:7481
+if (!Previous.empty()) {
+  auto Decl = *Previous.begin();
+  const bool sameDC = Decl->getDeclContext()->getRedeclContext()->Equals(

Please spell out the type here.



Comment at: clang/lib/Sema/SemaDecl.cpp:7482
+  auto Decl = *Previous.begin();
+  const bool sameDC = Decl->getDeclContext()->getRedeclContext()->Equals(
+  DC->getRedeclContext());





Comment at: clang/lib/Sema/SemaDecl.cpp:7484-7486
+  if (sameDC && isDeclInScope(Decl, CurContext, S, false)) {
+DiagPlaceholderVariableDefinition(D.getIdentifierLoc());
+  }





Comment at: clang/lib/Sema/SemaDecl.cpp:8331
 
+  // Never warn about shadowing placeholder variable
+  if (ShadowedDecl->isPlaceholderVar())





Comment at: clang/lib/Sema/SemaDecl.cpp:18217
 
+  NewFD->setIsPlaceholderVar(LangOpts.CPlusPlus && II && II->isPlaceholder());
   if (PrevDecl && !isa(PrevDecl)) {

shafik wrote:
> cor3ntin wrote:
> > shafik wrote:
> > > Why can't we fold this into `FieldDecl::Create`? This comment applies in 
> > > some other places as well.
> > By adding a parameter to FieldDecl::Create? We could, I'm not sure it would 
> > necessarily be cleaner. Let me know what you prefer.
> It seems like having the code consolidated make for better maintainability 
> and figuring this out for future folks but I am willing to be convinced 
> otherwise.
We need to support partial construction because of AST deserialization, but it 
seems to me that we don't need to add a new parameter to `FieldDecl::Create()` 
anyway; can't we have the `FieldDecl` constructor look at the given 
`IdentifierInfo *` in that case, and if it's nonnull, set the placeholder from 
that?



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:886
 DeclarationNameInfo NameInfo(B.Name, B.NameLoc);
+IdentifierInfo *VarName = B.Name;
 LookupResult Previous(*this, NameInfo, LookupOrdinaryName,

`assert(VarName && "Cannot have an unnamed binding declaration");` ?



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:917

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:18217
 
+  NewFD->setIsPlaceholderVar(LangOpts.CPlusPlus && II && II->isPlaceholder());
   if (PrevDecl && !isa(PrevDecl)) {

cor3ntin wrote:
> shafik wrote:
> > Why can't we fold this into `FieldDecl::Create`? This comment applies in 
> > some other places as well.
> By adding a parameter to FieldDecl::Create? We could, I'm not sure it would 
> necessarily be cleaner. Let me know what you prefer.
It seems like having the code consolidated make for better maintainability and 
figuring this out for future folks but I am willing to be convinced otherwise.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153536

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

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin updated this revision to Diff 534592.
cor3ntin added a comment.

- Fix missed Decl abbreviation change
- Fix feature macro test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153536

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/Decl.h
  clang/include/clang/AST/DeclBase.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/IdentifierTable.h
  clang/include/clang/Sema/Lookup.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/Lexer/cxx-features.cpp
  clang/test/Lexer/unicode.c
  clang/test/SemaCXX/cxx2c-placeholder-vars.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -145,7 +145,7 @@
  
   Placeholder variables with no name
   https://wg21.link/P2169R4;>P2169R4
-  No
+  Clang 17
  
 
 
Index: clang/test/SemaCXX/cxx2c-placeholder-vars.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/cxx2c-placeholder-vars.cpp
@@ -0,0 +1,98 @@
+// RUN: %clang -cc1 -fsyntax-only -verify -std=c++2c -Wunused-parameter -Wunused %s
+
+void static_var() {
+static int _; // expected-note {{previous definition is here}} \
+  // expected-note {{candidate}}
+static int _; // expected-error {{redefinition of '_'}}
+int _;// expected-warning {{placeholder variables are a C++2c extension}} \
+  // expected-note {{candidate}}
+_++; // expected-error{{reference to '_' is ambiguous}}
+}
+
+void static_var_2() {
+int _; // expected-note {{previous definition is here}}
+static int _; // expected-error {{redefinition of '_'}}
+}
+
+void bindings() {
+int arr[4] = {0, 1, 2, 3};
+auto [_, _, _, _] = arr; // expected-warning 3{{placeholder variables are a C++2c extension}} \
+ // expected-note 4{{placeholder declared here}}
+_ == 42; // expected-error {{referring to placeholder '_' is not allowed}}
+{
+auto [_, a, b, c] = arr;
+auto [_, _, _, _] = arr; // expected-warning 4{{placeholder variables are a C++2c extension}}
+}
+}
+
+void lambda() {
+(void)[_ = 0, _ = 1] { // expected-warning {{placeholder variables are a C++2c extension}} \
+   // expected-note 4{{placeholder declared here}} \\
+   // expected-warning 2{{placeholder variable has no side effect}}
+(void)_++; // expected-error {{referring to placeholder '_' is not allowed}}
+};
+}
+
+namespace global_var {
+int _; // expected-note {{previous definition is here}}
+int _; // expected-error {{redefinition of '_'}}
+}
+
+namespace global_fun {
+void _();
+void _();
+
+void _() {} // expected-note {{previous definition is here}}
+void _() {} // expected-error {{redefinition of '_'}}
+void _(int){};
+}
+
+void extern_test() {
+extern int _;
+extern int _; // expected-note {{candidate}}
+int _; //expected-note {{candidate}}
+_++; // expected-error {{reference to '_' is ambiguous}}
+}
+
+
+struct Members {
+int _; // expected-note {{placeholder declared here}}
+int _; // expected-warning{{placeholder variables are a C++2c extension}} \
+   // expected-note {{placeholder declared here}}
+void f() {
+_++; // expected-error {{referring to placeholder '_' is not allowed}}
+}
+};
+
+namespace using_ {
+int _; // expected-note {{target of using declaration}}
+void f() {
+int _; // expected-note {{conflicting declaration}}
+_ = 0;
+using using_::_; // expected-error {{target of using declaration conflicts with declaration already in scope}}
+}
+}
+
+
+void call(int);
+void test_param(int _) {}
+void test_params(int _, int _); // expected-error {{redefinition of parameter '_'}} \
+// expected-note {{previous declaration is here}}
+
+template  // expected-error {{declaration of '_' shadows template parameter}} \
+  // expected-note  {{template parameter is declared here}}
+auto i = 0;
+
+template 
+concept C = requires(T _, T _) {  // expected-error {{redefinition of parameter '_'}} \
+// expected-note {{previous declaration is here}}
+T{};
+};
+
+struct S {
+int a;
+};
+
+void f(S a, S _) { // expected-warning {{unused parameter 'a'}}
+
+}
Index: clang/test/Lexer/unicode.c
===
--- clang/test/Lexer/unicode.c
+++ clang/test/Lexer/unicode.c
@@ -27,7 

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin updated this revision to Diff 534455.
cor3ntin added a comment.

Address ChuanqiXu's feedback


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153536

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/Decl.h
  clang/include/clang/AST/DeclBase.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/IdentifierTable.h
  clang/include/clang/Sema/Lookup.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/Lexer/cxx-features.cpp
  clang/test/Lexer/unicode.c
  clang/test/SemaCXX/cxx2c-placeholder-vars.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -145,7 +145,7 @@
  
   Placeholder variables with no name
   https://wg21.link/P2169R4;>P2169R4
-  No
+  Clang 17
  
 
 
Index: clang/test/SemaCXX/cxx2c-placeholder-vars.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/cxx2c-placeholder-vars.cpp
@@ -0,0 +1,98 @@
+// RUN: %clang -cc1 -fsyntax-only -verify -std=c++2c -Wunused-parameter -Wunused %s
+
+void static_var() {
+static int _; // expected-note {{previous definition is here}} \
+  // expected-note {{candidate}}
+static int _; // expected-error {{redefinition of '_'}}
+int _;// expected-warning {{placeholder variables are a C++2c extension}} \
+  // expected-note {{candidate}}
+_++; // expected-error{{reference to '_' is ambiguous}}
+}
+
+void static_var_2() {
+int _; // expected-note {{previous definition is here}}
+static int _; // expected-error {{redefinition of '_'}}
+}
+
+void bindings() {
+int arr[4] = {0, 1, 2, 3};
+auto [_, _, _, _] = arr; // expected-warning 3{{placeholder variables are a C++2c extension}} \
+ // expected-note 4{{placeholder declared here}}
+_ == 42; // expected-error {{referring to placeholder '_' is not allowed}}
+{
+auto [_, a, b, c] = arr;
+auto [_, _, _, _] = arr; // expected-warning 4{{placeholder variables are a C++2c extension}}
+}
+}
+
+void lambda() {
+(void)[_ = 0, _ = 1] { // expected-warning {{placeholder variables are a C++2c extension}} \
+   // expected-note 4{{placeholder declared here}} \\
+   // expected-warning 2{{placeholder variable has no side effect}}
+(void)_++; // expected-error {{referring to placeholder '_' is not allowed}}
+};
+}
+
+namespace global_var {
+int _; // expected-note {{previous definition is here}}
+int _; // expected-error {{redefinition of '_'}}
+}
+
+namespace global_fun {
+void _();
+void _();
+
+void _() {} // expected-note {{previous definition is here}}
+void _() {} // expected-error {{redefinition of '_'}}
+void _(int){};
+}
+
+void extern_test() {
+extern int _;
+extern int _; // expected-note {{candidate}}
+int _; //expected-note {{candidate}}
+_++; // expected-error {{reference to '_' is ambiguous}}
+}
+
+
+struct Members {
+int _; // expected-note {{placeholder declared here}}
+int _; // expected-warning{{placeholder variables are a C++2c extension}} \
+   // expected-note {{placeholder declared here}}
+void f() {
+_++; // expected-error {{referring to placeholder '_' is not allowed}}
+}
+};
+
+namespace using_ {
+int _; // expected-note {{target of using declaration}}
+void f() {
+int _; // expected-note {{conflicting declaration}}
+_ = 0;
+using using_::_; // expected-error {{target of using declaration conflicts with declaration already in scope}}
+}
+}
+
+
+void call(int);
+void test_param(int _) {}
+void test_params(int _, int _); // expected-error {{redefinition of parameter '_'}} \
+// expected-note {{previous declaration is here}}
+
+template  // expected-error {{declaration of '_' shadows template parameter}} \
+  // expected-note  {{template parameter is declared here}}
+auto i = 0;
+
+template 
+concept C = requires(T _, T _) {  // expected-error {{redefinition of parameter '_'}} \
+// expected-note {{previous declaration is here}}
+T{};
+};
+
+struct S {
+int a;
+};
+
+void f(S a, S _) { // expected-warning {{unused parameter 'a'}}
+
+}
Index: clang/test/Lexer/unicode.c
===
--- clang/test/Lexer/unicode.c
+++ clang/test/Lexer/unicode.c
@@ -27,7 +27,7 @@
 CHECK : The preprocessor 

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added inline comments.



Comment at: clang/lib/Serialization/ASTWriterDecl.cpp:2043
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // isMutable
-  Abv->Add(BitCodeAbbrevOp(0));   // StorageKind
+  Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // IsPlaceholder
+  Abv->Add(BitCodeAbbrevOp(0)); // StorageKind

In case `IsPlaceholder` must be 0 when use the abbreviation for FieldDecl, it 
is better to use `Abv->Add(BitCodeAbbrevOp(0));` here.



Comment at: clang/lib/Serialization/ASTWriterDecl.cpp:2077
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // isMutable
+  Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // IsPlaceholder
   Abv->Add(BitCodeAbbrevOp(0));   // InitStyle

ditto



Comment at: clang/lib/Serialization/ASTWriterDecl.cpp:2233
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // isARCPseudoStrong
+  Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // IsPlaceholder
   Abv->Add(BitCodeAbbrevOp(0));   // Linkage

ditto



Comment at: clang/lib/Serialization/ASTWriterDecl.cpp:2311
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // isARCPseudoStrong
+  Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // IsPlaceholder
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // 
IsThisDeclarationADemotedDefinition

ditto.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153536

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

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin updated this revision to Diff 534329.
cor3ntin marked 2 inline comments as done.
cor3ntin added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153536

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/Decl.h
  clang/include/clang/AST/DeclBase.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/IdentifierTable.h
  clang/include/clang/Sema/Lookup.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/Lexer/cxx-features.cpp
  clang/test/Lexer/unicode.c
  clang/test/SemaCXX/cxx2c-placeholder-vars.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -145,7 +145,7 @@
  
   Placeholder variables with no name
   https://wg21.link/P2169R4;>P2169R4
-  No
+  Clang 17
  
 
 
Index: clang/test/SemaCXX/cxx2c-placeholder-vars.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/cxx2c-placeholder-vars.cpp
@@ -0,0 +1,98 @@
+// RUN: %clang -cc1 -fsyntax-only -verify -std=c++2c -Wunused-parameter -Wunused %s
+
+void static_var() {
+static int _; // expected-note {{previous definition is here}} \
+  // expected-note {{candidate}}
+static int _; // expected-error {{redefinition of '_'}}
+int _;// expected-warning {{placeholder variables are a C++2c extension}} \
+  // expected-note {{candidate}}
+_++; // expected-error{{reference to '_' is ambiguous}}
+}
+
+void static_var_2() {
+int _; // expected-note {{previous definition is here}}
+static int _; // expected-error {{redefinition of '_'}}
+}
+
+void bindings() {
+int arr[4] = {0, 1, 2, 3};
+auto [_, _, _, _] = arr; // expected-warning 3{{placeholder variables are a C++2c extension}} \
+ // expected-note 4{{placeholder declared here}}
+_ == 42; // expected-error {{referring to placeholder '_' is not allowed}}
+{
+auto [_, a, b, c] = arr;
+auto [_, _, _, _] = arr; // expected-warning 4{{placeholder variables are a C++2c extension}}
+}
+}
+
+void lambda() {
+(void)[_ = 0, _ = 1] { // expected-warning {{placeholder variables are a C++2c extension}} \
+   // expected-note 4{{placeholder declared here}} \\
+   // expected-warning 2{{placeholder variable has no side effect}}
+(void)_++; // expected-error {{referring to placeholder '_' is not allowed}}
+};
+}
+
+namespace global_var {
+int _; // expected-note {{previous definition is here}}
+int _; // expected-error {{redefinition of '_'}}
+}
+
+namespace global_fun {
+void _();
+void _();
+
+void _() {} // expected-note {{previous definition is here}}
+void _() {} // expected-error {{redefinition of '_'}}
+void _(int){};
+}
+
+void extern_test() {
+extern int _;
+extern int _; // expected-note {{candidate}}
+int _; //expected-note {{candidate}}
+_++; // expected-error {{reference to '_' is ambiguous}}
+}
+
+
+struct Members {
+int _; // expected-note {{placeholder declared here}}
+int _; // expected-warning{{placeholder variables are a C++2c extension}} \
+   // expected-note {{placeholder declared here}}
+void f() {
+_++; // expected-error {{referring to placeholder '_' is not allowed}}
+}
+};
+
+namespace using_ {
+int _; // expected-note {{target of using declaration}}
+void f() {
+int _; // expected-note {{conflicting declaration}}
+_ = 0;
+using using_::_; // expected-error {{target of using declaration conflicts with declaration already in scope}}
+}
+}
+
+
+void call(int);
+void test_param(int _) {}
+void test_params(int _, int _); // expected-error {{redefinition of parameter '_'}} \
+// expected-note {{previous declaration is here}}
+
+template  // expected-error {{declaration of '_' shadows template parameter}} \
+  // expected-note  {{template parameter is declared here}}
+auto i = 0;
+
+template 
+concept C = requires(T _, T _) {  // expected-error {{redefinition of parameter '_'}} \
+// expected-note {{previous declaration is here}}
+T{};
+};
+
+struct S {
+int a;
+};
+
+void f(S a, S _) { // expected-warning {{unused parameter 'a'}}
+
+}
Index: clang/test/Lexer/unicode.c
===
--- clang/test/Lexer/unicode.c
+++ clang/test/Lexer/unicode.c
@@ -27,7 +27,7 @@
 CHECK 

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin marked 3 inline comments as done.
cor3ntin added inline comments.



Comment at: clang/lib/AST/Decl.cpp:1831
+  // Objective-C as an extension.
+  if (isa(this) && getLangOpts().ObjC)
 return true;

shafik wrote:
> It is not obvious to me if this is a drive by fix or this has a larger effect 
> in the context of this change. Is there a test that demonstrates the effect 
> of this change?
That's a left over from when the patched allowed placeholder in parameter names 
(EWG voted against that). thanks!



Comment at: clang/lib/Sema/SemaDecl.cpp:2024
   if (const VarDecl *VD = dyn_cast(D)) {
+if (D->isPlaceholderVar())
+  return !VD->hasInit();

erichkeane wrote:
> Why would we get here?  Doesn't line 1995 pick this up?  Or am I missing 
> where D is modified?
> 
> ALSO, I'd suggest making this VD->isPlaceholderVar, as to not annoy the SA 
> tools.
Yep, this was dead code, thanks!



Comment at: clang/lib/Sema/SemaDecl.cpp:18217
 
+  NewFD->setIsPlaceholderVar(LangOpts.CPlusPlus && II && II->isPlaceholder());
   if (PrevDecl && !isa(PrevDecl)) {

shafik wrote:
> Why can't we fold this into `FieldDecl::Create`? This comment applies in some 
> other places as well.
By adding a parameter to FieldDecl::Create? We could, I'm not sure it would 
necessarily be cleaner. Let me know what you prefer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153536

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

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin updated this revision to Diff 534185.
cor3ntin added a comment.

- Address Shafik's an Erich's comments
- Add missing serialization code


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153536

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/Decl.h
  clang/include/clang/AST/DeclBase.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/IdentifierTable.h
  clang/include/clang/Sema/Lookup.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/Lexer/cxx-features.cpp
  clang/test/Lexer/unicode.c
  clang/test/SemaCXX/cxx2c-placeholder-vars.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -145,7 +145,7 @@
  
   Placeholder variables with no name
   https://wg21.link/P2169R4;>P2169R4
-  No
+  Clang 17
  
 
 
Index: clang/test/SemaCXX/cxx2c-placeholder-vars.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/cxx2c-placeholder-vars.cpp
@@ -0,0 +1,98 @@
+// RUN: %clang -cc1 -fsyntax-only -verify -std=c++2c -Wunused-parameter -Wunused %s
+
+void static_var() {
+static int _; // expected-note {{previous definition is here}} \
+  // expected-note {{candidate}}
+static int _; // expected-error {{redefinition of '_'}}
+int _;// expected-warning {{placeholder variables are a C++2c extension}} \
+  // expected-note {{candidate}}
+_++; // expected-error{{reference to '_' is ambiguous}}
+}
+
+void static_var_2() {
+int _; // expected-note {{previous definition is here}}
+static int _; // expected-error {{redefinition of '_'}}
+}
+
+void bindings() {
+int arr[4] = {0, 1, 2, 3};
+auto [_, _, _, _] = arr; // expected-warning 3{{placeholder variables are a C++2c extension}} \
+ // expected-note 4{{placeholder declared here}}
+_ == 42; // expected-error {{referring to placeholder '_' is not allowed}}
+{
+auto [_, a, b, c] = arr;
+auto [_, _, _, _] = arr; // expected-warning 4{{placeholder variables are a C++2c extension}}
+}
+}
+
+void lambda() {
+(void)[_ = 0, _ = 1] { // expected-warning {{placeholder variables are a C++2c extension}} \
+   // expected-note 4{{placeholder declared here}} \\
+   // expected-warning 2{{placeholder variable has no side effect}}
+(void)_++; // expected-error {{referring to placeholder '_' is not allowed}}
+};
+}
+
+namespace global_var {
+int _; // expected-note {{previous definition is here}}
+int _; // expected-error {{redefinition of '_'}}
+}
+
+namespace global_fun {
+void _();
+void _();
+
+void _() {} // expected-note {{previous definition is here}}
+void _() {} // expected-error {{redefinition of '_'}}
+void _(int){};
+}
+
+void extern_test() {
+extern int _;
+extern int _; // expected-note {{candidate}}
+int _; //expected-note {{candidate}}
+_++; // expected-error {{reference to '_' is ambiguous}}
+}
+
+
+struct Members {
+int _; // expected-note {{placeholder declared here}}
+int _; // expected-warning{{placeholder variables are a C++2c extension}} \
+   // expected-note {{placeholder declared here}}
+void f() {
+_++; // expected-error {{referring to placeholder '_' is not allowed}}
+}
+};
+
+namespace using_ {
+int _; // expected-note {{target of using declaration}}
+void f() {
+int _; // expected-note {{conflicting declaration}}
+_ = 0;
+using using_::_; // expected-error {{target of using declaration conflicts with declaration already in scope}}
+}
+}
+
+
+void call(int);
+void test_param(int _) {}
+void test_params(int _, int _); // expected-error {{redefinition of parameter '_'}} \
+// expected-note {{previous declaration is here}}
+
+template  // expected-error {{declaration of '_' shadows template parameter}} \
+  // expected-note  {{template parameter is declared here}}
+auto i = 0;
+
+template 
+concept C = requires(T _, T _) {  // expected-error {{redefinition of parameter '_'}} \
+// expected-note {{previous declaration is here}}
+T{};
+};
+
+struct S {
+int a;
+};
+
+void f(S a, S _) { // expected-warning {{unused parameter 'a'}}
+
+}
Index: clang/test/Lexer/unicode.c
===
--- clang/test/Lexer/unicode.c
+++ clang/test/Lexer/unicode.c
@@ 

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:18217
 
+  NewFD->setIsPlaceholderVar(LangOpts.CPlusPlus && II && II->isPlaceholder());
   if (PrevDecl && !isa(PrevDecl)) {

Why can't we fold this into `FieldDecl::Create`? This comment applies in some 
other places as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153536

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

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added inline comments.



Comment at: clang/lib/AST/Decl.cpp:1831
+  // Objective-C as an extension.
+  if (isa(this) && getLangOpts().ObjC)
 return true;

It is not obvious to me if this is a drive by fix or this has a larger effect 
in the context of this change. Is there a test that demonstrates the effect of 
this change?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153536

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

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:2024
   if (const VarDecl *VD = dyn_cast(D)) {
+if (D->isPlaceholderVar())
+  return !VD->hasInit();

Why would we get here?  Doesn't line 1995 pick this up?  Or am I missing where 
D is modified?

ALSO, I'd suggest making this VD->isPlaceholderVar, as to not annoy the SA 
tools.



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:725
 
+void Sema::NotePlaceholderVariableDefinition(SourceLocation Loc) {
+  Diag(Loc, getLangOpts().CPlusPlus26

Not a fan of this function name... Perhaps Diag instead of Note?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153536

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

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added inline comments.



Comment at: clang/test/Lexer/unicode.c:30
 
-int _;
+int a;
 

tbaeder wrote:
> Are these changes only for the case where we compile as c++? I know lots of C 
> (and c++?) projects use `_` as GNU gettext identifier to mark a translatable 
> string.
> Are these changes only for the case where we compile as c++
Yes

>  I know lots of C (and c++?) projects use _ as GNU gettext identifier to mark 
> a translatable string.

That's doesn't interfere with this feature. if `_` is defined as a macro, it's 
simply not possible to declare a variable of that name,  let alone more than 
one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153536

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

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[Timm Bäder via Phabricator via cfe-commits]

tbaeder added inline comments.



Comment at: clang/test/Lexer/unicode.c:30
 
-int _;
+int a;
 

Are these changes only for the case where we compile as c++? I know lots of C 
(and c++?) projects use `_` as GNU gettext identifier to mark a translatable 
string.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153536

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

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin created this revision.
Herald added a project: All.
cor3ntin requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This is a C++ feature that allows the use of `_` to
declare multiple variable of that name in the same scope;
these variables can then not be refered to.

In addition, while P2169  does not extend to 
parameter \
declarations, we stop warning on unused parameters of that name,
for consistency.

The feature is backported to all C++ language modes.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D153536

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/Decl.h
  clang/include/clang/AST/DeclBase.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/IdentifierTable.h
  clang/include/clang/Sema/Lookup.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/Decl.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/test/Lexer/cxx-features.cpp
  clang/test/Lexer/unicode.c
  clang/test/SemaCXX/cxx2c-placeholder-vars.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -145,7 +145,7 @@
  
   Placeholder variables with no name
   https://wg21.link/P2169R4;>P2169R4
-  No
+  Clang 17
  
 
 
Index: clang/test/SemaCXX/cxx2c-placeholder-vars.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/cxx2c-placeholder-vars.cpp
@@ -0,0 +1,98 @@
+// RUN: %clang -cc1 -fsyntax-only -verify -std=c++2c -Wunused-parameter -Wunused %s
+
+void static_var() {
+static int _; // expected-note {{previous definition is here}} \
+  // expected-note {{candidate}}
+static int _; // expected-error {{redefinition of '_'}}
+int _;// expected-warning {{placeholder variables are a C++2c extension}} \
+  // expected-note {{candidate}}
+_++; // expected-error{{reference to '_' is ambiguous}}
+}
+
+void static_var_2() {
+int _; // expected-note {{previous definition is here}}
+static int _; // expected-error {{redefinition of '_'}}
+}
+
+void bindings() {
+int arr[4] = {0, 1, 2, 3};
+auto [_, _, _, _] = arr; // expected-warning 3{{placeholder variables are a C++2c extension}} \
+ // expected-note 4{{placeholder declared here}}
+_ == 42; // expected-error {{referring to placeholder '_' is not allowed}}
+{
+auto [_, a, b, c] = arr;
+auto [_, _, _, _] = arr; // expected-warning 4{{placeholder variables are a C++2c extension}}
+}
+}
+
+void lambda() {
+(void)[_ = 0, _ = 1] { // expected-warning {{placeholder variables are a C++2c extension}} \
+   // expected-note 4{{placeholder declared here}} \\
+   // expected-warning 2{{placeholder variable has no side effect}}
+(void)_++; // expected-error {{referring to placeholder '_' is not allowed}}
+};
+}
+
+namespace global_var {
+int _; // expected-note {{previous definition is here}}
+int _; // expected-error {{redefinition of '_'}}
+}
+
+namespace global_fun {
+void _();
+void _();
+
+void _() {} // expected-note {{previous definition is here}}
+void _() {} // expected-error {{redefinition of '_'}}
+void _(int){};
+}
+
+void extern_test() {
+extern int _;
+extern int _; // expected-note {{candidate}}
+int _; //expected-note {{candidate}}
+_++; // expected-error {{reference to '_' is ambiguous}}
+}
+
+
+struct Members {
+int _; // expected-note {{placeholder declared here}}
+int _; // expected-warning{{placeholder variables are a C++2c extension}} \
+   // expected-note {{placeholder declared here}}
+void f() {
+_++; // expected-error {{referring to placeholder '_' is not allowed}}
+}
+};
+
+namespace using_ {
+int _; // expected-note {{target of using declaration}}
+void f() {
+int _; // expected-note {{conflicting declaration}}
+_ = 0;
+using using_::_; // expected-error {{target of using declaration conflicts with declaration already in scope}}
+}
+}
+
+
+void call(int);
+void test_param(int _) {}
+void test_params(int _, int _); // expected-error {{redefinition of parameter '_'}} \
+// expected-note {{previous declaration is here}}
+
+template  // expected-error {{declaration of '_' shadows template parameter}} \
+  // expected-note  {{template parameter is declared here}}
+auto i = 0;
+
+template 
+concept C = requires(T _, T _) {  // expected-error {{redefinition of parameter '_'}} \
+// expected-note {{previous declaration is here}}
+T{};
+};
+
+struct S {
+int a;
+};
+