Bug 110349 - [C++26] P2169R4 - Placeholder variables with no name
Summary: [C++26] P2169R4 - Placeholder variables with no name
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 14.0
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks: c++26-core
  Show dependency treegraph
 
Reported: 2023-06-21 16:27 UTC by Marek Polacek
Modified: 2023-11-30 08:16 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2023-06-22 00:00:00


Attachments
gcc14-pr110349-wip.patch (3.90 KB, patch)
2023-08-11 15:32 UTC, Jakub Jelinek
Details | Diff
gcc14-pr110349-wip.patch (5.76 KB, patch)
2023-08-18 15:01 UTC, Jakub Jelinek
Details | Diff
gcc14-pr110349-wip.patch (8.53 KB, patch)
2023-08-18 18:34 UTC, Jakub Jelinek
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Marek Polacek 2023-06-21 16:27:02 UTC
See <https://wg21.link/P2169R4>.
Comment 1 Andrew Pinski 2023-06-22 06:03:05 UTC
Confirmed.
Comment 2 Jakub Jelinek 2023-08-11 15:32:04 UTC
Created attachment 55725 [details]
gcc14-pr110349-wip.patch

I've played with this a little bit so far, but am a little bit stuck.
When lookup_name reports error on ambiguous _ because 2+ declarations with the
_ name are in the same scope (counting function overloads as one obviously),
the error is reported too many times, even from tentative parsing.
And also unsure if it should return error_mark_node in that case or not, if I return error_mark_node I get some extra error recovery errors as well (though if the error is avoided during tentative parsing, I guess we have to return error_mark_node in that case.
Comment 3 Jason Merrill 2023-08-11 19:08:35 UTC
Yeah, we don't want to give errors in lookup_name, pretty much for the reasons you found.  cp_parser_lookup_name gives an ambiguity error when lookup_name returns a TREE_LIST, so I think it makes sense to represent the case of multiple _ declarations as a TREE_LIST rather than OVERLOAD.  Then I think you don't need PLACEHOLDER_AMBIGUOUS_BINDING_P because you can see that from the binding being a TREE_LIST.

I think find_last_decl should return nothing for a name-independent decl, and give up on a TREE_LIST.
Comment 4 Jakub Jelinek 2023-08-18 15:01:15 UTC
Created attachment 55757 [details]
gcc14-pr110349-wip.patch

Thanks, made some progress with that.
I wonder if the paper isn't incomplete though.
My understanding of the intent is that say
void foo () { auto a = [_ = 1, _ = 2] () {}; }
is valid, similarly
struct S { int _ = 1; int _ = 2; };
(and it seems the clang implementation allows that),
but we still have
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."
and
https://eel.is/c++draft/class.mem#general-5
"A member shall not be declared twice in the member-specification, except that"
and nothing mentioning the name-independent exception in either case.
Another thing is we have that
https://eel.is/c++draft/basic.scope.block#2
spot which has been changed by the paper, so
void baz (int _) { int _ = 1; }
void qux () { if (int _ = 2) { int _ = 3; } }
etc. cases are valid which have been invalid before, but the important question
is if ++_; is allowed after 1; and/or 3;
https://eel.is/c++draft/basic.scope.scope#6
has the note:
"An id-expression that names a unique name-independent declaration is usable until an additional declaration of the same name is introduced in the same scope ([basic.lookup.general])."
but does that apply here?
https://eel.is/c++draft/basic.scope.block#2 seems to talk about the behavior
in the same scope, but aren't the scopes different here?
Seems clang rejects the baz case with ++_; added after 1; (and the WIP patch does too), but doesn't reject ++_; added after 3; (while the WIP patch does).
Comment 5 Jakub Jelinek 2023-08-18 15:33:11 UTC
Also, is the structured binding in
void corge ()
{
  static int a[2];
  static auto [_, _] = a;
}
inhabitating namespace scope (so it is correct to reject it?  clang does, the WIP patch does as well).
Comment 6 Jason Merrill 2023-08-18 16:14:43 UTC
(In reply to Jakub Jelinek from comment #4)
> "A member shall not be declared twice in the member-specification, except
> that"
> and nothing mentioning the name-independent exception in either case.

Agreed, this seems like a missed edit, and we should accept those.

> Seems clang rejects the baz case with ++_; added after 1; (and the WIP patch
> does too), but doesn't reject ++_; added after 3; (while the WIP patch does).

I agree that these should be rejected and the wording needs improvement to specify that.

(In reply to Jakub Jelinek from comment #5)
> Also, is the structured binding in
> void corge ()
> {
>   static int a[2];
>   static auto [_, _] = a;
> }
> inhabitating namespace scope (so it is correct to reject it?  clang does,
> the WIP patch does as well).

No, it inhabits block/function scope.
Comment 7 Jakub Jelinek 2023-08-18 18:34:13 UTC
Created attachment 55759 [details]
gcc14-pr110349-wip.patch

Some further progress, with lambda capture handling fixed so that it doesn't emit the bogus errors and very basic start of non-static data member support.
In that area not really sure what to do, because presumably we want to fail
_ member lookups if it is ambiguous but there is tons of internal lookups
that will likely have to work somehow.  And there is the binary search on member_vec vs. linear search on member_vec vs. field search, and anonymous aggregates etc.

(In reply to Jason Merrill from comment #6)
> No, it inhabits block/function scope.

Ok, so should block/function scope static structured binding act allow (multiple) placeholders or should the standard wording be changed even for that case?

If I disable the last hunk in parser.cc in this patch, then #c6 works, but
void freddy ()
{
  static int a[2];
  static auto [_, _] = a;
  static auto [_, _] = a;
}
fails to assemble as there is _ZNDC1_1_EE emitted twice.
Comment 8 GCC Commits 2023-11-30 08:11:44 UTC
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:6c9973e46bdb4496b5af8a7170269e91e36ad35a

commit r14-5991-g6c9973e46bdb4496b5af8a7170269e91e36ad35a
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Thu Nov 30 09:09:21 2023 +0100

    c++: Implement C++26 P2169R4 - Placeholder variables with no name [PR110349]
    
    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.
    
    2023-11-30  Jakub Jelinek  <jakub@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_NAME_INDEPENDENT_DECL_P): Define.
            (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.
            (name_independent_linear_search): New function.
            (get_class_binding_direct): Handle member_vec_binary_search
            returning OVL_NAME_INDEPENDENT_DECL_P OVERLOAD.  Use
            name_independent_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.
            * 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 NAME_INDEPENDENT_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.
            (duplicate_decls): If in C++26 a _ named declaration conflicts with
            earlier declarations, emit explaining note why the new declaration
            is not name-independent.
            (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/name-independent-decl1.C: New test.
            * g++.dg/cpp26/name-independent-decl2.C: New test.
            * g++.dg/cpp26/name-independent-decl3.C: New test.
            * g++.dg/cpp26/name-independent-decl4.C: New test.
            * g++.dg/cpp26/name-independent-decl5.C: New test.
            * g++.dg/cpp26/name-independent-decl6.C: New test.
            * g++.dg/cpp26/feat-cxx26.C: Add __cpp_placeholder_variables test.
Comment 9 Jakub Jelinek 2023-11-30 08:16:59 UTC
Implemented for GCC 14.