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

aaron.ballman added a comment. In D153536#4577187 , @vitalybuka wrote: > In D153536#4576683 , @aaron.ballman > wrote: > >> In D153536#4571435 , @vitalybuka >> wrote:

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

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

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

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

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

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

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

aaron.ballman added a comment. In D153536#4570561 , @vitalybuka wrote: > In D153536#4570534 , @aaron.ballman > wrote: > >> In D153536#4570513 , @vitalybuka >> wrote:

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

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:

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

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

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

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

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

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?

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

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

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

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

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

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:

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

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 +

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

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

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

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

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

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

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

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

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

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,

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

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

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

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/

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

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

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

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

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

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:

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

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

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

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

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

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:

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

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

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

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

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

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

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

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;

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

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

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

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

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

dblaikie added a comment. In D153536#4475308 , @cor3ntin wrote: > In D153536#4475240 , @dblaikie > wrote: > >> In D153536#4475199 , @cor3ntin >> wrote: >> >>> In

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

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:

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

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

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

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

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

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

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

cor3ntin added a comment. In D153536#4475240 , @dblaikie wrote: > In D153536#4475199 , @cor3ntin > wrote: > >> In D153536#4474733 , @dblaikie >> wrote: >> >>> Maybe try

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

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

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

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/

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

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

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

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:

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

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

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

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

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

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 -

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

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 _, _; }

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

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:

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

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/

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

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

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

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

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

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;

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

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

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

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

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

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/

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

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

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

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

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

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:

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

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

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

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:

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

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

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

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

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

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:

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

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

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

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:

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

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

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

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

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

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

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

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

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

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

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

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