From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qk1-x72e.google.com (mail-qk1-x72e.google.com [IPv6:2607:f8b0:4864:20::72e]) by sourceware.org (Postfix) with ESMTPS id 5F7D93857B95 for ; Thu, 22 Sep 2022 11:32:17 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 5F7D93857B95 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-qk1-x72e.google.com with SMTP id 3so5907904qka.5 for ; Thu, 22 Sep 2022 04:32:17 -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:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :sender:from:to:cc:subject:date; bh=kKGYNj3HqXk6GQVTUEV0SiZawUN8/2VxaHUrW1m8200=; b=jjGGwXVRT7Dk0LTvRePDN2pbnhbYdHs0WQi58ggHzxRGJQk2PhOmdMtnhkN5jkvj2F gf57i+DoH5Bie9ibf2KHaMiQdwVk/yZi4zVDEz2LOUJZ0h3ZRX9CWbmEbR6WP9OYp+5+ +0KwAgJ+Sey5NAnUKL5ZgNZrslyH+fMcWak9L5cGpoCzmc5xs4rrRyupGjUdRc1n0pAD VmAwxvFVA1SlDuiPDbkGNNbtFbDUTho9C8PHjSGKNW2KSUOXyCmgBM//DYiz4etlsfc8 VENF13HRRfOO44aLgbjH+wE2cMtPSaWxh+I2nKgvvrlD+Y3FfpRf0R5l6r09KPes4VHg wJ3w== 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:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :sender:x-gm-message-state:from:to:cc:subject:date; bh=kKGYNj3HqXk6GQVTUEV0SiZawUN8/2VxaHUrW1m8200=; b=h7V2bPSl/kcy/ym6HTjMR+WLWGhJ2Mellm1bWtVUwnGtalVFkRkujnQDcu/GIuQX4E wQYDXWPetMH0OLwi33vixZGwabwIaUoIC9uSGlKzWp46XHhIzBNiJ2k4X6pv/VB8RGg+ 89zUYmCvoDSSoIAY/BABiHRZU4/fFn2OIzeVtCL6XbTgodjhhB/en4zrprM4nk8RTVoQ YGltwBGBkwJ7qs0gkE+YAE65giGOXk1mvox2JTFQEDxsO/IKDiZ5k8HH3ma4rQaV865J FMNCudYolaEfkh9cOITs9nXchpNQZK5NscTBDsoIJVuqcvsKsLT+w8h8WBrlQYjvR64h 9OOQ== X-Gm-Message-State: ACrzQf22ASG6a1FHpF80U9HhzSMQXPjGuFs/ysS5plGDxs1cpTCbyiyK DUO7jyiYQm9fO5P3Xr7IPwM= X-Google-Smtp-Source: AMsMyM7vsKYxTvll/dz5XYY54KqkNCWFeAqg8iYzbCd3yb6POvsp/QzpT8y87Ma/OIxZ37TSD7SRog== X-Received: by 2002:a05:620a:4691:b0:6ce:5121:dac0 with SMTP id bq17-20020a05620a469100b006ce5121dac0mr1644322qkb.317.1663846336798; Thu, 22 Sep 2022 04:32:16 -0700 (PDT) Received: from ?IPV6:2620:10d:c0a3:1407:3b43:6d5e:af4a:9ec4? ([2620:10d:c091:500::4:4dee]) by smtp.googlemail.com with ESMTPSA id g12-20020ac8468c000000b0035cb9531851sm3257119qto.65.2022.09.22.04.32.15 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 22 Sep 2022 04:32:16 -0700 (PDT) Sender: Nathan Sidwell Message-ID: <911d8574-ce8a-a17f-169d-86b78bd78f00@acm.org> Date: Thu, 22 Sep 2022 07:32:15 -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: partial variable template specializations [PR106826] To: Patrick Palka , gcc-patches@gcc.gnu.org Cc: jason@redhat.com References: <20220921161629.1738016-1-ppalka@redhat.com> Content-Language: en-US From: Nathan Sidwell In-Reply-To: <20220921161629.1738016-1-ppalka@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-3040.0 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/21/22 12:16, Patrick Palka wrote: > With partial variable template specializations, it looks like we > stream the VAR_DECL (i.e. the DECL_TEMPLATE_RESULT of the corresponding > TEMPLATE_DECL) since process_partial_specialization adds it to the > specializations table, but end up never streaming the corresponding > TEMPLATE_DECL itself that appears only in the primary template's > DECL_TEMPLATE_SPECIALIZATIONS list, which leads to the list being > incomplete on stream-in. > > The modules machinery already has special logic for streaming partial > specializations of class templates; this patch generalizes it to handle > those of variable templates as well. looks good, I didn't realize template vars had partial specializations. > > Tested on x86_64-pc-linux-gnu, does this look OK for trunk? > > PR c++/106826 > > gcc/cp/ChangeLog: > > * module.cc (trees_out::decl_value): Use get_template_info in > the MK_partial case. > (trees_out::key_mergeable): Likewise. > (trees_in::key_mergeable): Likewise. > (has_definition): Consider DECL_INITIAL of a partial variable > template specialization. > (depset::hash::make_dependency): Introduce a dependency of > partial variable template specializations too. > > gcc/testsuite/ChangeLog: > > * g++.dg/modules/partial-2_a.C: New test. > * g++.dg/modules/partial-2_b.C: New test. > --- > gcc/cp/module.cc | 32 +++++++++------- > gcc/testsuite/g++.dg/modules/partial-2_a.C | 43 ++++++++++++++++++++++ > gcc/testsuite/g++.dg/modules/partial-2_b.C | 21 +++++++++++ > 3 files changed, 82 insertions(+), 14 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/modules/partial-2_a.C > create mode 100644 gcc/testsuite/g++.dg/modules/partial-2_b.C > > diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc > index 9a9ef4e3332..334bde99b0f 100644 > --- a/gcc/cp/module.cc > +++ b/gcc/cp/module.cc > @@ -7789,8 +7789,9 @@ trees_out::decl_value (tree decl, depset *dep) > } > else > { > - tree_node (CLASSTYPE_TI_TEMPLATE (TREE_TYPE (inner))); > - tree_node (CLASSTYPE_TI_ARGS (TREE_TYPE (inner))); > + tree ti = get_template_info (inner); > + tree_node (TI_TEMPLATE (ti)); > + tree_node (TI_ARGS (ti)); > } > } > tree_node (get_constraints (decl)); > @@ -10626,8 +10627,9 @@ trees_out::key_mergeable (int tag, merge_kind mk, tree decl, tree inner, > case MK_partial: > { > key.constraints = get_constraints (inner); > - key.ret = CLASSTYPE_TI_TEMPLATE (TREE_TYPE (inner)); > - key.args = CLASSTYPE_TI_ARGS (TREE_TYPE (inner)); > + tree ti = get_template_info (inner); > + key.ret = TI_TEMPLATE (ti); > + key.args = TI_ARGS (ti); > } > break; > } > @@ -10866,8 +10868,8 @@ trees_in::key_mergeable (int tag, merge_kind mk, tree decl, tree inner, > spec; spec = TREE_CHAIN (spec)) > { > tree tmpl = TREE_VALUE (spec); > - if (template_args_equal (key.args, > - CLASSTYPE_TI_ARGS (TREE_TYPE (tmpl))) > + tree ti = get_template_info (tmpl); > + if (template_args_equal (key.args, TI_ARGS (ti)) > && cp_tree_equal (key.constraints, > get_constraints > (DECL_TEMPLATE_RESULT (tmpl)))) > @@ -11381,8 +11383,7 @@ has_definition (tree decl) > > case VAR_DECL: > if (DECL_LANG_SPECIFIC (decl) > - && DECL_TEMPLATE_INFO (decl) > - && DECL_USE_TEMPLATE (decl) < 2) > + && DECL_TEMPLATE_INFO (decl)) > return DECL_INITIAL (decl); > else > { > @@ -12498,11 +12499,14 @@ depset::hash::make_dependency (tree decl, entity_kind ek) > > if (!dep) > { > - if (DECL_IMPLICIT_TYPEDEF_P (decl) > - /* ... not an enum, for instance. */ > - && RECORD_OR_UNION_TYPE_P (TREE_TYPE (decl)) > - && TYPE_LANG_SPECIFIC (TREE_TYPE (decl)) > - && CLASSTYPE_USE_TEMPLATE (TREE_TYPE (decl)) == 2) > + if ((DECL_IMPLICIT_TYPEDEF_P (decl) > + /* ... not an enum, for instance. */ > + && RECORD_OR_UNION_TYPE_P (TREE_TYPE (decl)) > + && TYPE_LANG_SPECIFIC (TREE_TYPE (decl)) > + && CLASSTYPE_USE_TEMPLATE (TREE_TYPE (decl)) == 2) > + || (VAR_P (decl) > + && DECL_LANG_SPECIFIC (decl) > + && DECL_USE_TEMPLATE (decl) == 2)) > { > /* A partial or explicit specialization. Partial > specializations might not be in the hash table, because > @@ -12515,7 +12519,7 @@ depset::hash::make_dependency (tree decl, entity_kind ek) > dep_hash, and then convert the dep we just found into a > redirect. */ > > - tree ti = TYPE_TEMPLATE_INFO (TREE_TYPE (decl)); > + tree ti = get_template_info (decl); > tree tmpl = TI_TEMPLATE (ti); > tree partial = NULL_TREE; > for (tree spec = DECL_TEMPLATE_SPECIALIZATIONS (tmpl); > diff --git a/gcc/testsuite/g++.dg/modules/partial-2_a.C b/gcc/testsuite/g++.dg/modules/partial-2_a.C > new file mode 100644 > index 00000000000..2681bb59ce8 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/partial-2_a.C > @@ -0,0 +1,43 @@ > +// PR c++/106826 > +// { dg-additional-options -fmodules-ts } > +// { 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 > diff --git a/gcc/testsuite/g++.dg/modules/partial-2_b.C b/gcc/testsuite/g++.dg/modules/partial-2_b.C > new file mode 100644 > index 00000000000..0af41ef5e5e > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/partial-2_b.C > @@ -0,0 +1,21 @@ > +// PR c++/106826 > +// { 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 -- Nathan Sidwell