From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 655E03858C60 for ; Tue, 26 Oct 2021 20:58:16 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 655E03858C60 Received: from mail-qk1-f199.google.com (mail-qk1-f199.google.com [209.85.222.199]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-520-mN2YJ3S-OYGy-BVi1qwT0w-1; Tue, 26 Oct 2021 16:58:14 -0400 X-MC-Unique: mN2YJ3S-OYGy-BVi1qwT0w-1 Received: by mail-qk1-f199.google.com with SMTP id j17-20020a05620a0a5100b0045f8ed4f72fso267156qka.1 for ; Tue, 26 Oct 2021 13:58:14 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:subject :content-language:to:cc:references:from:in-reply-to :content-transfer-encoding; bh=WHLiYsiNUnwu8OA4oRfgFkeIN4TGJ+HHEpQz7UXLvtQ=; b=GTW1nMYImNSTA4A/JYB08NnVXb+KygTxiRyFZviaJtrgHTAajAoL2vEwKVctipcHeM /gQxAlNFGj+xK1iCUsilFeNWv/t8BiH1tsXZq6ds+cwXLsn+82VxqWUjp1Bh+Sg0waOd R7jG0LIWRALcLhOgLLznHKQTTPNzAEok34J0ZyE50bGLhhUq7M4jpynkE+Gx6Cfw+1L6 +Yx5/qpHTG1XGw47TTbedx+dUKqJEXY7LAt2S9WOuB5Te/Vt/M8xl4eu+yQB5f5fskJa kWD/kyESg0TuuZ3Jhvh+0pe8XtvsjafNCoGpuCTXifNZ+dGt5d+0ZIPo+AOHwtzwXLMg JXVg== X-Gm-Message-State: AOAM532LjZvm0HpBwrGDzYVDcRvQqogxOlOayl8w5kx3CNlCYeNa9bTb PW1rolVPWg80EIXOHydFR0P5yr2niIogS7jTp9dqhdoLIw7jAXv4034ywSu3JNm4boEUWY4R4VZ 1VGQNqjsgs5T5C73hNg== X-Received: by 2002:a37:8d8:: with SMTP id 207mr21062587qki.220.1635281893808; Tue, 26 Oct 2021 13:58:13 -0700 (PDT) X-Google-Smtp-Source: ABdhPJxYkHvPep5fBcyl6TdhYJclCR2vsTPyCNlevJXmr9xhemniijeN6jZlBbp/LLJsYrUmNaMIDg== X-Received: by 2002:a37:8d8:: with SMTP id 207mr21062552qki.220.1635281893255; Tue, 26 Oct 2021 13:58:13 -0700 (PDT) Received: from [192.168.1.149] (130-44-159-43.s15913.c3-0.arl-cbr1.sbo-arl.ma.cable.rcncustomer.com. [130.44.159.43]) by smtp.gmail.com with ESMTPSA id g1sm10936863qkd.89.2021.10.26.13.58.12 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 26 Oct 2021 13:58:12 -0700 (PDT) Message-ID: Date: Tue, 26 Oct 2021 16:58:11 -0400 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.2.0 Subject: Re: [PATCH, v2] c++: Diagnose taking address of an immediate member function [PR102753] To: Jakub Jelinek Cc: gcc-patches@gcc.gnu.org References: <20211018081258.GV304296@tucnak> <40b79f8d-dd22-a2f4-f63d-802d67395444@redhat.com> <20211019120021.GL304296@tucnak> <20211021111742.GG304296@tucnak> From: Jason Merrill In-Reply-To: <20211021111742.GG304296@tucnak> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, NICE_REPLY_A, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 26 Oct 2021 20:58:18 -0000 On 10/21/21 07:17, Jakub Jelinek wrote: > On Wed, Oct 20, 2021 at 07:16:44PM -0400, Jason Merrill wrote: >> or an unevaluated operand, or a subexpression of an immediate invocation. >> >> Hmm...that suggests that in consteval23.C, bar(foo) should also be OK, > > Ouch. That opens a big can of worms, see below. > >> because 'foo' is a subexpression of an immediate invocation. We can handle >> that by... >> >>> + >>> +bool >>> +in_immediate_context () >>> +{ >>> + return (cp_unevaluated_operand != 0 >>> + || (current_function_decl != NULL_TREE >>> + && DECL_IMMEDIATE_FUNCTION_P (current_function_decl)) >>> + || (current_binding_level->kind == sk_function_parms >>> + && current_binding_level->immediate_fn_ctx_p) >>> + || in_consteval_if_p); >>> +} >>> + >>> /* Return true if a call to FN with number of arguments NARGS >>> is an immediate invocation. */ >>> @@ -9451,6 +9459,12 @@ build_over_call (struct z_candidate *can >>> } >>> /* Default arguments */ >>> + bool save_in_consteval_if_p = in_consteval_if_p; >>> + /* If the call is immediate function invocation, make sure >>> + taking address of immediate functions is allowed in default >>> + arguments. */ >>> + if (immediate_invocation_p (STRIP_TEMPLATE (fn), nargs)) >>> + in_consteval_if_p = true; >> >> ...moving this earlier in build_over_call, e.g. shortly after >> not_really_used: >> >> You can also use make_temp_override. > > make_temp_override can't be used, because in_consteval_if_p is BOOL_BITFIELD > under the hood and binding a reference to a bit-field is not valid. > > In the patch I've used a specialized in_consteval_if_p_temp_override class > instead. And it either needs to be restored after all the argument > processing, because we call immediate_invocation_p again and because of > the in_consteval_if_p override it is then false (the patch calls icip.reset > ()), or we'd need to remember the result of the first immediate_invocation_p > call in some bool temporary and reuse that later. > Updated patch below. > > This fixes the static_assert (bar (foo) == 42); case newly added to > consteval23.C, but unfortunately that isn't enough. > Consider incremental patch to the testcase: > --- gcc/testsuite/g++.dg/cpp2a/consteval23.C.jj 2021-10-21 11:32:48.570586881 +0200 > +++ gcc/testsuite/g++.dg/cpp2a/consteval23.C 2021-10-21 12:47:42.281769906 +0200 > @@ -2,6 +2,7 @@ > // { dg-do compile { target c++20 } } > > consteval int foo () { return 42; } > +constexpr auto baz (int (*fn) ()) { return fn; } > > consteval int > bar (int (*fn) () = foo) > @@ -11,3 +12,5 @@ bar (int (*fn) () = foo) > > static_assert (bar () == 42); > static_assert (bar (foo) == 42); > +static_assert (bar (&foo) == 42); > +static_assert (bar (baz (&foo)) == 42); > > If even the last two lines are supposed to be valid, then I'm afraid we > need to reconsider the errors performed in cp_build_addr_expr_1, because > when that is called, we often don't know if we are in a subexpression > of immediate invocation or not, whether it is because we are in template, > something non-type dependent but other argumeents of the function call are > type dependent, or because it is an overloaded function and some overloads > are consteval and others are not and we are deep in a middle of some > argument parsing and other arguments haven't been parsed at all yet, etc. > > I'm afraid I don't have a good idea where to move that diagnostic to though, > it would need to be done somewhere where we are certain we aren't in a > subexpression of immediate invocation. Given statement expressions, even > diagnostics after parsing whole statements might not be good enough, e.g. > void > qux () > { > static_assert (bar (({ constexpr auto a = 1; foo; })) == 42); > } I suppose (a wrapper for) fold_build_cleanup_point_expr would be a possible place to check, since that's called for full-expressions. > But if we e.g. diagnose it somewhere after parsing the whole function (and > for templates only after instantiating it) plus where we handle > non-automatic variable initializers etc., will we handle all spots where > we should diagnose it? It better should be done before cp_fold... > In whatever spot is right doing something similar to what > cp_walk_tree find_immediate_fndecl, does, except probably error out on each > case we see (with the right locus) instead of just finding the first match. > > Note, trying clang++ on godbolt on the consteval23.C testcase with the above > ammendments, it diagnoses the = foo default argument, that seems to be a > clang bug to me, but if I comment out the > static_assert (bar () == 42); test and comment out the default argument > bar (int (*fn) () /* = foo */) > then it succeeds. It even accepts the above qux with statement expression. This patch is OK. > 2021-10-21 Jakub Jelinek > > PR c++/102753 > * cp-tree.h (saved_scope): Document that consteval_if_p member > is also set while processing immediate invocation. > (in_immediate_context): Declare. > * call.c (in_immediate_context): New function. > (immediate_invocation_p): Use it. > (struct in_consteval_if_p_temp_override): New class. > (build_over_call): Temporarily set in_consteval_if_p for processing > immediate invocation arguments. > * typeck.c (cp_build_addr_expr_1): Diagnose taking address of > an immediate method. Use t instead of TREE_OPERAND (arg, 1). > Use in_immediate_context function. > * constexpr.c (find_immediate_fndecl): Handle PTRMEM_CST > which refers to immediate function decl. > > * g++.dg/cpp2a/consteval13.C: Don't expect errors. > * g++.dg/cpp2a/consteval20.C: New test. > * g++.dg/cpp2a/consteval21.C: New test. > * g++.dg/cpp2a/consteval22.C: New test. > * g++.dg/cpp2a/consteval23.C: New test. > * g++.dg/cpp23/consteval-if11.C: New test. > > --- gcc/cp/cp-tree.h.jj 2021-10-20 08:35:26.850254029 +0200 > +++ gcc/cp/cp-tree.h 2021-10-21 11:36:51.636186234 +0200 > @@ -1825,7 +1825,8 @@ struct GTY(()) saved_scope { > if-statement. */ > BOOL_BITFIELD discarded_stmt : 1; > /* Nonzero if we are parsing or instantiating the compound-statement > - of consteval if statement. */ > + of consteval if statement. Also set while processing an immediate > + invocation. */ > BOOL_BITFIELD consteval_if_p : 1; > > int unevaluated_operand; > @@ -6547,6 +6548,7 @@ extern tree perform_direct_initializatio > tsubst_flags_t); > extern vec *resolve_args (vec*, tsubst_flags_t); > extern tree in_charge_arg_for_name (tree); > +extern bool in_immediate_context (); > extern tree build_cxx_call (tree, int, tree *, > tsubst_flags_t, > tree = NULL_TREE); > --- gcc/cp/call.c.jj 2021-10-20 08:35:26.808254618 +0200 > +++ gcc/cp/call.c 2021-10-21 12:10:42.591798807 +0200 > @@ -9025,6 +9025,20 @@ build_trivial_dtor_call (tree instance, > instance, clobber); > } > > +/* Return true if in an immediate function context, or an unevaluated operand, > + or a subexpression of an immediate invocation. */ > + > +bool > +in_immediate_context () > +{ > + return (cp_unevaluated_operand != 0 > + || (current_function_decl != NULL_TREE > + && DECL_IMMEDIATE_FUNCTION_P (current_function_decl)) > + || (current_binding_level->kind == sk_function_parms > + && current_binding_level->immediate_fn_ctx_p) > + || in_consteval_if_p); > +} > + > /* Return true if a call to FN with number of arguments NARGS > is an immediate invocation. */ > > @@ -9033,18 +9047,25 @@ immediate_invocation_p (tree fn, int nar > { > return (TREE_CODE (fn) == FUNCTION_DECL > && DECL_IMMEDIATE_FUNCTION_P (fn) > - && cp_unevaluated_operand == 0 > - && (current_function_decl == NULL_TREE > - || !DECL_IMMEDIATE_FUNCTION_P (current_function_decl)) > - && (current_binding_level->kind != sk_function_parms > - || !current_binding_level->immediate_fn_ctx_p) > - && !in_consteval_if_p > + && !in_immediate_context () > /* As an exception, we defer std::source_location::current () > invocations until genericization because LWG3396 mandates > special behavior for it. */ > && (nargs > 1 || !source_location_current_p (fn))); > } > > +/* temp_override for in_consteval_if_p, which can't use make_temp_override > + because it is a bitfield. */ > + > +struct in_consteval_if_p_temp_override { > + bool save_in_consteval_if_p; > + in_consteval_if_p_temp_override () > + : save_in_consteval_if_p (in_consteval_if_p) {} > + void reset () { in_consteval_if_p = save_in_consteval_if_p; } > + ~in_consteval_if_p_temp_override () > + { reset (); } > +}; > + > /* Subroutine of the various build_*_call functions. Overload resolution > has chosen a winning candidate CAND; build up a CALL_EXPR accordingly. > ARGS is a TREE_LIST of the unconverted arguments to the call. FLAGS is a > @@ -9254,6 +9275,12 @@ build_over_call (struct z_candidate *can > nargs = parmlen; > argarray = XALLOCAVEC (tree, nargs); > > + in_consteval_if_p_temp_override icip; > + /* If the call is immediate function invocation, make sure > + taking address of immediate functions is allowed in its arguments. */ > + if (immediate_invocation_p (STRIP_TEMPLATE (fn), nargs)) > + in_consteval_if_p = true; > + > /* The implicit parameters to a constructor are not considered by overload > resolution, and must be of the proper type. */ > if (DECL_CONSTRUCTOR_P (fn)) > @@ -9498,6 +9525,7 @@ build_over_call (struct z_candidate *can > > gcc_assert (j <= nargs); > nargs = j; > + icip.reset (); > > /* Avoid performing argument transformation if warnings are disabled. > When tf_warning is set and at least one of the warnings is active > --- gcc/cp/typeck.c.jj 2021-10-20 08:35:26.865253819 +0200 > +++ gcc/cp/typeck.c 2021-10-21 11:29:43.040182567 +0200 > @@ -6773,9 +6773,19 @@ cp_build_addr_expr_1 (tree arg, bool str > return error_mark_node; > } > > + if (TREE_CODE (t) == FUNCTION_DECL > + && DECL_IMMEDIATE_FUNCTION_P (t) > + && !in_immediate_context ()) > + { > + if (complain & tf_error) > + error_at (loc, "taking address of an immediate function %qD", > + t); > + return error_mark_node; > + } > + > type = build_ptrmem_type (context_for_name_lookup (t), > TREE_TYPE (t)); > - t = make_ptrmem_cst (type, TREE_OPERAND (arg, 1)); > + t = make_ptrmem_cst (type, t); > return t; > } > > @@ -6800,9 +6810,7 @@ cp_build_addr_expr_1 (tree arg, bool str > tree stripped_arg = tree_strip_any_location_wrapper (arg); > if (TREE_CODE (stripped_arg) == FUNCTION_DECL > && DECL_IMMEDIATE_FUNCTION_P (stripped_arg) > - && cp_unevaluated_operand == 0 > - && (current_function_decl == NULL_TREE > - || !DECL_IMMEDIATE_FUNCTION_P (current_function_decl))) > + && !in_immediate_context ()) > { > if (complain & tf_error) > error_at (loc, "taking address of an immediate function %qD", > --- gcc/cp/constexpr.c.jj 2021-10-20 08:35:26.825254380 +0200 > +++ gcc/cp/constexpr.c 2021-10-21 11:29:43.041182553 +0200 > @@ -7276,6 +7276,10 @@ find_immediate_fndecl (tree *tp, int */* > { > if (TREE_CODE (*tp) == FUNCTION_DECL && DECL_IMMEDIATE_FUNCTION_P (*tp)) > return *tp; > + if (TREE_CODE (*tp) == PTRMEM_CST > + && TREE_CODE (PTRMEM_CST_MEMBER (*tp)) == FUNCTION_DECL > + && DECL_IMMEDIATE_FUNCTION_P (PTRMEM_CST_MEMBER (*tp))) > + return PTRMEM_CST_MEMBER (*tp); > return NULL_TREE; > } > > --- gcc/testsuite/g++.dg/cpp2a/consteval13.C.jj 2020-01-12 11:54:37.140402440 +0100 > +++ gcc/testsuite/g++.dg/cpp2a/consteval13.C 2021-10-21 12:24:33.935180868 +0200 > @@ -10,8 +10,8 @@ void > foo () > { > auto qux = [] (fnptr a = quux ()) consteval { return a (); }; > - constexpr auto c = qux (baz); // { dg-error "28:taking address of an immediate function" } > - constexpr auto d = qux (bar); // { dg-error "28:taking address of an immediate function" } > + constexpr auto c = qux (baz); > + constexpr auto d = qux (bar); > static_assert (c == 1); > static_assert (d == 42); > } > --- gcc/testsuite/g++.dg/cpp2a/consteval20.C.jj 2021-10-21 11:29:43.041182553 +0200 > +++ gcc/testsuite/g++.dg/cpp2a/consteval20.C 2021-10-21 11:29:43.041182553 +0200 > @@ -0,0 +1,24 @@ > +// PR c++/102753 > +// { dg-do compile { target c++20 } } > + > +struct S { > + consteval int foo () const { return 42; } > +}; > + > +constexpr S s; > + > +int > +bar () > +{ > + return (s.*&S::foo) (); // { dg-error "taking address of an immediate function" } > +} > + > +constexpr auto a = &S::foo; // { dg-error "taking address of an immediate function" } > + > +consteval int > +baz () > +{ > + return (s.*&S::foo) (); > +} > + > +static_assert (baz () == 42); > --- gcc/testsuite/g++.dg/cpp2a/consteval21.C.jj 2021-10-21 11:29:43.041182553 +0200 > +++ gcc/testsuite/g++.dg/cpp2a/consteval21.C 2021-10-21 11:29:43.041182553 +0200 > @@ -0,0 +1,35 @@ > +// PR c++/102753 > +// { dg-do compile { target c++20 } } > + > +struct S { > + constexpr S () : s (0) {} > + consteval int foo () { return 1; } > + virtual consteval int bar () { return 2; } > + int s; > +}; > + > +consteval int foo () { return 42; } > + > +consteval int > +bar (int (*fn) () = &foo) > +{ > + return fn (); > +} > + > +consteval int > +baz (int (S::*fn) () = &S::foo) > +{ > + S s; > + return (s.*fn) (); > +} > + > +consteval int > +qux (int (S::*fn) () = &S::bar) > +{ > + S s; > + return (s.*fn) (); > +} > + > +static_assert (bar () == 42); > +static_assert (baz () == 1); > +static_assert (qux () == 2); > --- gcc/testsuite/g++.dg/cpp2a/consteval22.C.jj 2021-10-21 11:29:43.041182553 +0200 > +++ gcc/testsuite/g++.dg/cpp2a/consteval22.C 2021-10-21 11:29:43.041182553 +0200 > @@ -0,0 +1,34 @@ > +// PR c++/102753 > +// { dg-do compile { target c++20 } } > + > +struct S { > + constexpr S () : s (0) {} > + consteval int foo () { return 1; } > + virtual consteval int bar () { return 2; } > + int s; > +}; > +typedef int (S::*P) (); > + > +consteval P > +foo () > +{ > + return &S::foo; > +} > + > +consteval P > +bar () > +{ > + return &S::bar; > +} > + > +consteval int > +baz () > +{ > + S s; > + return (s.*(foo ())) () + (s.*(bar ())) (); > +} > + > +static_assert (baz () == 3); > + > +constexpr P a = foo (); // { dg-error "immediate evaluation returns address of immediate function" } > +constexpr P b = bar (); // { dg-error "immediate evaluation returns address of immediate function" } > --- gcc/testsuite/g++.dg/cpp2a/consteval23.C.jj 2021-10-21 11:29:43.041182553 +0200 > +++ gcc/testsuite/g++.dg/cpp2a/consteval23.C 2021-10-21 11:32:48.570586881 +0200 > @@ -0,0 +1,13 @@ > +// PR c++/102753 > +// { dg-do compile { target c++20 } } > + > +consteval int foo () { return 42; } > + > +consteval int > +bar (int (*fn) () = foo) > +{ > + return fn (); > +} > + > +static_assert (bar () == 42); > +static_assert (bar (foo) == 42); > --- gcc/testsuite/g++.dg/cpp23/consteval-if11.C.jj 2021-10-21 11:29:43.041182553 +0200 > +++ gcc/testsuite/g++.dg/cpp23/consteval-if11.C 2021-10-21 11:29:43.041182553 +0200 > @@ -0,0 +1,27 @@ > +// PR c++/102753 > +// { dg-do compile { target c++20 } } > +// { dg-options "" } > + > +struct S { > + constexpr S () : s (0) {} > + consteval int foo () { return 1; } > + virtual consteval int bar () { return 2; } > + int s; > +}; > + > +consteval int foo () { return 42; } > + > +constexpr int > +bar () > +{ > + if consteval { // { dg-warning "'if consteval' only available with" "" { target c++20_only } } > + int (*fn1) () = foo; > + int (S::*fn2) () = &S::foo; > + int (S::*fn3) () = &S::bar; > + S s; > + return fn1 () + (s.*fn2) () + (s.*fn3) (); > + } > + return 0; > +} > + > +static_assert (bar () == 45); > > > Jakub >