From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qt1-x831.google.com (mail-qt1-x831.google.com [IPv6:2607:f8b0:4864:20::831]) by sourceware.org (Postfix) with ESMTPS id 367B6385843A for ; Mon, 26 Sep 2022 14:47:49 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 367B6385843A Authentication-Results: sourceware.org; dmarc=fail (p=none dis=none) header.from=acm.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-qt1-x831.google.com with SMTP id g12so4234062qts.1 for ; Mon, 26 Sep 2022 07:47:49 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :sender:from:to:cc:subject:date; bh=Za8Ydi1FuRlkkmtEWotiITV8niVRe5CzrTITjDpD2e8=; b=D7fZ7D9ggxqQSgGZRlpwZkxolSTxyxbHIBSThyzIUUtGkFeptrYGgIIREV6k2hwCUj jKPBwU9B00vy2WGAJn/Z59P8p22KgxFqpQSwnnLdii9uF9ywnzEP9VMLCn0H9LO2nIWz 9+QOikEyMCuHdDeAUoXsHqPxJNhgY0ut7hdy7L9qn9x/nUpQdx2vOhHSvn7Zx63ESgRr RP64Iu9bcwtJDswetgGCC4r3ZgDqBR1tq1EyGwgtzapxUf0oJ1lgqwy+ujWSQn7tIlBc SmNU7mjbJ9zadzaqPJzzaDdwg0dDAGR4VY5hcaSNV1xiYHPajW8fK3jI79SeDVTS355f MOpA== 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 :sender:x-gm-message-state:from:to:cc:subject:date; bh=Za8Ydi1FuRlkkmtEWotiITV8niVRe5CzrTITjDpD2e8=; b=jieZyQENuy2GozBv1a7l+kAEx5UPTNzvW1R5vjabau3HMg07MG+hjxCQSd+hkGPw6v 2qG1YCBxtwgZxsSicyIi+E6N47nKuiklpRN4piStyAeChgDXMuUuzBPE0jf5OWVH2XmW Wf1oefdSaq0wUQrPY0DG5V9xJf2DF0NB6mc0VFZxY3AmXXpamWwFeTizf3w9UBkel8az 6QjsSR3VGUCSLKDmH+xK/UodL9wVEIZSO5fnKJTMiRIhvlr9sstIYkRED1ffNyym6Fdo vnXxhzM5mce9zl0FIgVgE54Jjmn6fUJg73uTiXBul1P6MLnNy5+90FutCC4iU/mQlW2o K8VQ== X-Gm-Message-State: ACrzQf0bGcvkpnUa5Xeg/dQnUKeqEekp+TT+GwfzJhyZxtd9H/PMSlqc S73uszxTb+ilEDMH65dNC5A= X-Google-Smtp-Source: AMsMyM40PDXwJcaqjd9+72snu1NEuJSAGzfpjc+AZQUEiBw+61mDR5so+mBHSWX58SAjZ0lmOmstdg== X-Received: by 2002:ac8:5b0e:0:b0:35c:bae7:d9eb with SMTP id m14-20020ac85b0e000000b0035cbae7d9ebmr18310835qtw.681.1664203668478; Mon, 26 Sep 2022 07:47:48 -0700 (PDT) Received: from ?IPV6:2620:10d:c0a3:1407:bb5b:4f29:715d:d2b6? ([2620:10d:c091:500::6:a953]) by smtp.googlemail.com with ESMTPSA id l27-20020a37f91b000000b006af3f3b385csm11539153qkj.98.2022.09.26.07.47.47 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 26 Sep 2022 07:47:48 -0700 (PDT) Sender: Nathan Sidwell Message-ID: <9dd2a7d3-4800-6db5-e678-3273755866ac@acm.org> Date: Mon, 26 Sep 2022 10:47:47 -0400 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.2.1 Subject: Re: [PATCH] c++ modules: variable template partial spec fixes [PR107033] Content-Language: en-US To: Patrick Palka , gcc-patches@gcc.gnu.org Cc: jason@redhat.com References: <20220926143620.24037-1-ppalka@redhat.com> From: Nathan Sidwell In-Reply-To: <20220926143620.24037-1-ppalka@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-3039.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_EF,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM,GIT_PATCH_0,HEADER_FROM_DIFFERENT_DOMAINS,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,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 9/26/22 10:36, Patrick Palka wrote: > In r13-2775-g32d8123cd6ce87 I overlooked that we need to adjust the > call to add_mergeable_specialization in the MK_partial case to correctly > handle variable template partial specializations (it currently assumes > we're always dealing with one for a class template). This fixes an ICE > when converting the testcase from that commit to use an importable > header instead of a named module. looks good, thanks > > Tested on x86_64-pc-linux-gnu, does this look OK for trunk? > > PR c++/107033 > > gcc/cp/ChangeLog: > > * module.cc (trees_in::decl_value): In the MK_partial case > for a variable template partial specialization, pass decl_p=true > to add_mergeable_specialization and set spec to the VAR_DECL > not the TEMPLATE_DECL. > * pt.cc (add_mergeable_specialization): For a variable template > partial specialization, set the TREE_TYPE of the new > DECL_TEMPLATE_SPECIALIZATIONS node to the TREE_TYPE of the > VAR_DECL not the VAR_DECL itself. > > gcc/testsuite/ChangeLog: > > * g++.dg/modules/partial-2.cc, g++.dg/modules/partial-2.h: New > files, factored out from ... > * g++.dg/modules/partial-2_a.C, g++.dg/modules/partial-2_b.C: ... > here. > * g++.dg/modules/partial-2_c.H: New test. > * g++.dg/modules/partial-2_d.C: New test. > --- > gcc/cp/module.cc | 17 ++++++---- > gcc/cp/pt.cc | 2 +- > gcc/testsuite/g++.dg/modules/partial-2.cc | 17 ++++++++++ > gcc/testsuite/g++.dg/modules/partial-2.h | 38 +++++++++++++++++++++ > gcc/testsuite/g++.dg/modules/partial-2_a.C | 39 +--------------------- > gcc/testsuite/g++.dg/modules/partial-2_b.C | 18 +--------- > gcc/testsuite/g++.dg/modules/partial-2_c.H | 5 +++ > gcc/testsuite/g++.dg/modules/partial-2_d.C | 8 +++++ > 8 files changed, 82 insertions(+), 62 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/modules/partial-2.cc > create mode 100644 gcc/testsuite/g++.dg/modules/partial-2.h > create mode 100644 gcc/testsuite/g++.dg/modules/partial-2_c.H > create mode 100644 gcc/testsuite/g++.dg/modules/partial-2_d.C > > diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc > index f23832cb56a..7496df5e843 100644 > --- a/gcc/cp/module.cc > +++ b/gcc/cp/module.cc > @@ -8185,13 +8185,18 @@ trees_in::decl_value () > /* Set the TEMPLATE_DECL's type. */ > TREE_TYPE (decl) = TREE_TYPE (inner); > > - if (mk & MK_template_mask > - || mk == MK_partial) > + /* Add to specialization tables now that constraints etc are > + added. */ > + if (mk == MK_partial) > { > - /* Add to specialization tables now that constraints etc are > - added. */ > - bool is_type = mk == MK_partial || !(mk & MK_tmpl_decl_mask); > - > + bool is_type = TREE_CODE (inner) == TYPE_DECL; > + spec.spec = is_type ? type : inner; > + add_mergeable_specialization (!is_type, false, > + &spec, decl, spec_flags); > + } > + else if (mk & MK_template_mask) > + { > + bool is_type = !(mk & MK_tmpl_decl_mask); > spec.spec = is_type ? type : mk & MK_tmpl_tmpl_mask ? inner : decl; > add_mergeable_specialization (!is_type, > !is_type && mk & MK_tmpl_alias_mask, > diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc > index db4e808adec..1f088fe281e 100644 > --- a/gcc/cp/pt.cc > +++ b/gcc/cp/pt.cc > @@ -31010,7 +31010,7 @@ add_mergeable_specialization (bool decl_p, bool alias_p, spec_entry *elt, > /* A partial specialization. */ > tree cons = tree_cons (elt->args, decl, > DECL_TEMPLATE_SPECIALIZATIONS (elt->tmpl)); > - TREE_TYPE (cons) = elt->spec; > + TREE_TYPE (cons) = decl_p ? TREE_TYPE (elt->spec) : elt->spec; > DECL_TEMPLATE_SPECIALIZATIONS (elt->tmpl) = cons; > } > } > diff --git a/gcc/testsuite/g++.dg/modules/partial-2.cc b/gcc/testsuite/g++.dg/modules/partial-2.cc > new file mode 100644 > index 00000000000..1316bf5e1c5 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/partial-2.cc > @@ -0,0 +1,17 @@ > +static_assert(is_reference_v); > +static_assert(is_reference_v); > +static_assert(!is_reference_v); > + > +static_assert(A::is_reference_v); > +static_assert(A::is_reference_v); > +static_assert(!A::is_reference_v); > + > +#if __cpp_concepts > +static_assert(concepts::is_reference_v); > +static_assert(concepts::is_reference_v); > +static_assert(!concepts::is_reference_v); > + > +static_assert(concepts::A::is_reference_v); > +static_assert(concepts::A::is_reference_v); > +static_assert(!concepts::A::is_reference_v); > +#endif > diff --git a/gcc/testsuite/g++.dg/modules/partial-2.h b/gcc/testsuite/g++.dg/modules/partial-2.h > new file mode 100644 > index 00000000000..afcfce791b3 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/partial-2.h > @@ -0,0 +1,38 @@ > +template constexpr bool is_reference_v = false; > +template constexpr bool is_reference_v = true; > +template constexpr bool is_reference_v = true; > + > +struct A { > + template static constexpr bool is_reference_v = false; > +}; > + > +template constexpr bool A::is_reference_v = true; > +template constexpr bool A::is_reference_v = true; > + > +#if __cpp_concepts > +namespace concepts { > + template bool is_reference_v; > + > + template requires __is_same(T, T&) > + constexpr bool is_reference_v = true; > + > + template requires __is_same(T, T&&) && (!__is_same(T, T&)) > + constexpr bool is_reference_v = true; > + > + template requires (!__is_same(T, T&)) && (!__is_same(T, T&&)) > + constexpr bool is_reference_v = false; > + > + struct A { > + template static bool is_reference_v; > + }; > + > + template requires __is_same(T, T&) > + constexpr bool A::is_reference_v = true; > + > + template requires __is_same(T, T&&) && (!__is_same(T, T&)) > + constexpr bool A::is_reference_v = true; > + > + template requires (!__is_same(T, T&)) && (!__is_same(T, T&&)) > + constexpr bool A::is_reference_v = false; > +} > +#endif > diff --git a/gcc/testsuite/g++.dg/modules/partial-2_a.C b/gcc/testsuite/g++.dg/modules/partial-2_a.C > index 2681bb59ce8..1582f56f2d4 100644 > --- a/gcc/testsuite/g++.dg/modules/partial-2_a.C > +++ b/gcc/testsuite/g++.dg/modules/partial-2_a.C > @@ -3,41 +3,4 @@ > // { dg-module-cmi pr106826 } > export module pr106826; > > -template constexpr bool is_reference_v = false; > -template constexpr bool is_reference_v = true; > -template constexpr bool is_reference_v = true; > - > -struct A { > - template static constexpr bool is_reference_v = false; > -}; > - > -template constexpr bool A::is_reference_v = true; > -template constexpr bool A::is_reference_v = true; > - > -#if __cpp_concepts > -namespace concepts { > - template bool is_reference_v; > - > - template requires __is_same(T, T&) > - constexpr bool is_reference_v = true; > - > - template requires __is_same(T, T&&) && (!__is_same(T, T&)) > - constexpr bool is_reference_v = true; > - > - template requires (!__is_same(T, T&)) && (!__is_same(T, T&&)) > - constexpr bool is_reference_v = false; > - > - struct A { > - template static bool is_reference_v; > - }; > - > - template requires __is_same(T, T&) > - constexpr bool A::is_reference_v = true; > - > - template requires __is_same(T, T&&) && (!__is_same(T, T&)) > - constexpr bool A::is_reference_v = true; > - > - template requires (!__is_same(T, T&)) && (!__is_same(T, T&&)) > - constexpr bool A::is_reference_v = false; > -} > -#endif > +#include "partial-2.h" > diff --git a/gcc/testsuite/g++.dg/modules/partial-2_b.C b/gcc/testsuite/g++.dg/modules/partial-2_b.C > index 0af41ef5e5e..1b0c7a53e9f 100644 > --- a/gcc/testsuite/g++.dg/modules/partial-2_b.C > +++ b/gcc/testsuite/g++.dg/modules/partial-2_b.C > @@ -2,20 +2,4 @@ > // { dg-additional-options -fmodules-ts } > module pr106826; > > -static_assert(is_reference_v); > -static_assert(is_reference_v); > -static_assert(!is_reference_v); > - > -static_assert(A::is_reference_v); > -static_assert(A::is_reference_v); > -static_assert(!A::is_reference_v); > - > -#if __cpp_concepts > -static_assert(concepts::is_reference_v); > -static_assert(concepts::is_reference_v); > -static_assert(!concepts::is_reference_v); > - > -static_assert(concepts::A::is_reference_v); > -static_assert(concepts::A::is_reference_v); > -static_assert(!concepts::A::is_reference_v); > -#endif > +#include "partial-2.cc" > diff --git a/gcc/testsuite/g++.dg/modules/partial-2_c.H b/gcc/testsuite/g++.dg/modules/partial-2_c.H > new file mode 100644 > index 00000000000..bd838529ce0 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/partial-2_c.H > @@ -0,0 +1,5 @@ > +// PR c++/107033 > +// { dg-additional-options -fmodule-header } > +// { dg-module-cmi {} } > + > +#include "partial-2.h" > diff --git a/gcc/testsuite/g++.dg/modules/partial-2_d.C b/gcc/testsuite/g++.dg/modules/partial-2_d.C > new file mode 100644 > index 00000000000..ed54d3c2884 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/partial-2_d.C > @@ -0,0 +1,8 @@ > +// PR c++/107033 > +// { dg-additional-options -fmodules-ts } > +// { dg-module-cmi pr107033 } > +export module pr107033; > + > +import "partial-2_c.H"; > + > +#include "partial-2.cc" -- Nathan Sidwell