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.129.124]) by sourceware.org (Postfix) with ESMTPS id A1E8F395A05D for ; Wed, 16 Nov 2022 13:14:00 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org A1E8F395A05D Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1668604440; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=AhpqcvSGEO68JyVBFTta8knYTnbLMdJiBN+oEtI45Ys=; b=hjLQNj8qkzk91qpUxf73CTt15UBFdZybZN81hmg/bUW/VbvhHdwdLeDY6rLXbJ9ZGtNbrO QGtgJ2ArQUNMDvaEIOXukNhs+DBRrb4iko5p4WubEqUiJIcmgtoJljeVyfePLZ4hcWXGq9 N2wNLVz32Lc4GgiQpoXnSNhEmzikTmQ= Received: from mail-qk1-f198.google.com (mail-qk1-f198.google.com [209.85.222.198]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-187-3jN596YBMrus6QHuEdu0og-1; Wed, 16 Nov 2022 08:13:58 -0500 X-MC-Unique: 3jN596YBMrus6QHuEdu0og-1 Received: by mail-qk1-f198.google.com with SMTP id j13-20020a05620a410d00b006e08208eb31so17135450qko.3 for ; Wed, 16 Nov 2022 05:13:58 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=AhpqcvSGEO68JyVBFTta8knYTnbLMdJiBN+oEtI45Ys=; b=TXgcVDp4TmES7+Af1zpwJOLBnJg/ORZy4mZ1GnZaK1PWlkwQTsCS2wULD3saqWc8Z9 RCQ07GCtVAtDpuUiOdISy6TbznLb03/3ri8oeX3PWbRCYOQ3hYUHSNzD7hADKw4b+e/j JEZpWNk0r187CQym9CorgWhJNbiBYZfQKMcb+vD+NDy4OkxNJKwC1tRR5To3TcTdwQCK EIjnqAt6QQ3tr5mdaX4Vacf0nEHgEq8+JLwDXM/PpDpJ+l9go9RWjKrmEaivVQo1AtpI XZUaBNyiQabeLvQJHOcQetY+ncA6H+S0f/SkL7xB0mos72ZhyewJfj+BjEFZmowiRkWb k2sQ== X-Gm-Message-State: ANoB5pnqsXHKaoNLdrBHHZa3MmJ4VQSeZItdlABk92073M7iymP05wEt zIashxP6cmoWgeCmKHgRJ8yjlpPrLLRqYWBId1cd8foNeEzV6rR4sYWMAmx9PybQRCLNcgVclTs eZtBd/rFZtfDk0tG7ZA== X-Received: by 2002:a0c:fc49:0:b0:4b7:6a75:486b with SMTP id w9-20020a0cfc49000000b004b76a75486bmr21106860qvp.78.1668604438084; Wed, 16 Nov 2022 05:13:58 -0800 (PST) X-Google-Smtp-Source: AA0mqf6FUgYLLuSqOGS9ybsQXcLUgIPztJ+AoiGhivvmEal4nZuZOsV9L/BBm6oO8xZ4wHCxr0rzCg== X-Received: by 2002:a0c:fc49:0:b0:4b7:6a75:486b with SMTP id w9-20020a0cfc49000000b004b76a75486bmr21106841qvp.78.1668604437683; Wed, 16 Nov 2022 05:13:57 -0800 (PST) Received: from [192.168.1.101] (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 z17-20020ac86b91000000b0038d9555b580sm8678938qts.44.2022.11.16.05.13.56 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 16 Nov 2022 05:13:56 -0800 (PST) Message-ID: <07640bac-1c0e-8472-29d6-eb6dc3766824@redhat.com> Date: Wed, 16 Nov 2022 08:13:56 -0500 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.4.2 Subject: Re: [PATCH] c++, v2: Fix up calls to static operator() or operator[] [PR107624] To: Jakub Jelinek Cc: gcc-patches@gcc.gnu.org References: <1ec099de-4247-29ec-0e06-602a90370388@redhat.com> <3a734cad-3036-8429-adc5-a663a4fc7c7d@redhat.com> From: Jason Merrill In-Reply-To: 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.4 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_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On 11/16/22 03:25, Jakub Jelinek wrote: > On Tue, Nov 15, 2022 at 04:49:27PM -0500, Jason Merrill wrote: >>> Or do you want to outline the >>> if (result != error_mark_node >>> && TREE_CODE (TREE_TYPE (cand->fn)) != METHOD_TYPE >>> && TREE_SIDE_EFFECTS (obj)) >>> { >>> /* But avoid the implicit lvalue-rvalue conversion when 'a' >>> is volatile. */ >>> tree a = obj; >>> if (TREE_THIS_VOLATILE (a)) >>> a = build_this (a); >>> if (TREE_SIDE_EFFECTS (a)) >>> result = build2 (COMPOUND_EXPR, TREE_TYPE (result), a, result); >>> } >>> part that is now repeated 4 times to some helper function? If yes, >>> any suggestion on a good name? >> >> Please. Maybe keep_unused_object_arg? > > So like this? > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? OK. > 2022-11-16 Jakub Jelinek > > PR c++/107624 > * call.cc (keep_unused_object_arg): New function. > (build_op_call): Use it. > (build_op_subscript): Likewise. > (build_new_op): Similarly for ARRAY_REF. > (build_new_method_call): Likewise. > * decl.cc (grok_op_properties): For C++20 and earlier, if operator[] > is static member function, require exactly one parameter rather than > exactly two parameters. > > * g++.dg/cpp23/static-operator-call4.C: New test. > * g++.dg/cpp23/subscript10.C: New test. > * g++.dg/cpp23/subscript11.C: New test. > > --- gcc/cp/call.cc.jj 2022-11-15 07:59:57.337231337 +0100 > +++ gcc/cp/call.cc 2022-11-15 23:10:14.077666314 +0100 > @@ -5017,6 +5017,33 @@ build_operator_new_call (tree fnname, ve > return ret; > } > > +/* Evaluate side-effects from OBJ before evaluating call > + to FN in RESULT expression. > + This is for expressions of the form `obj->fn(...)' > + where `fn' turns out to be a static member function and > + `obj' needs to be evaluated. `fn' could be also static operator[] > + or static operator(), in which cases the source expression > + would be `obj[...]' or `obj(...)'. */ > + > +static tree > +keep_unused_object_arg (tree result, tree obj, tree fn) > +{ > + if (result == NULL_TREE > + || result == error_mark_node > + || TREE_CODE (TREE_TYPE (fn)) == METHOD_TYPE > + || !TREE_SIDE_EFFECTS (obj)) > + return result; > + > + /* But avoid the implicit lvalue-rvalue conversion when `obj' is > + volatile. */ > + tree a = obj; > + if (TREE_THIS_VOLATILE (a)) > + a = build_this (a); > + if (TREE_SIDE_EFFECTS (a)) > + return build2 (COMPOUND_EXPR, TREE_TYPE (result), a, result); > + return result; > +} > + > /* Build a new call to operator(). This may change ARGS. */ > > tree > @@ -5137,7 +5164,13 @@ build_op_call (tree obj, vec else if (TREE_CODE (cand->fn) == FUNCTION_DECL > && DECL_OVERLOADED_OPERATOR_P (cand->fn) > && DECL_OVERLOADED_OPERATOR_IS (cand->fn, CALL_EXPR)) > - result = build_over_call (cand, LOOKUP_NORMAL, complain); > + { > + result = build_over_call (cand, LOOKUP_NORMAL, complain); > + /* In an expression of the form `a()' where cand->fn > + which is operator() turns out to be a static member function, > + `a' is none-the-less evaluated. */ > + result = keep_unused_object_arg (result, obj, cand->fn); > + } > else > { > if (TREE_CODE (cand->fn) == FUNCTION_DECL) > @@ -7046,6 +7079,12 @@ build_new_op (const op_location_t &loc, > gcc_unreachable (); > } > } > + > + /* In an expression of the form `a[]' where cand->fn > + which is operator[] turns out to be a static member function, > + `a' is none-the-less evaluated. */ > + if (code == ARRAY_REF) > + result = keep_unused_object_arg (result, arg1, cand->fn); > } > else > { > @@ -7302,6 +7341,11 @@ build_op_subscript (const op_location_t > /* Specify evaluation order as per P0145R2. */ > CALL_EXPR_ORDERED_ARGS (call) = op_is_ordered (ARRAY_REF) == 1; > } > + > + /* In an expression of the form `a[]' where cand->fn > + which is operator[] turns out to be a static member function, > + `a' is none-the-less evaluated. */ > + result = keep_unused_object_arg (result, obj, cand->fn); > } > else > gcc_unreachable (); > @@ -11494,21 +11538,11 @@ build_new_method_call (tree instance, tr > /* In an expression of the form `a->f()' where `f' turns > out to be a static member function, `a' is > none-the-less evaluated. */ > - if (TREE_CODE (TREE_TYPE (fn)) != METHOD_TYPE > - && !is_dummy_object (instance) > - && TREE_SIDE_EFFECTS (instance)) > - { > - /* But avoid the implicit lvalue-rvalue conversion when 'a' > - is volatile. */ > - tree a = instance; > - if (TREE_THIS_VOLATILE (a)) > - a = build_this (a); > - if (TREE_SIDE_EFFECTS (a)) > - call = build2 (COMPOUND_EXPR, TREE_TYPE (call), a, call); > - } > - else if (call != error_mark_node > - && DECL_DESTRUCTOR_P (cand->fn) > - && !VOID_TYPE_P (TREE_TYPE (call))) > + if (!is_dummy_object (instance)) > + call = keep_unused_object_arg (call, instance, fn); > + if (call != error_mark_node > + && DECL_DESTRUCTOR_P (cand->fn) > + && !VOID_TYPE_P (TREE_TYPE (call))) > /* An explicit call of the form "x->~X()" has type > "void". However, on platforms where destructors > return "this" (i.e., those where > --- gcc/cp/decl.cc.jj 2022-11-15 07:59:57.321231551 +0100 > +++ gcc/cp/decl.cc 2022-11-15 13:13:51.814356965 +0100 > @@ -15400,6 +15400,10 @@ grok_op_properties (tree decl, bool comp > pedwarn (loc, OPT_Wc__23_extensions, "%qD may be a static member " > "function only with %<-std=c++23%> or %<-std=gnu++23%>", > decl); > + if (operator_code == ARRAY_REF) > + /* static operator[] should have exactly one argument > + for C++20 and earlier, so that it isn't multidimensional. */ > + op_flags = OVL_OP_FLAG_UNARY; > } > else if (DECL_STATIC_FUNCTION_P (decl)) > { > --- gcc/testsuite/g++.dg/cpp23/static-operator-call4.C.jj 2022-11-15 11:26:28.187511136 +0100 > +++ gcc/testsuite/g++.dg/cpp23/static-operator-call4.C 2022-11-15 11:27:19.684814244 +0100 > @@ -0,0 +1,37 @@ > +// PR c++/107624 > +// { dg-do run { target c++11 } } > +// { dg-options "" } > + > +int n[3]; > +struct S { > + static void operator() (int x) { n[0] |= (1 << x); } // { dg-warning "may be a static member function only with" "" { target c++20_down } } > + static void baz (int x) { n[1] |= (1 << x); } > + int s; > +}; > +volatile S s[2]; > + > +S & > +foo (int x) > +{ > + static S t; > + n[2] |= (1 << x); > + return t; > +} > + > +int > +main () > +{ > + int i = 0; > + foo (0) (0); > + if (n[0] != 1 || n[1] || n[2] != 1) > + __builtin_abort (); > + foo (1).baz (1); > + if (n[0] != 1 || n[1] != 2 || n[2] != 3) > + __builtin_abort (); > + s[i++] (2); > + if (i != 1 || n[0] != 5 || n[1] != 2 || n[2] != 3) > + __builtin_abort (); > + s[--i].baz (3); > + if (i != 0 || n[0] != 5 || n[1] != 10 || n[2] != 3) > + __builtin_abort (); > +} > --- gcc/testsuite/g++.dg/cpp23/subscript10.C.jj 2022-11-15 11:27:48.810420112 +0100 > +++ gcc/testsuite/g++.dg/cpp23/subscript10.C 2022-11-15 13:07:02.974885454 +0100 > @@ -0,0 +1,46 @@ > +// PR c++/107624 > +// { dg-do run { target c++11 } } > +// { dg-options "" } > + > +int n[3]; > +struct S { > + static int &operator[] (int x) { n[0] |= (1 << x); return n[2]; } // { dg-warning "may be a static member function only with" "" { target c++20_down } } > +#if __cpp_multidimensional_subscript >= 202211L > + static int &operator[] () { n[0] |= (1 << 8); return n[2]; } > + static int &operator[] (int y, int z, int w) { n[0] |= (1 << y) | (1 << z) | (1 << w); return n[2]; } > +#endif > + int s; > +}; > +volatile S s[2]; > + > +S & > +foo (int x) > +{ > + static S t; > + n[1] |= (1 << x); > + return t; > +} > + > +int > +main () > +{ > + int i = 0; > + foo (0) [0]++; > + if (n[0] != 1 || n[1] != 1 || n[2] != 1) > + __builtin_abort (); > + s[i++][2]++; > + if (i != 1 || n[0] != 5 || n[1] != 1 || n[2] != 2) > + __builtin_abort (); > +#if __cpp_multidimensional_subscript >= 202211L > + foo (3) []++; > + if (n[0] != 261 || n[1] != 9 || n[2] != 3) > + __builtin_abort (); > + int y = 10; > + int z = 10; > + int w = 13; > + foo (4) [y++, ++z, w++]++; > + if (n[0] != 11525 || n[1] != 25 || n[2] != 4 > + || y != 11 || z != 11 || w != 14) > + __builtin_abort (); > +#endif > +} > --- gcc/testsuite/g++.dg/cpp23/subscript11.C.jj 2022-11-15 13:14:50.938557460 +0100 > +++ gcc/testsuite/g++.dg/cpp23/subscript11.C 2022-11-15 13:19:20.681910074 +0100 > @@ -0,0 +1,11 @@ > +// { dg-do compile { target c++11 } } > +// { dg-options "" } > + > +struct S { > + static int &operator[] (int); // { dg-warning "may be a static member function only with" "" { target c++20_down } } > + static int &operator[] (); // { dg-warning "may be a static member function only with" "" { target c++20_down } } > + // { dg-error "'static int& S::operator\\\[\\\]\\\(\\\)' must have exactly one argument" "" { target c++20_down } .-1 } > + static int &operator[] (int, int, int); // { dg-warning "may be a static member function only with" "" { target c++20_down } } > + // { dg-error "'static int& S::operator\\\[\\\]\\\(int, int, int\\\)' must have exactly one argument" "" { target c++20_down } .-1 } > + int s; > +}; > > > Jakub >