From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qv1-xf33.google.com (mail-qv1-xf33.google.com [IPv6:2607:f8b0:4864:20::f33]) by sourceware.org (Postfix) with ESMTPS id D2A0B3858D28 for ; Tue, 11 Oct 2022 17:40:19 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org D2A0B3858D28 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-qv1-xf33.google.com with SMTP id i9so9434208qvo.0 for ; Tue, 11 Oct 2022 10:40:19 -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:message-id:reply-to; bh=1+WIVx3LU3dGmq4/C1Aphg5PUHF2SYk8C64QK1FcdYU=; b=pF1MO4mOGz0S9+Wtq2zj7T0BSZosio9r2QOZrRdanoaHKwWDW0/MZm5cT1FxXV08OV LJdKxHzvqoe3cNQuld4RL56UhOEWZa1j3aioShUCLI87JJVlxA5vowLXxAW/isgWG15B v76HavznYjXCxZmx8UACWAbxNKBZ1rr1Q/CsVwAH/8IUIfJQHwAuigQHzm5z9dkixeEa 8GfH0WuefdIsElmQEeeI8aSzHRiWE+oxlh+XfGlQG07SQYyHhND1/Rf/DXVKpr6NfWnV XBfZIKx0t17RR+0hRpkCKtQrqRukC5YU5Y/qNG8WFud84qHDobUw6X8NXJILVDa4QCQk Siww== 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=1+WIVx3LU3dGmq4/C1Aphg5PUHF2SYk8C64QK1FcdYU=; b=ZV35uqP8rjEGlFXDaDOFwAlgcRVa3qriNrK1SI/oNjvMXYgRzZSuPSNhhA32k/NRom OUcf8bnIj/kfvAn8ZH2VBY1hgJDnb8RBLr+6QwedjcGHzxuMLJDqYw0wss6xMzYhEdNZ WIy+q/ApAFQ4AIj9C61aEFo77MHB4nCFHtxDsPJpVPnE6WLFpp3oHCq3r/mNZ5oQd1pG 9yer2nn3sLQ2I1Ft7T7FYYWgCNkFJ3yN7S7dfAjFHnjpy0IQxjoj37nWFXZ2NRmDfTB9 852Xoqli+PjIV1MFOsnWZ3jUwR5eMN+2nWfWFSCQYXsgNUmDtpc/ybTtmQaGpA6Z9oMn 6Y5Q== X-Gm-Message-State: ACrzQf0KbDc7W0LGlLH2egQZX5Vf/Ast08HqLaFp+Yg4CjA4W1HD6L+D 6+7uCS4+8jPdB8TCXa9K39A= X-Google-Smtp-Source: AMsMyM5M20aAHUuso8ivetClKMlKUexHKfNLyY3dooZdcRqLadCI5EpZ/ematFbih5+rjZ31wB+p7w== X-Received: by 2002:a05:6214:e6f:b0:4b3:f41c:a59 with SMTP id jz15-20020a0562140e6f00b004b3f41c0a59mr10595428qvb.59.1665510019073; Tue, 11 Oct 2022 10:40:19 -0700 (PDT) Received: from ?IPV6:2620:10d:c0a8:11c9::1442? ([2620:10d:c091:480::bdde]) by smtp.googlemail.com with ESMTPSA id 65-20020a370844000000b006ec51f95e97sm8755504qki.67.2022.10.11.10.40.17 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 11 Oct 2022 10:40:18 -0700 (PDT) Sender: Nathan Sidwell Message-ID: <4a5f7f70-9626-f067-0ba9-9bf993da8a34@acm.org> Date: Tue, 11 Oct 2022 13:40:17 -0400 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.3.1 Subject: Re: [PATCH] c++ modules: ICE with templated friend and std namespace [PR100134] Content-Language: en-US To: Patrick Palka , gcc-patches@gcc.gnu.org Cc: jason@redhat.com References: <20221011153507.784631-1-ppalka@redhat.com> From: Nathan Sidwell In-Reply-To: <20221011153507.784631-1-ppalka@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-3040.6 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 10/11/22 11:35, Patrick Palka wrote: > IIUC the function depset::hash::add_binding_entity has an assert > verifying that if a namespace contains an exported entity, then > the namespace must have been opened in the module purview: > > if (data->hash->add_namespace_entities (decl, data->partitions)) > { > /* It contains an exported thing, so it is exported. */ > gcc_checking_assert (DECL_MODULE_PURVIEW_P (decl)); > DECL_MODULE_EXPORT_P (decl) = true; > } > > We're tripping over this assert in the below testcase because by > instantiating and exporting std::A, we end up in turn defining > and exporting the hidden friend std::f without ever having opening > the enclosing namespace std within the module purview and thus > DECL_MODULE_PURVIEW_P (std_node) is false. > > Note that it's important that the enclosing namespace is std here: if we > use a different namespace then the ICE disappears. This probably has > something to do with the fact that we predefine std via push_namespace > from cxx_init_decl_processing (which makes it look like we've opened the > namespace in the TU), whereas with another namespace we would instead > lazily obtain the NAMESPACE_DECL from add_imported_namespace. > > Since templated frined functions are special in that they give us a way > to declare a new namespace-scope function without having to explicitly > open the namespace, this patch proposes to fix this issue by propagating > DECL_MODULE_PURVIEW_P from a friend function to the enclosing namespace > when instantiating the friend. ouch. This is ok, but I think we have a bug -- what is the module ownership of the friend introduced by the instantiation? Haha, there's a note on 13.7.5/3 -- the attachment is to the same module as the befriending class. That means we end up creating and writing out entities that exist in the symbol table (albeit hidden) whose module ownership is neither the global module or the tu's module. That's not something the module machinery anticipates. We'll get the mangling wrong for starters. Hmm. These are probably rare. Thinking about the right solution though ... nathan > > Tested on x86_64-pc-linux-gnu, does this look like the right fix? Other > solutions that seem to work are to set DECL_MODULE_PURVIEW_P on std_node > after the fact from declare_module, or simply to suppress the assert for > std_node. > > PR c++/100134 > > gcc/cp/ChangeLog: > > * pt.cc (tsubst_friend_function): Propagate DECL_MODULE_PURVIEW_P > from the new declaration to the enclosing namespace scope. > > gcc/testsuite/ChangeLog: > > * g++.dg/modules/tpl-friend-8_a.H: New test. > * g++.dg/modules/tpl-friend-8_b.C: New test. > --- > gcc/cp/pt.cc | 7 +++++++ > gcc/testsuite/g++.dg/modules/tpl-friend-8_a.H | 9 +++++++++ > gcc/testsuite/g++.dg/modules/tpl-friend-8_b.C | 8 ++++++++ > 3 files changed, 24 insertions(+) > create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-8_a.H > create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-8_b.C > > diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc > index 5b9fc588a21..9e3085f3fa6 100644 > --- a/gcc/cp/pt.cc > +++ b/gcc/cp/pt.cc > @@ -11448,6 +11448,13 @@ tsubst_friend_function (tree decl, tree args) > by duplicate_decls. */ > new_friend = old_decl; > } > + > + /* We've just added a new namespace-scope entity to the purview without > + necessarily having opened the enclosing namespace, so make sure the > + enclosing namespace is in the purview now too. */ > + if (TREE_CODE (DECL_CONTEXT (new_friend)) == NAMESPACE_DECL) > + DECL_MODULE_PURVIEW_P (DECL_CONTEXT (new_friend)) > + |= DECL_MODULE_PURVIEW_P (STRIP_TEMPLATE (new_friend)); > } > else > { > diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-8_a.H b/gcc/testsuite/g++.dg/modules/tpl-friend-8_a.H > new file mode 100644 > index 00000000000..bd2290460b5 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-8_a.H > @@ -0,0 +1,9 @@ > +// PR c++/100134 > +// { dg-additional-options -fmodule-header } > +// { dg-module-cmi {} } > + > +namespace std { > + template struct A { > + friend void f(A) { } > + }; > +} > diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-8_b.C b/gcc/testsuite/g++.dg/modules/tpl-friend-8_b.C > new file mode 100644 > index 00000000000..76d7447c2eb > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-8_b.C > @@ -0,0 +1,8 @@ > +// PR c++/100134 > +// { dg-additional-options -fmodules-ts } > +// { dg-module-cmi pr100134 } > +export module pr100134; > + > +import "tpl-friend-8_a.H"; > + > +export std::A a; -- Nathan Sidwell