On 11/24/23 03:34, Jakub Jelinek wrote:
On Mon, Sep 18, 2023 at 07:12:40PM +0200, Jakub Jelinek via Gcc-patches wrote:
On Tue, Aug 22, 2023 at 09:39:11AM +0200, Jakub Jelinek via Gcc-patches wrote:
The following patch implements the C++26 P2169R4 paper.
As written in the PR, the patch expects that:
1) https://eel.is/c++draft/expr.prim.lambda.capture#2
"Ignoring appearances in initializers of init-captures, an identifier
or this shall not appear more than once in a lambda-capture."
is adjusted such that name-independent lambda captures with initializers
can violate this rule (but lambda captures which aren't name-independent
can't appear after name-independent ones)
2) https://eel.is/c++draft/class.mem#general-5
"A member shall not be declared twice in the member-specification,
except that"
having an exception that name-independent non-static data member
declarations can appear multiple times (but again, if there is
a member which isn't name-independent, it can't appear after
name-independent ones)
3) it assumes that any name-independent declarations which weren't
previously valid result in the _ lookups being ambiguous, not just
if there are 2 _ declarations in the same scope, in particular the
https://eel.is/c++draft/basic.scope#block-2 mentioned cases
4) it assumes that _ in static function/block scope structured bindings
is never name-independent like in namespace scope structured bindings;
it matches clang behavior and is consistent with e.g. static type _;
not being name-independent both at namespace scope and at function/block
scope
As you preferred in the PR, for local scope bindings, the ambiguous cases
use a TREE_LIST with the ambiguous cases which can often be directly fed
into print_candidates. For member_vec after sorting/deduping, I chose to use
instead OVERLOAD with a new flag but only internally inside of the
member_vec, get_class_binding_direct turns it into a TREE_LIST. There are
2 reasons for that, in order to keep the member_vec binary search fast, I
think it is better to keep OVL_NAME usable on all elements because having
to special case TREE_LIST would slow everything down, and the callers need
to be able to chain the results anyway and so need an unshared TREE_LIST
they can tweak/destroy anyway.
name-independent declarations (even in older standards) will not have
-Wunused{,-variable,-but-set-variable} or -Wshadow* warnings diagnosed, but
unlike e.g. the clang implementation, this patch does diagnose
-Wunused-parameter for parameters with _ names because they aren't
name-independent and one can just omit their name instead.
The patch no longer applies, because of the check_local_shadow changes.
The patch doesn't apply any longer again, because of the P2741R3
static_assert changes, both because I've included the first 2 hunks from
this patch in that commit and because there is a conflict in c_cpp_builtins.
Here is a new version which applies.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2023-11-23 Jakub Jelinek <ja...@redhat.com>
PR c++/110349
gcc/c-family/
* c-cppbuiltin.cc (c_cpp_builtins): Predefine
__cpp_placeholder_variables=202306L for C++26.
gcc/cp/
* cp-tree.h: Implement C++26 P2169R4 - Placeholder variables with no
name.
(OVL_PLACEHOLDER_P): Define.
Throughout this patch let's follow the standard and avoid using the word
"placeholder" to refer to name-independent decls. There are a lot of
other things that could be meant by "placeholder", notably "auto".
(add_capture): Add unsigned * argument.
(name_independent_decl_p): New inline function.
* name-lookup.cc (class name_lookup): Make ambiguous and
add_value members public.
(placeholder_linear_search): New function.
(get_class_binding_direct): Handle member_vec_binary_search
returning OVL_PLACEHOLDER_P OVERLOAD. Use
placeholder_linear_search rather than fields_linear_search
for linear lookup of _ name if !want_type.
(member_name_cmp): Sort name-independent declarations first.
(member_vec_dedup): Handle name-independent declarations.
(pop_local_binding): Handle binding->value being a TREE_LIST for
ambiguous name-independent declarations.
(supplement_binding): Handle name-independent declarations.
(update_binding): Likewise.
(check_local_shadow): Return tree rather than void, normally
NULL_TREE but old for name-independent declarations which used
to conflict with outer scope declaration. Don't emit -Wshadow*
warnings for name-independent declarations.
(pushdecl): Handle name-independent declarations.
(lookup_name): Likewise.
* search.cc (lookup_field_r): Handle nval being a TREE_LIST.
* lambda.cc (build_capture_proxy): Adjust for ___.<number>
names of members.
(add_capture): Add PLACEHOLDER_CNT argument. Use ___.<number>
name rather than ___ for second and following capture with
_ name.
(add_default_capture): Adjust add_capture caller.
* decl.cc (poplevel): Don't warn about name-independent declarations.
(reshape_init_class): If field is a TREE_LIST, emit an ambiguity
error with list of candidates rather than error about non-existing
non-static data member.
* parser.cc (cp_parser_lambda_introducer): Adjust add_capture callers.
Allow name-independent capture redeclarations.
(cp_parser_decomposition_declaration): Set decl_specs.storage_class
to sc_static for static structured bindings.
* pt.cc (tsubst_lambda_expr): Adjust add_capture caller.
gcc/testsuite/
* g++.dg/cpp26/placeholder1.C: New test.
* g++.dg/cpp26/placeholder2.C: New test.
* g++.dg/cpp26/placeholder3.C: New test.
* g++.dg/cpp26/placeholder4.C: New test.
* g++.dg/cpp26/placeholder5.C: New test.
* g++.dg/cpp26/placeholder6.C: New test.
* g++.dg/cpp26/feat-cxx26.C: Add __cpp_placeholder_variables test.
@@ -1843,6 +1909,34 @@ get_class_binding_direct (tree klass, tr
val = member_vec_binary_search (member_vec, lookup);
if (!val)
;
+ else if (TREE_CODE (val) == OVERLOAD && OVL_PLACEHOLDER_P (val))
+ {
+ if (want_type)
+ {
+ while (TREE_CODE (val) == OVERLOAD && OVL_PLACEHOLDER_P (val))
+ val = OVL_CHAIN (val);
+ if (STAT_HACK_P (val))
+ val = STAT_TYPE (val);
+ else if (!DECL_DECLARES_TYPE_P (val))
+ val = NULL_TREE;
+ }
+ else
+ {
Please add a comment here with your rationale from the commit message.
@@ -2479,10 +2609,27 @@ pop_local_binding (tree id, tree decl)
away. */
if (binding->value == decl)
binding->value = NULL_TREE;
+ else if (binding->type == decl)
+ binding->type = NULL_TREE;
else
{
- gcc_checking_assert (binding->type == decl);
- binding->type = NULL_TREE;
+ /* Name-independent variable was found after at least one declaration
+ with the same name. */
+ gcc_assert (TREE_CODE (binding->value) == TREE_LIST);
+ if (TREE_VALUE (binding->value) != decl)
+ {
+ binding->value = nreverse (binding->value);
+ /* Skip over TREE_LISTs added for check_local_shadow detected
This should mention pushdecl, since that's where they're actually added
(which, as mentioned below, I don't understand why).
+ declarations, formerly at the tail, now at the start of the
+ list. */
+ while (TREE_PURPOSE (binding->value) == error_mark_node)
+ binding->value = TREE_CHAIN (binding->value);
+ }
+ gcc_assert (TREE_VALUE (binding->value) == decl);
+ binding->value = TREE_CHAIN (binding->value);
+ while (binding->value
+ && TREE_PURPOSE (binding->value) == error_mark_node)
+ binding->value = TREE_CHAIN (binding->value);
}
if (!binding->value && !binding->type)
@@ -3780,7 +3988,20 @@ pushdecl (tree decl, bool hiding)
if (level->kind != sk_namespace)
{
- check_local_shadow (decl);
+ tree local_shadow = check_local_shadow (decl);
+ if (placeholder_p && local_shadow)
+ {
+ if (cxx_dialect < cxx26 && !placeholder_diagnosed_p)
+ pedwarn (DECL_SOURCE_LOCATION (decl), OPT_Wc__26_extensions,
+ "placeholder variables only available with "
+ "%<-std=c++2c%> or %<-std=gnu++2c%>");
+ placeholder_diagnosed_p = true;
+ if (old == NULL_TREE)
+ {
+ old = build_tree_list (error_mark_node, local_shadow);
+ TREE_TYPE (old) = error_mark_node;
+ }
+ }
This needs a rationale comment; I don't understand what the purpose is.
if (TREE_CODE (decl) == NAMESPACE_DECL)
/* A local namespace alias. */
@@ -7579,7 +7800,30 @@ lookup_name (tree name, LOOK_where where
&& (bool (want & LOOK_want::HIDDEN_LAMBDA)
|| !is_lambda_ignored_entity (iter->value))
&& qualify_lookup (iter->value, want))
- binding = iter->value;
+ {
+ binding = iter->value;
+ if (binding
+ && TREE_CODE (binding) == TREE_LIST
+ && name_independent_decl_p (TREE_VALUE (binding)))
+ {
+ for (tree b = binding; b; b = TREE_CHAIN (b))
+ if (TREE_CHAIN (b) == NULL
+ && TREE_CODE (TREE_VALUE (b)) == OVERLOAD)
+ {
+ /* If the scope has an overload with _ function
+ declarations followed by at least one
+ name-independent declaration, we shouldn't return
+ iter->value but a new TREE_LIST containing the
+ name-independent declaration(s) and functions
+ from the OVERLOAD. */
Why is the iter->value TREE_LIST unsuitable?
--- gcc/testsuite/g++.dg/cpp26/placeholder1.C.jj 2023-09-18
12:42:16.309387948 +0200
+++ gcc/testsuite/g++.dg/cpp26/placeholder1.C 2023-09-18 12:59:09.286891703
+0200
@@ -0,0 +1,194 @@
+// P2169R4 - A nice placeholder with no name
+// { dg-do compile { target c++11 } }
+// { dg-options "-Wunused-variable -Wunused-but-set-variable -Wunused-parameter
-Wshadow" }
+
+int a[3];
+
+void
+foo ()
+{
+ {
+ int _ = 1;
+ ++_;
+ }
+ {
+ int _ = 3;
+ ++_;
+ int _ = 4; // { dg-warning "placeholder variables only available
with" "" { target c++23_down } }
+ }
+ {
+ int _ = 5;
+ --_;
+ int _ = 6; // { dg-warning "placeholder variables only available
with" "" { target c++23_down } }
+ int _ = 7; // { dg-warning "placeholder variables only available
with" "" { target c++23_down } }
+ }
+ {
+ auto [i, j, _] = a; // { dg-warning "structured bindings only available
with" "" { target c++14_down } }
+ ++i;
+ ++_;
+ }
+ {
+ auto [_, _, k] = a; // { dg-warning "placeholder variables only available
with" "" { target c++23_down } }
+ ++k; // { dg-warning "structured bindings only available with"
"" { target c++14_down } .-1 }
+ }
+ {
+ auto [i, j, _] = a; // { dg-warning "structured bindings only available
with" "" { target c++14_down } }
+ auto [_, k, l] = a; // { dg-warning "placeholder variables only available
with" "" { target c++23_down } }
+ ++i; // { dg-warning "structured bindings only available with"
"" { target c++14_down } .-1 }
+ ++l;
+ }
+ {
+ int _;
+ _ = 1;
+ }
+ {
+ int _ = 1;
+ }
+ {
+ int _;
+ }
+ {
+ static int _; // { dg-warning "unused variable" }
+ int _ = 1; // { dg-warning "placeholder variables only available
with" "" { target c++23_down } }
+ }
+ {
+ extern int _ (int);
+ extern long _ (long);
+ extern float _ (float);
+ int _ = 1; // { dg-warning "placeholder variables only available
with" "" { target c++23_down } }
+ }
+ {
+ extern double _ (double);
+ extern short _ (short);
+ int _ = 1; // { dg-warning "placeholder variables only available
with" "" { target c++23_down } }
+ int _ = 2; // { dg-warning "placeholder variables only available
with" "" { target c++23_down } }
+ }
+ {
+ int _ = 1;
+ {
+ int _ = 2;
+ ++_;
+ }
+ {
+ static int _ = 3; // { dg-warning "declaration of '_' shadows
a previous local" }
Perhaps we don't want to warn if either of the declarations is
name-independent?
--- gcc/testsuite/g++.dg/cpp26/placeholder2.C.jj 2023-09-18
12:42:16.309387948 +0200
+++ gcc/testsuite/g++.dg/cpp26/placeholder2.C 2023-09-18 12:42:16.309387948
+0200
@@ -0,0 +1,171 @@
+// P2169R4 - A nice placeholder with no name
+// { dg-do compile { target c++11 } }
+// { dg-options "" }
+
+int a[3];
+
+void
+foo ()
+{
+ {
+ extern int _ (int);
+ int _ = 2; // { dg-warning "placeholder variables only available
with" "" { target c++23_down } }
+ extern long _ (long); // { dg-error "redeclared as different kind of
entity" }
+ }
+ {
+ int _ = 3;
+ extern int _ (int); // { dg-error "redeclared as different kind
of entity" }
+ }
+ {
+ int _ = 4;
+ static int _ = 5; // { dg-error "redeclaration of 'int _'" }
+ }
+ {
+ int _ = 6;
+ int _ = 7; // { dg-warning "placeholder variables only available
with" "" { target c++23_down } }
+ ++_; // { dg-error "reference to '_' is ambiguous" }
+ }
+ {
+ int _ = 8;
+ int _ = 9; // { dg-warning "placeholder variables only available
with" "" { target c++23_down } }
+ int _ = 10; // { dg-warning "placeholder variables only available
with" "" { target c++23_down } }
+ ++_; // { dg-error "reference to '_' is ambiguous" }
+ }
+ {
+ static int _ = 11;
+ static int _ = 12; // { dg-error "redeclaration of 'int _'" }
+ int _ = 13; // { dg-warning "placeholder variables only available
with" "" { target c++23_down } }
+ }
+ {
+ extern int _ (int);
+ extern long _ (long);
+ extern float _ (float);
+ int _ = 1; // { dg-warning "placeholder variables only available
with" "" { target c++23_down } }
+ ++_; // { dg-error "reference to '_' is ambiguous" }
+ }
+ {
+ extern double _ (double);
+ extern short _ (short);
+ int _ = 1; // { dg-warning "placeholder variables only available
with" "" { target c++23_down } }
+ ++_; // { dg-error "reference to '_' is ambiguous" }
+ int _ = 2; // { dg-warning "placeholder variables only available
with" "" { target c++23_down } }
+ ++_; // { dg-error "reference to '_' is ambiguous" }
+ }
+ {
+ auto [i, _, _] = a; // { dg-warning "placeholder variables only available
with" "" { target c++23_down } }
+ // { dg-warning "structured bindings only available with"
"" { target c++14_down } .-1 }
+ ++_; // { dg-error "reference to '_' is ambiguous" }
+ }
+ {
+ auto [i, j, _] = a; // { dg-warning "structured bindings only available
with" "" { target c++14_down } }
+ auto [k, _, l] = a; // { dg-warning "placeholder variables only available
with" "" { target c++23_down } }
+ // { dg-warning "structured bindings only available with"
"" { target c++14_down } .-1 }
+ ++_; // { dg-error "reference to '_' is ambiguous" }
+ }
+ {
+ static auto [i, _, _] = a; // { dg-error "redeclaration of 'auto _'" }
Would it be easy to add a note explaining that static static bindings
aren't name-independent?
+ // { dg-warning "structured bindings only available with"
"" { target c++14_down } .-1 }
+ // { dg-warning "structured binding declaration can be
'static' only in" "" { target c++17_down } .-2 }
+ }
+}
+
+int
+bar (int _ = 0)
+{
+ int _ = 1; // { dg-warning "placeholder variables only available
with" "" { target c++23_down } }
+ ++_; // { dg-error "reference to '_' is ambiguous" }
+ return 0;
+}
+
+void
+baz ()
+{
+ if (int _ = bar ())
+ {
+ int _ = 6; // { dg-warning "placeholder variables only available
with" "" { target c++23_down } }
+ ++_; // { dg-error "reference to '_' is ambiguous" }
+ }
+ else
+ {
+ int _ = 7; // { dg-warning "placeholder variables only available
with" "" { target c++23_down } }
+ ++_; // { dg-error "reference to '_' is ambiguous" }
+ }
+ while (int _ = bar ())
+ {
+ int _ = 8; // { dg-warning "placeholder variables only available
with" "" { target c++23_down } }
+ ++_; // { dg-error "reference to '_' is ambiguous" }
+ }
+ for (int _ = bar (); _; ++_)
+ {
+ int _ = 9; // { dg-warning "placeholder variables only available
with" "" { target c++23_down } }
+ ++_; // { dg-error "reference to '_' is ambiguous" }
+ }
+}
+
+namespace A
+{
+ int _ = 1;
+ int _ = 1; // { dg-error "redefinition of 'int A::_'" }
Similarly, a note explaining that namespace-scope variables aren't
name-independent.
+}
+
+namespace B
+{
+ auto [_, _, _] = a; // { dg-error "redefinition of 'auto B::_'" }
+ // { dg-warning "structured bindings only available with"
"" { target c++14_down } .-1 }
+}
+
+void
+qux ()
+{
+ auto c = [_ = 2, _ = 3] () { // { dg-warning "placeholder variables only available
with" "" { target c++23_down } }
+ // { dg-warning "lambda capture initializers only available
with" "" { target c++11_down } .-1 }
+ (void) _; // { dg-error "reference to '_' is ambiguous" }
+ };
+ {
+ int _ = 4;
+ auto d = [_, _ = 5] () { // { dg-warning "placeholder variables only available
with" "" { target c++23_down } }
+ // { dg-warning "lambda capture initializers only available
with" "" { target c++11_down } .-1 }
+ (void) _; // { dg-error "reference to '_' is
ambiguous" }
+ };
+ }
+ auto e = [_ = 1] (int _) {}; // { dg-warning "lambda capture initializers only available
with" "" { target c++11_down } }
+} // { dg-error "lambda parameter '_' previously declared as a
capture" "" { target *-*-* } .-1 }
+
+void
+corge (int _, int _) // { dg-error "redefinition of 'int _'" }
...likewise parameters.
