From patchwork Thu Mar 28 12:22:59 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nathaniel Shead X-Patchwork-Id: 87771 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 0EDEC3858C98 for ; Thu, 28 Mar 2024 12:23:36 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mail-pf1-x42b.google.com (mail-pf1-x42b.google.com [IPv6:2607:f8b0:4864:20::42b]) by sourceware.org (Postfix) with ESMTPS id 5C6433858D1E for ; Thu, 28 Mar 2024 12:23:06 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 5C6433858D1E 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 5C6433858D1E Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::42b ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1711628588; cv=none; b=XtSmahDVJU9b6Lh4zxKLguSsaZ+xE/cNPjf3sSToTjJ0eeg12aQX3WiUNpAn8fZml/3Z96T8TT/7+P/EkSVevtLCWgEtS6Wlls85cUpaO56vmTyuRUsmvDHP6y6HyFq4hNVwFTA7sOX/ZFT6M0fhazR45+6pMmWNj26Ssc4+fYs= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1711628588; c=relaxed/simple; bh=+SvsR1xdOpAe7mZduIRw3siCZ0v6V1FfC9fJ7UDL+cM=; h=DKIM-Signature:Message-ID:Date:From:To:Subject:MIME-Version; b=MeidCqOpBaDx5Yu30pQNiMeOJ8+ie6M95m4Otfhzr4elltvNX9hZy5Ozt/u7no6e5rFDyv8+5Vj8gUlWLO/r+18lnRgPxyPiVDg+5D4bA9TBxyZVGCTaSTSQFStrPQ3JcPqBnmvAtocne/D+IyJUlLxkJ8lhOQ+g5YXOQ/FWU5k= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-pf1-x42b.google.com with SMTP id d2e1a72fcca58-6ea838bf357so803325b3a.0 for ; Thu, 28 Mar 2024 05:23:06 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1711628585; x=1712233385; darn=gcc.gnu.org; h=content-disposition:mime-version:subject:cc:to:from:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=q0DvTss0JWrnVxQa9NGMJ3biyr+DsVSWXqfaP2AEDn4=; b=eoyGM3vBD4czsxz/VhZg1DvkrP8T3Sxwpz3VbIS+SbNuGhBXTW6qpLqaVKXU4zhMzk +ZUdGh+ylONPJXTirReRlCeg/dGInBG7+zGhJZVzJkZRWEza8E25KflRInZ7Npt9gbvh VF5oT3mUnSSfjMUJd6Ok1e//xvhkOYbHAsppv1lDhhLU5GfiSCELhxx4hrNlbwJTiwcT wMJGb/q+cYIuAnqpi5q30NYxImYgwVIkM0LSn6awRcuYuqVeps0n+DOZBQe8xFdep4m7 2ILxHusml7vjTpLmdAt9mH8RUFp+KdaulDsg1HsSizVXwZdd4HV7JWsR8rxlgX1hMAKT hdnw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1711628585; x=1712233385; h=content-disposition:mime-version:subject:cc:to:from:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=q0DvTss0JWrnVxQa9NGMJ3biyr+DsVSWXqfaP2AEDn4=; b=wYxMmrNjTyTK5HkwKdShuj7GjqE1gPM63q600W0va4lkSeBGiSdDruNT4UHwphNJfZ H+USty4S9NqCjXlxF8VlY+dPj0oHNIgpAe1cuWAioJ/wH1Dfn5GuKVRiJUXX7eRCm2F/ RWiqhHscNsfv6cvQsH4b9VJTXIgrO7/VC9pRXs8dVz/ir0tmBZ7VcEmmwFf/SFL6K5W2 ObhJG5YZo7hTG6aTbv7khAYYUNWVt1xTW+WyOh0Iu4ipmV2vGpGi6yFuHNaO9e+jtgQW nXTsBTnrrUUf8tePMDsQqGmmqSLQcyN9vEfdaE7tWqCHw9sS8UrY9m/C7H8wwU14eeaX hvag== X-Gm-Message-State: AOJu0YzcVHJ2YzHJ4znRszlIj735+cOewIQOE1Zptg5CEW8U9J6hkvaB vIKbEtYTc2YpeuLSGibGR9zFBbHvz8qJd4j4GnBUmWRSe32yNNv/HdQeEL+C X-Google-Smtp-Source: AGHT+IG/KzdnNSqsuxDFW0HdvpkbhnPvSQcY+t87jEOAHgom1qLiyFwmJ4eydty21xHlfoio/8XQLg== X-Received: by 2002:a05:6a21:3385:b0:1a3:68ff:5805 with SMTP id yy5-20020a056a21338500b001a368ff5805mr2825127pzb.44.1711628585145; Thu, 28 Mar 2024 05:23:05 -0700 (PDT) Received: from Thaum. (193-119-89-230.tpgi.com.au. [193.119.89.230]) by smtp.gmail.com with ESMTPSA id p56-20020a056a0026f800b006e4d8687f44sm1233977pfw.102.2024.03.28.05.23.02 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 28 Mar 2024 05:23:04 -0700 (PDT) Message-ID: <66056128.050a0220.c7f0.2cc8@mx.google.com> X-Google-Original-Message-ID: Date: Thu, 28 Mar 2024 23:22:59 +1100 From: Nathaniel Shead To: gcc-patches@gcc.gnu.org Cc: Jason Merrill , Nathan Sidwell , Patrick Palka Subject: [PATCH] c++/modules: Prefer partition indexes when installing imported entities [PR99377] MIME-Version: 1.0 Content-Disposition: inline X-Spam-Status: No, score=-11.3 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, URIBL_BLACK 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+patchwork=sourceware.org@gcc.gnu.org Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk? -- >8 -- The testcase in comment 15 of the linked PR is caused because the following assumption in depset::hash::make_dependency doesn't hold: if (DECL_LANG_SPECIFIC (not_tmpl) && DECL_MODULE_IMPORT_P (not_tmpl)) { /* Store the module number and index in cluster/section, so we don't have to look them up again. */ unsigned index = import_entity_index (decl); module_state *from = import_entity_module (index); /* Remap will be zero for imports from partitions, which we want to treat as-if declared in this TU. */ if (from->remap) { dep->cluster = index - from->entity_lwm; dep->section = from->remap; dep->set_flag_bit (); } } This is because at least for template specialisations, we first see the declaration in the header unit imported from the partition, and then the instantiation provided by the partition itself. This means that the 'import_entity_index' lookup doesn't report that the specialisation was declared in the partition and thus should be considered as-if it was part of the TU, and get exported. To fix this, this patch allows, as a special case for installing an entity from a partition, to overwrite the entity_map entry with the (later) index into the partition so that this assumption holds again. We only do this for the first time we override with a partition, so that entities are at least still reported as originating from the first imported partition that declares them (rather than the last); existing tests check for this and this seems to be a friendlier approach to go for, albeit slightly more expensive. PR c++/99377 gcc/cp/ChangeLog: * module.cc (trees_in::install_entity): Overwrite entity map index if installing from a partition. gcc/testsuite/ChangeLog: * g++.dg/modules/pr99377-3_a.H: New test. * g++.dg/modules/pr99377-3_b.C: New test. * g++.dg/modules/pr99377-3_c.C: New test. * g++.dg/modules/pr99377-3_d.C: New test. Signed-off-by: Nathaniel Shead --- gcc/cp/module.cc | 13 +++++++++++++ gcc/testsuite/g++.dg/modules/pr99377-3_a.H | 17 +++++++++++++++++ gcc/testsuite/g++.dg/modules/pr99377-3_b.C | 10 ++++++++++ gcc/testsuite/g++.dg/modules/pr99377-3_c.C | 5 +++++ gcc/testsuite/g++.dg/modules/pr99377-3_d.C | 8 ++++++++ 5 files changed, 53 insertions(+) create mode 100644 gcc/testsuite/g++.dg/modules/pr99377-3_a.H create mode 100644 gcc/testsuite/g++.dg/modules/pr99377-3_b.C create mode 100644 gcc/testsuite/g++.dg/modules/pr99377-3_c.C create mode 100644 gcc/testsuite/g++.dg/modules/pr99377-3_d.C diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc index 8aab9ea0bae..55ca17a88da 100644 --- a/gcc/cp/module.cc +++ b/gcc/cp/module.cc @@ -7649,6 +7649,19 @@ trees_in::install_entity (tree decl) gcc_checking_assert (!existed); slot = ident; } + else if (state->is_partition ()) + { + /* The decl is already in the entity map but we see it again now from a + partition: we want to overwrite if the original decl wasn't also from + a (possibly different) partition. Otherwise, for things like template + instantiations, make_dependency might not realise that this is also + provided from a partition and should be considered part of this module + (and thus always exported). */ + unsigned *slot = entity_map->get (DECL_UID (decl)); + module_state *imp = import_entity_module (*slot); + if (!imp->is_partition ()) + *slot = ident; + } return true; } diff --git a/gcc/testsuite/g++.dg/modules/pr99377-3_a.H b/gcc/testsuite/g++.dg/modules/pr99377-3_a.H new file mode 100644 index 00000000000..580a7631ae1 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/pr99377-3_a.H @@ -0,0 +1,17 @@ +// { dg-additional-options "-fmodule-header" } +// { dg-module-cmi {} } + +template +struct Widget +{ + Widget (int) { } + + bool First() const { return true; } + + bool Second () const { return true;} +}; + +inline void Frob (const Widget& w) noexcept +{ + w.First (); +} diff --git a/gcc/testsuite/g++.dg/modules/pr99377-3_b.C b/gcc/testsuite/g++.dg/modules/pr99377-3_b.C new file mode 100644 index 00000000000..5cbce7b3544 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/pr99377-3_b.C @@ -0,0 +1,10 @@ +// { dg-additional-options "-fmodules-ts" } +// { dg-module-cmi Foo:check } + +export module Foo:check; +import "pr99377-3_a.H"; + +export inline bool Check (const Widget& w) +{ + return w.Second (); +} diff --git a/gcc/testsuite/g++.dg/modules/pr99377-3_c.C b/gcc/testsuite/g++.dg/modules/pr99377-3_c.C new file mode 100644 index 00000000000..fa7c24203bd --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/pr99377-3_c.C @@ -0,0 +1,5 @@ +// { dg-additional-options "-fmodules-ts" } +// { dg-module-cmi Foo } + +export module Foo; +export import :check; diff --git a/gcc/testsuite/g++.dg/modules/pr99377-3_d.C b/gcc/testsuite/g++.dg/modules/pr99377-3_d.C new file mode 100644 index 00000000000..cb1f28321b1 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/pr99377-3_d.C @@ -0,0 +1,8 @@ +// { dg-module-do link } +// { dg-additional-options "-fmodules-ts" } + +import Foo; + +int main() { + return Check(0) ? 0 : 1; +}