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 3C53C3857733 for ; Wed, 10 May 2023 22:08:23 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 3C53C3857733 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=1683756502; 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=BMG5dTZHg+6Ubdo9NZPgLUMOZ7Ezn7J3Or+3B/6GxA0=; b=EjZkfZwS/HP0g6lPA7LkSjI5tBmi/C/1gWL8hXIbeeiUJhbAJcCcKztAhI4M9OSpfbuTKV S324mGFXoIqiT4thLNheFdJe8BpDesuKuwoyKkY0wo0kMc4V3hfHn3eO2DdeYAOw/tzzpE /PIKfOp/3YGhUPYKhhmnrWRaPy2ReuM= Received: from mail-qt1-f198.google.com (mail-qt1-f198.google.com [209.85.160.198]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-45-O8WgtOpmOeaCRYP-eQ3dXw-1; Wed, 10 May 2023 18:08:21 -0400 X-MC-Unique: O8WgtOpmOeaCRYP-eQ3dXw-1 Received: by mail-qt1-f198.google.com with SMTP id d75a77b69052e-3f3c9860201so11462641cf.0 for ; Wed, 10 May 2023 15:08:21 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1683756501; x=1686348501; 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=BMG5dTZHg+6Ubdo9NZPgLUMOZ7Ezn7J3Or+3B/6GxA0=; b=dqy9qR9Mynao6UD9bt9QmwiJUyE9t/kRrxZW9DomIkP0pYYHrIAmHmVA5r1UOS/3RF pY5QzjpMkK20/N5Nl13++xdREJaIwfcMYB/HxE8NPdzxflmG47eXLM9q0QZyTwOLj6LH +7GC3fK95GrBZ1t/BsG/ZgcnUHLOspmYXY4BLX6DJ+XJiVNZg+UVLcy63neh17VrEWWy iCQ682ZCvRXPnksr/XtMmuFhRH8bZDvanFhJy4FtqhUapKhHSQF8kfvd7BArvGy7qZz8 wjgIlrl5l45vcWLJNPumdXyGIK4gIz8851V0P9g9fh8Xq+9IwVMFjfF0MaHOau3uueHp 6tLA== X-Gm-Message-State: AC+VfDyHK3C0f8n9GkKZn7JtkcYNVNpS6pRD1pFCZTJPpVI6PiDx5NLD 8+nOu5GY3AzdL6PSnNzCklj109BF4jtLF/qhR+o2YCNpThFR4XJaeihrhRrFWxiZsSrIywReR4H bT6KvVEHjzB1kfusr6A== X-Received: by 2002:ac8:5847:0:b0:3f2:25e6:47bc with SMTP id h7-20020ac85847000000b003f225e647bcmr31031272qth.68.1683756501084; Wed, 10 May 2023 15:08:21 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ78tXNOS3lzvXcouBocc2Yt9ERlGUxARBhQIJXFalEVBjX3LqUmrWCjgdBbVuq7XYaQRmNYJw== X-Received: by 2002:ac8:5847:0:b0:3f2:25e6:47bc with SMTP id h7-20020ac85847000000b003f225e647bcmr31031226qth.68.1683756500676; Wed, 10 May 2023 15:08:20 -0700 (PDT) Received: from [192.168.1.108] (130-44-146-16.s12558.c3-0.arl-cbr1.sbo-arl.ma.cable.rcncustomer.com. [130.44.146.16]) by smtp.gmail.com with ESMTPSA id bs7-20020ac86f07000000b003f215cfab53sm889890qtb.53.2023.05.10.15.08.19 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 10 May 2023 15:08:20 -0700 (PDT) Message-ID: <402c3b96-c6f2-bbcb-ddcf-f4e4b0fbb056@redhat.com> Date: Wed, 10 May 2023 18:08:19 -0400 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.10.1 Subject: Re: [PATCH v2] c++: wrong std::is_convertible with cv-qual fn [PR109680] To: Marek Polacek Cc: GCC Patches References: <20230502231015.56181-1-polacek@redhat.com> <5dad74d3-5d80-fb3d-5774-675f36cad249@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=-14.4 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_NONE,TXREP,T_SCC_BODY_TEXT_LINE 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 5/10/23 17:28, Marek Polacek wrote: > On Wed, May 03, 2023 at 03:37:03PM -0400, Jason Merrill wrote: >> On 5/2/23 19:10, Marek Polacek wrote: >>> This PR points out that std::is_convertible has given the wrong answer >>> in >>> >>> static_assert (!std::is_convertible_v , ""); >>> >>> since r13-2822 implemented __is_{,nothrow_}convertible. >>> >>> std::is_convertible uses the imaginary >>> >>> To test() { return std::declval(); } >>> >>> to do its job. Here, From is 'int () const'. std::declval is defined as: >>> >>> template >>> typename std::add_rvalue_reference::type declval() noexcept; >>> >>> std::add_rvalue_reference is defined as "If T is a function type that >>> has no cv- or ref- qualifier or an object type, provides a member typedef >>> type which is T&&, otherwise type is T." >>> >>> In our case, T is cv-qualified, so the result is T, so we end up with >>> >>> int () const declval() noexcept; >>> >>> which is invalid. In other words, this is pretty much like: >>> >>> using T = int () const; >>> T fn1(); // bad, fn returning a fn >>> T& fn2(); // bad, cannot declare reference to qualified function type >>> T* fn3(); // bad, cannot declare pointer to qualified function type >>> >>> using U = int (); >>> U fn4(); // bad, fn returning a fn >>> U& fn5(); // OK >>> U* fn6(); // OK >>> >>> I think is_convertible_helper needs to simulate std::declval better. >>> I wouldn't be surprised if other type traits needed a similar fix. >>> >>> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/13? >>> >>> I've tested the new test with G++12 and clang++ as well (with >>> std::is_convertible). >>> >>> PR c++/109680 >>> >>> gcc/cp/ChangeLog: >>> >>> * method.cc (is_convertible_helper): Correct simulating std::declval. >>> >>> gcc/testsuite/ChangeLog: >>> >>> * g++.dg/ext/is_convertible6.C: New test. >>> --- >>> gcc/cp/method.cc | 20 ++++++++++++++++++++ >>> gcc/testsuite/g++.dg/ext/is_convertible6.C | 16 ++++++++++++++++ >>> 2 files changed, 36 insertions(+) >>> create mode 100644 gcc/testsuite/g++.dg/ext/is_convertible6.C >>> >>> diff --git a/gcc/cp/method.cc b/gcc/cp/method.cc >>> index 00eae56eb5b..38eb7520312 100644 >>> --- a/gcc/cp/method.cc >>> +++ b/gcc/cp/method.cc >>> @@ -2245,6 +2245,26 @@ is_convertible_helper (tree from, tree to) >>> { >>> if (VOID_TYPE_P (from) && VOID_TYPE_P (to)) >>> return integer_one_node; >>> + /* std::is_{,nothrow_}convertible test whether the imaginary function >>> + definition >>> + >>> + To test() { return std::declval(); } >>> + >>> + is well-formed. A function can't return a function... */ >>> + if (FUNC_OR_METHOD_TYPE_P (to) >>> + /* ...neither can From be a function with cv-/ref-qualifiers: >>> + std::declval is defined as >>> + >>> + template >>> + typename std::add_rvalue_reference::type declval() noexcept; >>> + >>> + and std::add_rvalue_reference yields T when T is a function with >>> + cv- or ref-qualifiers, making the definition ill-formed. >>> + ??? Should we check this in other uses of build_stub_object too? */ >> >> Probably we want a build_trait_object that wraps build_stub_object with >> these extra checks, or does something that exercises more of the normal >> code, maybe by tsubsting into T (U&&) with { to, from }? > > I did the former. We only have the To type when processing > __is_convertible, so I think that can't be part of the declval > function, which takes another type, From. IOW, if we want to factor > declval out, build_trait_object will take only one type. Eh, that > still sounds confusing. But the patch is simple. > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? OK. > -- >8 -- > This PR points out that std::is_convertible has given the wrong answer > in > > static_assert (!std::is_convertible_v , ""); > > since r13-2822 implemented __is_{,nothrow_}convertible. > > std::is_convertible uses the imaginary > > To test() { return std::declval(); } > > to do its job. Here, From is 'int () const'. std::declval is defined as: > > template > typename std::add_rvalue_reference::type declval() noexcept; > > std::add_rvalue_reference is defined as "If T is a function type that > has no cv- or ref- qualifier or an object type, provides a member typedef > type which is T&&, otherwise type is T." > > In our case, T is cv-qualified, so the result is T, so we end up with > > int () const declval() noexcept; > > which is invalid. In other words, this is pretty much like: > > using T = int () const; > T fn1(); // bad, fn returning a fn > T& fn2(); // bad, cannot declare reference to qualified function type > T* fn3(); // bad, cannot declare pointer to qualified function type > > using U = int (); > U fn4(); // bad, fn returning a fn > U& fn5(); // OK > U* fn6(); // OK > > I think is_convertible_helper needs to simulate std::declval better. > To that end, I'm introducing build_trait_object, to be used where > a declval is needed. > > PR c++/109680 > > gcc/cp/ChangeLog: > > * method.cc (build_trait_object): New. > (assignable_expr): Use it. > (ref_xes_from_temporary): Likewise. > (is_convertible_helper): Likewise. Check FUNC_OR_METHOD_TYPE_P. > > gcc/testsuite/ChangeLog: > > * g++.dg/ext/is_convertible6.C: New test. > --- > gcc/cp/method.cc | 39 +++++++++++++++++++--- > gcc/testsuite/g++.dg/ext/is_convertible6.C | 16 +++++++++ > 2 files changed, 51 insertions(+), 4 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/ext/is_convertible6.C > > diff --git a/gcc/cp/method.cc b/gcc/cp/method.cc > index 00eae56eb5b..7ec7bfe8387 100644 > --- a/gcc/cp/method.cc > +++ b/gcc/cp/method.cc > @@ -1902,6 +1902,27 @@ build_stub_object (tree reftype) > return convert_from_reference (stub); > } > > +/* Build a std::declval() expression and return it. */ > + > +tree > +build_trait_object (tree type) > +{ > + /* TYPE can't be a function with cv-/ref-qualifiers: std::declval is > + defined as > + > + template > + typename std::add_rvalue_reference::type declval() noexcept; > + > + and std::add_rvalue_reference yields T when T is a function with > + cv- or ref-qualifiers, making the definition ill-formed. */ > + if (FUNC_OR_METHOD_TYPE_P (type) > + && (type_memfn_quals (type) != TYPE_UNQUALIFIED > + || type_memfn_rqual (type) != REF_QUAL_NONE)) > + return error_mark_node; > + > + return build_stub_object (type); > +} > + > /* Determine which function will be called when looking up NAME in TYPE, > called with a single ARGTYPE argument, or no argument if ARGTYPE is > null. FLAGS and COMPLAIN are as for build_new_method_call. > @@ -2050,8 +2071,8 @@ static tree > assignable_expr (tree to, tree from) > { > cp_unevaluated cp_uneval_guard; > - to = build_stub_object (to); > - from = build_stub_object (from); > + to = build_trait_object (to); > + from = build_trait_object (from); > tree r = cp_build_modify_expr (input_location, to, NOP_EXPR, from, tf_none); > return r; > } > @@ -2231,7 +2252,9 @@ ref_xes_from_temporary (tree to, tree from, bool direct_init_p) > return false; > /* We don't check is_constructible: if T isn't constructible > from U, we won't be able to create a conversion. */ > - tree val = build_stub_object (from); > + tree val = build_trait_object (from); > + if (val == error_mark_node) > + return false; > if (!TYPE_REF_P (from) && TREE_CODE (from) != FUNCTION_TYPE) > val = CLASS_TYPE_P (from) ? force_rvalue (val, tf_none) : rvalue (val); > return ref_conv_binds_to_temporary (to, val, direct_init_p).is_true (); > @@ -2246,7 +2269,15 @@ is_convertible_helper (tree from, tree to) > if (VOID_TYPE_P (from) && VOID_TYPE_P (to)) > return integer_one_node; > cp_unevaluated u; > - tree expr = build_stub_object (from); > + tree expr = build_trait_object (from); > + /* std::is_{,nothrow_}convertible test whether the imaginary function > + definition > + > + To test() { return std::declval(); } > + > + is well-formed. A function can't return a function. */ > + if (FUNC_OR_METHOD_TYPE_P (to) || expr == error_mark_node) > + return error_mark_node; > deferring_access_check_sentinel acs (dk_no_deferred); > return perform_implicit_conversion (to, expr, tf_none); > } > diff --git a/gcc/testsuite/g++.dg/ext/is_convertible6.C b/gcc/testsuite/g++.dg/ext/is_convertible6.C > new file mode 100644 > index 00000000000..180582663e8 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/ext/is_convertible6.C > @@ -0,0 +1,16 @@ > +// PR c++/109680 > +// { dg-do compile { target c++11 } } > + > +#define SA(X) static_assert((X),#X) > + > +SA(!__is_convertible(int () const, int (*)())); > +SA(!__is_convertible(int (*)(), int () const)); > + > +SA( __is_convertible(int (), int (*)())); > +SA(!__is_convertible(int (*)(), int ())); > + > +SA( __is_convertible(int (int), int (*) (int))); > +SA(!__is_convertible(int (*) (int), int (int))); > + > +SA(!__is_convertible(int (int) const, int (*) (int))); > +SA(!__is_convertible(int (*) (int), int (int) const)); > > base-commit: bdc10c2bfaceb3be567e0a27d8951a22b4be2ed4