From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qt1-x836.google.com (mail-qt1-x836.google.com [IPv6:2607:f8b0:4864:20::836]) by sourceware.org (Postfix) with ESMTPS id 367A93858C5E for ; Fri, 3 Feb 2023 00:56:46 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 367A93858C5E 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-x836.google.com with SMTP id g7so4009344qto.11 for ; Thu, 02 Feb 2023 16:56:46 -0800 (PST) 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:message-id:reply-to; bh=RJsN7LCboM5j2Ob7yxNcz6sjo46LXj7FVEVTtIxIITw=; b=PWCpl2CFcAgcD2bc+Zt/9o1hw3Nx6g8grMEtmJc5Qmq19dANIb2NnHW6karwJ72sQ8 Uc3Sc9j5lPdCAcynx2qnD/UQHXysa2JueZ1g3jkP1R2F2uBqax8vIVj/owT/D64Ls1GA tz2yvVg0D2Ie7CGrDk5XXxhyGv/sWxJwpocf/n5IuOZjQhzEVPxKFazAc3hsE2hawrsY tOqXlAFIAXYF+dUEOsVvjZ+81R77L1GtJnlKpwh7UL8kKDSspR4wPfV7TfwIiZaWov5e 1PaJpRpdK2PFBVWaeLnQbujm8TfJhzVcJCcjY7CMbrtOIZ97JN6R7ai5ORQwquwBdpvX suHQ== 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:message-id :reply-to; bh=RJsN7LCboM5j2Ob7yxNcz6sjo46LXj7FVEVTtIxIITw=; b=3pzF/aPsIN6JRVFnuA5TowJ1BbV5FJjiybqobDgnPyBNOQA0ruooeKF9jHaTum5n4U WvKsdMHnQC80AlhiPhRuYXwd09YqBeHiS4QiqlnDNp/CaJum7hd+r3apC7dQ29U80PMW iwxX7blujpjzlyPPmAVk8F0BdL5EapwywN9m6XxSqmR4h70Z0O/foQifZjvoF44RnLut I0IY+Yz551H2RNh9KQLClOzvfOZFDq1KOeG8WQ3Jy7GUDt8Flp41IlnXDcKLiQH9AxWx +8mEIX5MVqGkx/ABYgdeAhb5F6QlKo6kWDlgECsG+SX2cos8h7STT81bQ73Gqfu/OBLe b4oA== X-Gm-Message-State: AO0yUKUilqwzAqcRVTuovtzM234o8gezkF9pV8kkv1PfLi/MeOzIP68p AuZsJcjhhvf29bT3nW/Ko1I= X-Google-Smtp-Source: AK7set9esOFD0eqJsiAnv9kph2ymjGLbDTPXgwjUuzXAc7NsutZu6cr5z5q1BcLqVTscnr21mL/BAg== X-Received: by 2002:ac8:5bd5:0:b0:3b8:ca58:ee4c with SMTP id b21-20020ac85bd5000000b003b8ca58ee4cmr17215962qtb.2.1675385805431; Thu, 02 Feb 2023 16:56:45 -0800 (PST) Received: from ?IPV6:2601:19c:527f:bfd0::3? ([2601:19c:527f:bfd0::3]) by smtp.googlemail.com with ESMTPSA id bb10-20020a05622a1b0a00b003b9a73cd120sm656025qtb.17.2023.02.02.16.56.44 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 02 Feb 2023 16:56:45 -0800 (PST) Sender: Nathan Sidwell Message-ID: <0c6cd4d0-1f01-76e0-3c90-42bb4698649a@acm.org> Date: Thu, 2 Feb 2023 19:56:44 -0500 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.6.0 Subject: Re: [PATCH] c++ modules: uninstantiated template friend class [PR104234] Content-Language: en-US To: Patrick Palka , gcc-patches@gcc.gnu.org Cc: jason@redhat.com References: <20230125201643.506666-1-ppalka@redhat.com> From: Nathan Sidwell In-Reply-To: <20230125201643.506666-1-ppalka@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-3038.7 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: That might be sufficient for this case, but temploid friends violate an assumption of the implementation -- namely that module A cannot create an entity that belongs in module B's symbol table. This causes a bunch of excitement, particularly around handling (well formed) duplicatd instantions. I'm not sure of the way to handle that, but I suspect something along the lines of a flag on such decls and a new hash table to hold these exceptions. nathan On 1/25/23 15:16, Patrick Palka wrote: > Here we're not clearing DECL_UNINSTANTIATED_TEMPLATE_FRIEND_P for > the instantiated/injected template friend class B, which confuses a > later call to get_originating_module_decl for B. This patch fixes this > by clearing the flag in tsubst_friend_class (as is already done for > template friend functions by r11-5730-gf7aeb823d9b0de). > > After fixing that, we still fail to compile the testcase, rejecting the > later definition of B with > > friend-6_a.C:10:26: error: cannot declare ‘struct B’ in a different module > > ultimately because DECL_MODULE_ATTACH_P wasn't set on the original > (injected) declaration of B. This patch fixes this by calling > set_originating_module in tsubst_friend_class, but for that to work it > seems we need to relax the assert in this latter function since > get_originating_module_decl when called on the TYPE_DECL for B returns > the corresponding TEMPLATE_DECL. > > (Alternatively we can instead call set_originating_module on the > TYPE_DECL B as soon as it's created in lookup_template_class (which is > what pushtag does), which doesn't need this assert change because at > this point the TYPE_DECL doesn't have any TEMPLATE_INFO so > get_originating_module_decl becomes a no-op. Would that be preferable?) > > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for > trunk? > > PR c++/104234 > > gcc/cp/ChangeLog: > > * module.cc (set_originating_module): Document default argument. > Relax assert to look through DECL_TEMPLATE_RESULT in the result > of get_originating_module_decl. > * pt.cc (tsubst_friend_class): Clear > DECL_UNINSTANTIATED_TEMPLATE_FRIEND_P and call > set_originating_module on the instantiated template friend class. > > gcc/testsuite/ChangeLog: > > * g++.dg/modules/friend-6_a.C: New test. > --- > gcc/cp/module.cc | 8 ++++++-- > gcc/cp/pt.cc | 3 +++ > gcc/testsuite/g++.dg/modules/friend-6_a.C | 10 ++++++++++ > 3 files changed, 19 insertions(+), 2 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/modules/friend-6_a.C > > diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc > index 7133009dba5..234ce43b70f 100644 > --- a/gcc/cp/module.cc > +++ b/gcc/cp/module.cc > @@ -18843,14 +18843,18 @@ set_defining_module (tree decl) > } > > void > -set_originating_module (tree decl, bool friend_p ATTRIBUTE_UNUSED) > +set_originating_module (tree decl, bool friend_p /* = false */) > { > set_instantiating_module (decl); > > if (!DECL_NAMESPACE_SCOPE_P (decl)) > return; > > - gcc_checking_assert (friend_p || decl == get_originating_module_decl (decl)); > + if (!friend_p) > + { > + tree o = get_originating_module_decl (decl); > + gcc_checking_assert (STRIP_TEMPLATE (o) == decl); > + } > > if (module_attach_p ()) > { > diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc > index cbe5898b553..f2ee74025e7 100644 > --- a/gcc/cp/pt.cc > +++ b/gcc/cp/pt.cc > @@ -11520,6 +11520,9 @@ tsubst_friend_class (tree friend_tmpl, tree args) > CLASSTYPE_TI_ARGS (TREE_TYPE (tmpl)) > = INNERMOST_TEMPLATE_ARGS (CLASSTYPE_TI_ARGS (TREE_TYPE (tmpl))); > > + DECL_UNINSTANTIATED_TEMPLATE_FRIEND_P (tmpl) = false; > + set_originating_module (DECL_TEMPLATE_RESULT (tmpl)); > + > /* Substitute into and set the constraints on the new declaration. */ > if (tree ci = get_constraints (friend_tmpl)) > { > diff --git a/gcc/testsuite/g++.dg/modules/friend-6_a.C b/gcc/testsuite/g++.dg/modules/friend-6_a.C > new file mode 100644 > index 00000000000..97017e4ee78 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/friend-6_a.C > @@ -0,0 +1,10 @@ > +// PR c++/104234 > +// { dg-additional-options "-fmodules-ts" } > +// { dg-module-cmi pr104234 } > +export module pr104234; > + > +template struct A { > + template friend struct B; > +}; > +A a; > +template struct B { }; -- Nathan Sidwell