From patchwork Mon Jan 8 09:57:35 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nathaniel Shead X-Patchwork-Id: 83520 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 CD763385E838 for ; Mon, 8 Jan 2024 09:58:15 +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 10376385842A for ; Mon, 8 Jan 2024 09:57:42 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 10376385842A 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 10376385842A 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=1704707864; cv=none; b=nvi+1HH4fvUqAWrjtsQ4wc8AD7KtZxvKP2qDTwOFzMR5PEVs5Tp++zbbmeep6UKD5GK3CEmksOujLbNsf+xROEQo9CiyZr39AY9206OoOSE1pIZOZNGoTw7ZCGzmhar72f9WdL563Up1JzuYaGPWhqzZx09VZuYMtQK735oTXfM= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1704707864; c=relaxed/simple; bh=8HsOOsEJMmPZtXcayL9fT/3dHJdfy2mOgGTPUXATEdA=; h=DKIM-Signature:Message-ID:Date:From:To:Subject:MIME-Version; b=mNXXcbMxQVfLmdoy5TuQ7Sbsh7RxhcfLp5wdwuR6GNq15UcmDVFTx+gQEqIYatDNPUc7bpxJP+yOf6HeZaAjZTOHZ4ok7bXk9/ot+ZRZK6BWJOzIcyrrPxVKPKrfO3gugHC2EKpWb/yn59S4K08cO6mIWj2BrlFd1nBACpNNrK4= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-pg1-x52f.google.com with SMTP id 41be03b00d2f7-5ceb3fe708eso210652a12.3 for ; Mon, 08 Jan 2024 01:57:42 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1704707861; x=1705312661; 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=jZgdpAqv68YlAuPDU0ithpY4s+xoGrRPLRW1yXozCHw=; b=jd2IJgve7hfPPfshT1oEzq00aZuLH8xWdUhAYVBSWgjhK6+kPOxQefAI8Y3bDb8q0g oePqSrXa0yOqGgtzj+ZBedulmUxI/V7q57bjfnkaTZrdEtrXrxO0/0MxNIH58jAs8teO 2nRKHd79qNa8i5sOY2nadmsSv6cuRaEPkEhPO8TsYAiEXCIwLwWnqKJgzVFcErM0FZ0s E69Dyr0OoUrTtbrH3EwVSs5q0roQ07odmbS7dPR0ysiJLsRrv8DGyPDLiysPWZoVwiT+ Y2Z0Ptkulk+FrCv6DbPIWbx64ip1qt5V49fzeM0z8fAP9hLB6VbOkYx1r7QQmEQaqlG/ w8Dw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1704707861; x=1705312661; 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=jZgdpAqv68YlAuPDU0ithpY4s+xoGrRPLRW1yXozCHw=; b=jppdHXvTjOmhdmjGQxdxUYfTyp1Wj5O4zGqTg4aa1ueo8Wusy3jPET3+im6q/LDkd/ kDV5OQlET7iQ7NQbSumwjVQqbgw1GuI8WDncDHjpapyRUuCXldW45jyJHOjZl+xJFybA Jz4wsJ3fyv2KKvW0Z5gqb1iVPl62qqCMhryljPjP4UUiNOiPLY0gimVrwjO2hJg1Z2v9 iBF3xnwe1keDHKIFrk1EnhN+wVTCbMSipArVjWFtZ4b1Qx9JIFhMwguJAXTt+GW/1VKF IP89jBX/RKLV8MIpXy/RWnalakTh2BHzc0O4BoJvlrD2t5Fcax+21sBXI2nNlxVHQkEP 1DbQ== X-Gm-Message-State: AOJu0YxtxRXsJXZVgcCRCS7zO4TRRe2PjcgJBZqc54kQFdvRCmMwut04 dhcb51FBCgxbllaLbZl10tc= X-Google-Smtp-Source: AGHT+IF4IIut1nQ6lqoMg+NHvF0Mecxa7OGNSBpT5AlTAiyPqjb+wD71mkILXMzdYHwA+Or7MK6IkA== X-Received: by 2002:a17:903:1248:b0:1d4:f114:62b5 with SMTP id u8-20020a170903124800b001d4f11462b5mr1269722plh.89.1704707860767; Mon, 08 Jan 2024 01:57:40 -0800 (PST) Received: from Thaum. (60-241-118-84.tpgi.com.au. [60.241.118.84]) by smtp.gmail.com with ESMTPSA id w5-20020a170902d3c500b001d07d83fdd0sm5876523plb.238.2024.01.08.01.57.38 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 08 Jan 2024 01:57:40 -0800 (PST) Message-ID: <659bc714.170a0220.c04dd.cc6a@mx.google.com> X-Google-Original-Message-ID: Date: Mon, 8 Jan 2024 20:57:35 +1100 From: Nathaniel Shead To: Patrick Palka Cc: gcc-patches@gcc.gnu.org, Jason Merrill , Nathan Sidwell Subject: [PATCH v2] c++/modules: Differentiate extern templates and TYPE_DECL_SUPPRESS_DEBUG [PR112820] References: <656c78b2.170a0220.62689.59e8@mx.google.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-Spam-Status: No, score=-10.1 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, RCVD_IN_BARRACUDACENTRAL, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE 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 On Thu, Jan 04, 2024 at 03:39:15PM -0500, Patrick Palka wrote: > On Sun, 3 Dec 2023, Nathaniel Shead wrote: > > > Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk? > > > > -- >8 -- > > > > The TYPE_DECL_SUPPRESS_DEBUG and DECL_EXTERNAL flags use the same > > underlying bit. This is causing confusion when attempting to determine > > the interface for a streamed-in class type, since the modules code > > currently assumes that all DECL_EXTERNAL types are extern templates. > > However, when -g is specified then TYPE_DECL_SUPPRESS_DEBUG (and hence > > DECL_EXTERNAL) is marked on various other kinds of declarations, such as > > vtables, which causes them to never be emitted. > > Good catch.. Maybe we should use different bits for these flags? I wouldn't be > surprised if this bit sharing causes issues elsewhere in the compiler. The > documentation in tree.h / tree-core.h says DECL_EXTERNAL is only valid for > VAR_DECL and FUNCTION_DECL, so at one point it was safe to share the same bit > but that's not true anymore it seems. > > Looking at tree-core.h:tree_decl_common luckily we have plenty of spare bits. > We could also e.g. make TYPE_DECL_SUPPRESS_DEBUG use the decl_not_flexarray bit > which is otherwise only used for FIELD_DECL. > That seems like a good idea, thanks. How does this look? Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk? -- >8 -- Currently, DECL_EXTERNAL and TYPE_DECL_SUPPRESS_DEBUG share a bit. This causes issues with module code, which then incorrectly assumes that anything with suppressed debug info (such as vtables when '-g' is specified) is an extern template and thus prevents their emission. This patch splits the two flags up; extern templates continue to use the DECL_EXTERNAL flag (and the documentation is updated to indicate this), but TYPE_DECL_SUPPRESS_DEBUG now uses the 'decl_not_flexarray' flag, which currently is only used by FIELD_DECLs. PR c++/112820 PR c++/102607 gcc/cp/ChangeLog: * pt.cc (mark_class_instantiated): Set DECL_EXTERNAL explicitly. gcc/ChangeLog: * tree-core.h (struct tree_decl_common): Update comments. * tree.h (DECL_EXTERNAL): Update comments. (TYPE_DECL_SUPPRESS_DEBUG): Use 'decl_not_flexarray' instead. gcc/testsuite/ChangeLog: * g++.dg/modules/debug-2_a.C: New test. * g++.dg/modules/debug-2_b.C: New test. * g++.dg/modules/debug-2_c.C: New test. * g++.dg/modules/debug-3_a.C: New test. * g++.dg/modules/debug-3_b.C: New test. Signed-off-by: Nathaniel Shead --- gcc/cp/pt.cc | 1 + gcc/testsuite/g++.dg/modules/debug-2_a.C | 9 +++++++++ gcc/testsuite/g++.dg/modules/debug-2_b.C | 8 ++++++++ gcc/testsuite/g++.dg/modules/debug-2_c.C | 9 +++++++++ gcc/testsuite/g++.dg/modules/debug-3_a.C | 8 ++++++++ gcc/testsuite/g++.dg/modules/debug-3_b.C | 9 +++++++++ gcc/tree-core.h | 6 +++--- gcc/tree.h | 8 ++++---- 8 files changed, 51 insertions(+), 7 deletions(-) create mode 100644 gcc/testsuite/g++.dg/modules/debug-2_a.C create mode 100644 gcc/testsuite/g++.dg/modules/debug-2_b.C create mode 100644 gcc/testsuite/g++.dg/modules/debug-2_c.C create mode 100644 gcc/testsuite/g++.dg/modules/debug-3_a.C create mode 100644 gcc/testsuite/g++.dg/modules/debug-3_b.C diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc index e38e7a773f0..7839745035b 100644 --- a/gcc/cp/pt.cc +++ b/gcc/cp/pt.cc @@ -26256,6 +26256,7 @@ mark_class_instantiated (tree t, int extern_p) SET_CLASSTYPE_EXPLICIT_INSTANTIATION (t); SET_CLASSTYPE_INTERFACE_KNOWN (t); CLASSTYPE_INTERFACE_ONLY (t) = extern_p; + DECL_EXTERNAL (TYPE_NAME (t)) = extern_p; TYPE_DECL_SUPPRESS_DEBUG (TYPE_NAME (t)) = extern_p; if (! extern_p) { diff --git a/gcc/testsuite/g++.dg/modules/debug-2_a.C b/gcc/testsuite/g++.dg/modules/debug-2_a.C new file mode 100644 index 00000000000..eed0905542b --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/debug-2_a.C @@ -0,0 +1,9 @@ +// PR c++/112820 +// { dg-additional-options "-fmodules-ts -g" } +// { dg-module-cmi io } + +export module io; + +export struct error { + virtual const char* what() const noexcept; +}; diff --git a/gcc/testsuite/g++.dg/modules/debug-2_b.C b/gcc/testsuite/g++.dg/modules/debug-2_b.C new file mode 100644 index 00000000000..fc9afbc02e0 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/debug-2_b.C @@ -0,0 +1,8 @@ +// PR c++/112820 +// { dg-additional-options "-fmodules-ts -g" } + +module io; + +const char* error::what() const noexcept { + return "bla"; +} diff --git a/gcc/testsuite/g++.dg/modules/debug-2_c.C b/gcc/testsuite/g++.dg/modules/debug-2_c.C new file mode 100644 index 00000000000..37117f69dcd --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/debug-2_c.C @@ -0,0 +1,9 @@ +// PR c++/112820 +// { dg-module-do link } +// { dg-additional-options "-fmodules-ts -g" } + +import io; + +int main() { + error{}; +} diff --git a/gcc/testsuite/g++.dg/modules/debug-3_a.C b/gcc/testsuite/g++.dg/modules/debug-3_a.C new file mode 100644 index 00000000000..9e33d8260fd --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/debug-3_a.C @@ -0,0 +1,8 @@ +// PR c++/102607 +// { dg-additional-options "-fmodules-ts -g" } +// { dg-module-cmi mod } + +export module mod; +export struct B { + virtual ~B() = default; +}; diff --git a/gcc/testsuite/g++.dg/modules/debug-3_b.C b/gcc/testsuite/g++.dg/modules/debug-3_b.C new file mode 100644 index 00000000000..03c78b71b5d --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/debug-3_b.C @@ -0,0 +1,9 @@ +// PR c++/102607 +// { dg-module-do link } +// { dg-additional-options "-fmodules-ts -g" } + +import mod; +int main() { + struct D : B {}; + (void)D{}; +} diff --git a/gcc/tree-core.h b/gcc/tree-core.h index 8a89462bd7e..1ca4d4c07bd 100644 --- a/gcc/tree-core.h +++ b/gcc/tree-core.h @@ -1814,8 +1814,7 @@ struct GTY(()) tree_decl_common { In FIELD_DECL, this is DECL_FIELD_ABI_IGNORED. */ unsigned decl_flag_0 : 1; /* In FIELD_DECL, this is DECL_BIT_FIELD - In VAR_DECL and FUNCTION_DECL, this is DECL_EXTERNAL. - In TYPE_DECL, this is TYPE_DECL_SUPPRESS_DEBUG. */ + In VAR_DECL, FUNCTION_DECL, and TYPE_DECL, this is DECL_EXTERNAL. */ unsigned decl_flag_1 : 1; /* In FIELD_DECL, this is DECL_NONADDRESSABLE_P In VAR_DECL, PARM_DECL and RESULT_DECL, this is @@ -1845,7 +1844,8 @@ struct GTY(()) tree_decl_common { TYPE_WARN_IF_NOT_ALIGN. */ unsigned int warn_if_not_align : 6; - /* In FIELD_DECL, this is DECL_NOT_FLEXARRAY. */ + /* In FIELD_DECL, this is DECL_NOT_FLEXARRAY. + In TYPE_DECL, this is TYPE_DECL_SUPPRESS_DEBUG. */ unsigned int decl_not_flexarray : 1; /* 5 bits unused. */ diff --git a/gcc/tree.h b/gcc/tree.h index 972a067a1f7..8bdc471ad4e 100644 --- a/gcc/tree.h +++ b/gcc/tree.h @@ -2848,9 +2848,9 @@ extern tree vector_element_bits_tree (const_tree); #define DECL_LANG_SPECIFIC(NODE) \ (DECL_COMMON_CHECK (NODE)->decl_common.lang_specific) -/* In a VAR_DECL or FUNCTION_DECL, nonzero means external reference: - do not allocate storage, and refer to a definition elsewhere. Note that - this does not necessarily imply the entity represented by NODE +/* In a VAR_DECL, FUNCTION_DECL, or TYPE_DECL, nonzero means external + reference: do not allocate storage, and refer to a definition elsewhere. + Note that this does not necessarily imply the entity represented by NODE has no program source-level definition in this translation unit. For example, for a FUNCTION_DECL, DECL_SAVED_TREE may be non-NULL and DECL_EXTERNAL may be true simultaneously; that can be the case for @@ -3553,7 +3553,7 @@ extern vec **decl_debug_args_insert (tree); into stabs. Instead it will generate cross reference ('x') of names. This uses the same flag as DECL_EXTERNAL. */ #define TYPE_DECL_SUPPRESS_DEBUG(NODE) \ - (TYPE_DECL_CHECK (NODE)->decl_common.decl_flag_1) + (TYPE_DECL_CHECK (NODE)->decl_common.decl_not_flexarray) /* Getter of the imported declaration associated to the IMPORTED_DECL node. */