From patchwork Tue Jul 9 09:16:08 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nathaniel Shead X-Patchwork-Id: 1958242 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20230601 header.b=HbzAgO8A; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=2620:52:3:1:0:246e:9693:128c; helo=server2.sourceware.org; envelope-from=gcc-patches-bounces~incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=patchwork.ozlabs.org) Received: from server2.sourceware.org (server2.sourceware.org [IPv6:2620:52:3:1:0:246e:9693:128c]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (secp384r1) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4WJFjp4Jrjz1xqc for ; Tue, 9 Jul 2024 19:16:45 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 17DA838471DA for ; Tue, 9 Jul 2024 09:16:43 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mail-pg1-x52f.google.com (mail-pg1-x52f.google.com [IPv6:2607:f8b0:4864:20::52f]) by sourceware.org (Postfix) with ESMTPS id 67F7E3858420 for ; Tue, 9 Jul 2024 09:16:18 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 67F7E3858420 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 67F7E3858420 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::52f ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1720516581; cv=none; b=pn+ZKybbO/WJ3LRyXF6zD+5JFjmZAbkyvhdFOES+i8NgX8T0w1mNg4/P3eTOAHW2cmCwQfWeD0bRTfmO8zefZOHlDFW7UndVF4jguNlXNFjfZy8KPgEXxFsYVv6zRzNSGDd4D029aOSrQbtECMrhtJsTo95AVEmjFpGcFYPwJjo= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1720516581; c=relaxed/simple; bh=NZRbi6nB1vgNqw+Jo9RrSR1o3eTghHdlEvtH038npWE=; h=DKIM-Signature:Message-ID:Date:From:To:Subject:MIME-Version; b=JKKknOn35AnyiLNrj/Bg3DtWZNglfS9vilZG2wij+L7MOnnq9Ft/wrhPvN/hkKNJMNYSkUwN0qCJI3SEyfJ/PNghQP/evZe4yLk0HCYIIfUaNzvkn1B7q3X9Vo2KI066tyz52OfH9em+nzcs0xxWTwJO8vsLS3Qg/uVQXMGkMts= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-pg1-x52f.google.com with SMTP id 41be03b00d2f7-710437d0affso2728300a12.3 for ; Tue, 09 Jul 2024 02:16:18 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1720516575; x=1721121375; darn=gcc.gnu.org; h=in-reply-to:content-disposition:mime-version:references:subject:cc :to:from:date:message-id:from:to:cc:subject:date:message-id:reply-to; bh=OM9W2lm5qK6vxhewN4h81vqHPqtn+9KTKL5hypYBNAw=; b=HbzAgO8Aw+O1pT5NT5srC6vuh0EDbpd7ZILF6jte/njg1K5ep2HusHbsTKuIe6EQmk 3zLjIxUOyYlqWRlca1gwUJkXS+qaB2KYdPtD9ickPhbekV0WPnH/77dWcvKsxDlw5wLt jrjnFlQwbWpAxB8an4aC9XiGhCE7IhZ2lhZtHnzypql1DTsA1Tnj8XziBZg39jCUhy3H aYH9wVpocuhZrcusJdBhJBpNqYG87u3KuAn0W1Mf6WDpE6No3Ov9C1AXtOIJRUp7XXYl sO+AIkGEjEIl5trcwyLfs2DKYLyets93to+euDkeK+1yz5iOGRiTdlVBegWqm2Zy9hLd YIgw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1720516575; x=1721121375; h=in-reply-to:content-disposition:mime-version:references:subject:cc :to:from:date:message-id:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=OM9W2lm5qK6vxhewN4h81vqHPqtn+9KTKL5hypYBNAw=; b=XK2QWZ24Jlq5NiDVgh1pb/3DdIsCrXnA6dGAMzwl4iEXVG+uwzMnMdRAs+BLT3ht1z umOK7VdlcnOlxsqN+RCRnXyhFmvooJNisIQIxiMGPSEQ978vb+uckohi4/mZh5wFWi0k ZB4WutW6MzfbhVVtfixIJje2gBPPJFcF4XU3mUSADiNXVUisfP0ZmAEqZy0/cWPgdgmY P0ZXEDwRqFuPHmJkTn4U/68qfM+h9uXad22pOeFZZv5powQenJWJarRLdhVbcvRA32O2 XRSrKMpdDFfTemf/VH+f3twdSS0dFUzvUCjlgpzeyAy/QoB4dzu18AnVTaig420cKERY 2ZEw== X-Gm-Message-State: AOJu0YzYChmmtHBBMWZScAz6R1pi0TVqB5MDjPZwLZNt2OMoST+fcNRS fyJGMgYbLKu0T3eOSJpctAKw9uiT3JUTUhJCFzKdzhu/ZWg67yPb X-Google-Smtp-Source: AGHT+IEqsI8Um5GhGacR0lwf6IsEryMeSt50LAaoL6ULr/HoauIvjN9GRkJKTBaheaGz3pFSi5dFMw== X-Received: by 2002:a05:6a21:38b:b0:1c2:92a1:9320 with SMTP id adf61e73a8af0-1c298214963mr1843214637.7.1720516574693; Tue, 09 Jul 2024 02:16:14 -0700 (PDT) Received: from Thaum. (125-209-140-42.tpgi.com.au. [125.209.140.42]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-1fbb6ab79e6sm11572025ad.141.2024.07.09.02.16.12 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 09 Jul 2024 02:16:14 -0700 (PDT) Message-ID: <668cffde.170a0220.a8f8b.42fe@mx.google.com> X-Google-Original-Message-ID: Date: Tue, 9 Jul 2024 19:16:08 +1000 From: Nathaniel Shead To: Patrick Palka Cc: gcc-patches@gcc.gnu.org, Jason Merrill , Nathan Sidwell Subject: [PATCH v2] c++/modules: Keep entity mapping info across duplicate_decls [PR99241] References: <668b6ef5.170a0220.fa269.4683@mx.google.com> <009c5a23-b19a-209a-bb38-1c06e33a3a38@idea> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <009c5a23-b19a-209a-bb38-1c06e33a3a38@idea> X-Spam-Status: No, score=-12.2 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, 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 X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gcc-patches-bounces~incoming=patchwork.ozlabs.org@gcc.gnu.org On Mon, Jul 08, 2024 at 11:21:55AM -0400, Patrick Palka wrote: > On Mon, 8 Jul 2024, Nathaniel Shead wrote: > > > Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk/14? > > > > -- >8 -- > > > > When duplicate_decls finds a match with an existing imported > > declaration, it clears DECL_LANG_SPECIFIC of the olddecl and replaces it > > with the contents of newdecl; this clears DECL_MODULE_ENTITY_P causing > > an ICE if the same declaration is imported again later. > > > > This fixes the issue by ensuring that the flag is transferred to newdecl > > before clearing so that it ends up on olddecl again. > > > > For future-proofing we also do the same with DECL_MODULE_KEYED_DECLS_P, > > though because we don't yet support textual redefinition merging we > > can't yet test this works as intended. I don't expect it's possible for > > a new declaration already to have extra keyed decls mismatching that of > > the old declaration though, so I don't do anything with 'keyed_map' at > > this time. > > Makes sense, thanks for tracking this down! > > > > > PR c++/99241 > > > > gcc/cp/ChangeLog: > > > > * decl.cc (duplicate_decls): Merge module entity information. > > > > gcc/testsuite/ChangeLog: > > > > * g++.dg/modules/pr99241_a.H: New test. > > * g++.dg/modules/pr99241_b.H: New test. > > * g++.dg/modules/pr99241_c.C: New test. > > > > Signed-off-by: Nathaniel Shead > > --- > > gcc/cp/decl.cc | 17 +++++++++++++++++ > > gcc/testsuite/g++.dg/modules/pr99241_a.H | 3 +++ > > gcc/testsuite/g++.dg/modules/pr99241_b.H | 3 +++ > > gcc/testsuite/g++.dg/modules/pr99241_c.C | 5 +++++ > > 4 files changed, 28 insertions(+) > > create mode 100644 gcc/testsuite/g++.dg/modules/pr99241_a.H > > create mode 100644 gcc/testsuite/g++.dg/modules/pr99241_b.H > > create mode 100644 gcc/testsuite/g++.dg/modules/pr99241_c.C > > > > diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc > > index 29616100cfe..b3a770df926 100644 > > --- a/gcc/cp/decl.cc > > +++ b/gcc/cp/decl.cc > > @@ -3149,6 +3149,23 @@ duplicate_decls (tree newdecl, tree olddecl, bool hiding, bool was_hidden) > > if (TREE_CODE (newdecl) == FIELD_DECL) > > DECL_PACKED (olddecl) = DECL_PACKED (newdecl); > > > > + /* Merge module entity mapping information. */ > > + if (modules_p ()) > > + { > > + tree old_nontmpl = STRIP_TEMPLATE (olddecl); > > It seems duplicate_decls has an unconditional early exit code path for > TEMPLATE_DECL > > if (TREE_CODE (newdecl) == TEMPLATE_DECL) > { > tree old_result = DECL_TEMPLATE_RESULT (olddecl); > ... > > return olddecl; > } > > so I think these STRIP_TEMPLATEs are unneeded? (And I guess this early > exit explains why the testcase doesn't ICE when 'terminate' is a > function template.) > Ah right, thanks for spotting that. That does make it look a bit cleaner; here's an updated patch. Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk/14? -- >8 -- When duplicate_decls finds a match with an existing imported declaration, it clears DECL_LANG_SPECIFIC of the olddecl and replaces it with the contents of newdecl; this clears DECL_MODULE_ENTITY_P causing an ICE if the same declaration is imported again later. This fixes the issue by ensuring that the flag is transferred to newdecl before clearing so that it ends up on olddecl again. For future-proofing we also do the same with DECL_MODULE_KEYED_DECLS_P, though because we don't yet support textual redefinition merging we can't yet test this works as intended. I don't expect it's possible for a new declaration already to have extra keyed decls mismatching that of the old declaration though, so I don't do anything with 'keyed_map' at this time. PR c++/99241 gcc/cp/ChangeLog: * decl.cc (duplicate_decls): Merge module entity information. gcc/testsuite/ChangeLog: * g++.dg/modules/pr99241_a.H: New test. * g++.dg/modules/pr99241_b.H: New test. * g++.dg/modules/pr99241_c.C: New test. Signed-off-by: Nathaniel Shead --- gcc/cp/decl.cc | 10 ++++++++++ gcc/testsuite/g++.dg/modules/pr99241_a.H | 3 +++ gcc/testsuite/g++.dg/modules/pr99241_b.H | 3 +++ gcc/testsuite/g++.dg/modules/pr99241_c.C | 5 +++++ 4 files changed, 21 insertions(+) create mode 100644 gcc/testsuite/g++.dg/modules/pr99241_a.H create mode 100644 gcc/testsuite/g++.dg/modules/pr99241_b.H create mode 100644 gcc/testsuite/g++.dg/modules/pr99241_c.C diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc index 29616100cfe..f184aa70aa4 100644 --- a/gcc/cp/decl.cc +++ b/gcc/cp/decl.cc @@ -3149,6 +3149,16 @@ duplicate_decls (tree newdecl, tree olddecl, bool hiding, bool was_hidden) if (TREE_CODE (newdecl) == FIELD_DECL) DECL_PACKED (olddecl) = DECL_PACKED (newdecl); + /* Merge module entity mapping information. */ + if (DECL_LANG_SPECIFIC (olddecl) + && (DECL_MODULE_ENTITY_P (olddecl) + || DECL_MODULE_KEYED_DECLS_P (olddecl))) + { + retrofit_lang_decl (newdecl); + DECL_MODULE_ENTITY_P (newdecl) = DECL_MODULE_ENTITY_P (olddecl); + DECL_MODULE_KEYED_DECLS_P (newdecl) = DECL_MODULE_KEYED_DECLS_P (olddecl); + } + /* The DECL_LANG_SPECIFIC information in OLDDECL will be replaced with that from NEWDECL below. */ if (DECL_LANG_SPECIFIC (olddecl)) diff --git a/gcc/testsuite/g++.dg/modules/pr99241_a.H b/gcc/testsuite/g++.dg/modules/pr99241_a.H new file mode 100644 index 00000000000..c7031f08eb5 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/pr99241_a.H @@ -0,0 +1,3 @@ +// { dg-additional-options "-fmodule-header" } +// { dg-module-cmi {} } +void terminate(); diff --git a/gcc/testsuite/g++.dg/modules/pr99241_b.H b/gcc/testsuite/g++.dg/modules/pr99241_b.H new file mode 100644 index 00000000000..c7031f08eb5 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/pr99241_b.H @@ -0,0 +1,3 @@ +// { dg-additional-options "-fmodule-header" } +// { dg-module-cmi {} } +void terminate(); diff --git a/gcc/testsuite/g++.dg/modules/pr99241_c.C b/gcc/testsuite/g++.dg/modules/pr99241_c.C new file mode 100644 index 00000000000..7f2b1bb43ea --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/pr99241_c.C @@ -0,0 +1,5 @@ +// { dg-additional-options "-fmodules-ts -fno-module-lazy" } + +import "pr99241_a.H"; +void terminate(); +import "pr99241_b.H";