From patchwork Thu Feb 29 20:56:39 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Palka X-Patchwork-Id: 1906474 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=UuvzFU9A; 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 4Tm3Sr2r5Cz1yX7 for ; Fri, 1 Mar 2024 07:57:32 +1100 (AEDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 4BDB23858431 for ; Thu, 29 Feb 2024 20:57:30 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id A30F23858C53 for ; Thu, 29 Feb 2024 20:56:45 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org A30F23858C53 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org A30F23858C53 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.129.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1709240209; cv=none; b=HlydzGD8rjg888UDC28JLJqNa2qkDOkjcfid/0fylcRYRE9agCxhkgK01jqUi5dIUH/Jjrq/bk1joGHaTnKqnz0TvP0/FmH92kjfBE4+IyqLHOTOozq523EpGhBJNFF/bSYM4s/8S02gEcpxUx9JegZJ2l5182Y0xzNC6Hervkk= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1709240209; c=relaxed/simple; bh=vE+YYkkF61yK9+1bVyNQeorGX3spRi4p15P8Mgi7ea4=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=Mi7kkGIetASbcpHQ/+Rm8WLrJF7COMKSqJkN52qivbLPOdRCZVWRlxqGiMFev8FxE88RViu6p2Hdz6LKXUkg6790G2uFAFSRvzmGD3lNGxER1UXr2SDgWyhZ0gNBZhXrxBd/GneQYKi6eE0lvZELiUsWwX6F3g8WVwavo5oWE5o= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1709240205; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=J2F3AM9ahAPb2n17GezPOJzgATrO17jo+GH4TLvZCjE=; b=UuvzFU9AMMwBF1sLl87x2IVX8aQcEwg7yNv8+hvBNFu14WPobYX/KwGhd2v/8q3um7LwSX +1OYLjWKz3K3NV2HJ5x1UbPA67pMS2zgHgY0bE/ihCIfFhBlrHSgRmTqxSRvfcCiitodN3 x3dObuSB3VIaDtVnfJFyw1SKmpqDkTY= Received: from mail-qk1-f198.google.com (mail-qk1-f198.google.com [209.85.222.198]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-425-oULDl9gFOJe-ttONuJoaFA-1; Thu, 29 Feb 2024 15:56:44 -0500 X-MC-Unique: oULDl9gFOJe-ttONuJoaFA-1 Received: by mail-qk1-f198.google.com with SMTP id af79cd13be357-787d1903812so163409385a.2 for ; Thu, 29 Feb 2024 12:56:44 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1709240202; x=1709845002; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=J2F3AM9ahAPb2n17GezPOJzgATrO17jo+GH4TLvZCjE=; b=kNtZDh03UloNkXigokN3QukWWCk/dLL3TvCOA7s9cG7BKrSEAw9AE3bFvbucqkdGLN vRZ2SMvd5O8elA/R85WXWcokcjjqEjjq7MHGKur3DzBabYXcw941qw98K8Cb5Ai5i2NN AS15jicRqTG61daGIBwyIWxTYnqM7PhJrcTpNaxp4gM2J21+jYfhs+Rfi6BRzt0GuGpd 4DRmDrgV4XXIrrqMO1hasveFSAV5uR7jc8gnHEsuW6Ei+iRixSYRCAmeGCYLdIsn17zz CUJMxqfMQwbKDKlBQtEn7nXbRwSM7pINs7SGWORqXGslKLXx0eH5/kUjtbRveuzwsqaA SeHQ== X-Gm-Message-State: AOJu0Yy4wjbXyz1cPMvwU7AL/ANhIHD1/ICWadnA3yWdSdpap3sw1tHJ mUtf5JrMEHWPwAyyTd/0a0QDq2AJEU+xJQ+twgQOO0g4UhEhcB/Eg2WgB2Wu4ETLEfEsSMJXHVr 5f63mLEc/uAmqrQJe/2EDpJkR1RZvIj3tmaNixPKk4dWxFTejj2TSkYTT+myS7uGtqwtZTbe3eB R/JvnDXRd6P7t6JrO0iIqyzWDBBG3vDf1OpW92 X-Received: by 2002:a05:620a:891:b0:787:e31a:3077 with SMTP id b17-20020a05620a089100b00787e31a3077mr3502686qka.77.1709240202328; Thu, 29 Feb 2024 12:56:42 -0800 (PST) X-Google-Smtp-Source: AGHT+IEBqLhnHUhQB+WMDLBpIycDKdUmYxSn1bx1pmGeqPmt+LaNNudPyOM7qZLZWLJFclSHzB2OzQ== X-Received: by 2002:a05:620a:891:b0:787:e31a:3077 with SMTP id b17-20020a05620a089100b00787e31a3077mr3502662qka.77.1709240201830; Thu, 29 Feb 2024 12:56:41 -0800 (PST) Received: from localhost.localdomain (ool-457670bb.dyn.optonline.net. [69.118.112.187]) by smtp.gmail.com with ESMTPSA id t3-20020a05620a004300b00787b7732c0csm1032113qkt.4.2024.02.29.12.56.41 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 29 Feb 2024 12:56:41 -0800 (PST) From: Patrick Palka To: gcc-patches@gcc.gnu.org Cc: jason@redhat.com, nathan@acm.org, Patrick Palka Subject: [PATCH] c++/modules: depending local enums [PR104919, PR106009] Date: Thu, 29 Feb 2024 15:56:39 -0500 Message-ID: <20240229205639.4112668-1-ppalka@redhat.com> X-Mailer: git-send-email 2.44.0.53.g0f9d4d28b7 MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-12.9 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE, 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+incoming=patchwork.ozlabs.org@gcc.gnu.org Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for trunk? -- >8 -- For local enums defined in a non-template function or a function template instantiation it seems we neglect to make the function depend on the enum definition, which ultimately causes streaming to fail due to the enum definition not being streamed before uses of its enumerators are streamed, as far as I can tell. The code responsible for adding such dependencies is gcc/cp/module.cc @@ -8784,17 +8784,6 @@ trees_out::decl_node (tree decl, walk_kind ref) depset *dep = NULL; if (streaming_p ()) dep = dep_hash->find_dependency (decl); ! else if (TREE_CODE (ctx) != FUNCTION_DECL ! || TREE_CODE (decl) == TEMPLATE_DECL ! || (dep_hash->sneakoscope && DECL_IMPLICIT_TYPEDEF_P (decl)) ! || (DECL_LANG_SPECIFIC (decl) ! && DECL_MODULE_IMPORT_P (decl))) ! { ! auto kind = (TREE_CODE (decl) == NAMESPACE_DECL ! && !DECL_NAMESPACE_ALIAS (decl) ! ? depset::EK_NAMESPACE : depset::EK_DECL); ! dep = dep_hash->add_dependency (decl, kind); ! } if (!dep) { and the condition there notably excludes local TYPE_DECLs from a non-template function or a function template instantiation. (For a TYPE_DECL from a function template definition, we'll be dealing with the corresponding TEMPLATE_DECL instead, so we'll add the dependency.) Local classes seem fine as-is but perhaps by accident: with a local class we end up depending on the injected-class-name of the local class since it satisfies the above conditions. A local enum doesn't have such a TYPE_DECL member than we can depend on (its CONST_DECLs are handled earlier as tt_enum_decl tags). This patch attempts to fix this by keeping the 'sneakoscope' flag set while walking the definition of a function, so that we add this needed dependency between a containing function (non-template or specialization) and its local types. Currently it's set only when walking the declaration (presumably to catch local types that escape via a deduced return type), but it seems to make sense to add a dependency regardless of the type escapes. This was nearly enough to make things work, except we now ran into issues with the local TYPE/CONST_DECL copies when streaming the constexpr version of a function body. It occurred to me that we don't need to make copies of local types when copying a constexpr function body; only VAR_DECLs etc need to be copied for sake of recursive constexpr calls. So this patch adjusts copy_fn accordingly. PR c++/104919 PR c++/106009 gcc/cp/ChangeLog: * module.cc (depset::hash::find_dependencies): Keep sneakoscope set when walking the definition. gcc/ChangeLog: * tree-inline.cc (remap_decl): Handle copy_decl returning the original decl. (remap_decls): Handle remap_decl returning the original decl. (copy_fn): Adjust copy_decl callback to skip TYPE_DECL and CONST_DECL. gcc/testsuite/ChangeLog: * g++.dg/modules/tdef-7.h: * g++.dg/modules/tdef-7_b.C: * g++.dg/modules/enum-13_a.C: New test. * g++.dg/modules/enum-13_b.C: New test. --- gcc/cp/module.cc | 2 +- gcc/testsuite/g++.dg/modules/enum-13_a.C | 23 +++++++++++++++++++++++ gcc/testsuite/g++.dg/modules/enum-13_b.C | 8 ++++++++ gcc/testsuite/g++.dg/modules/tdef-7.h | 2 -- gcc/testsuite/g++.dg/modules/tdef-7_b.C | 2 +- gcc/tree-inline.cc | 14 +++++++++++--- 6 files changed, 44 insertions(+), 7 deletions(-) create mode 100644 gcc/testsuite/g++.dg/modules/enum-13_a.C create mode 100644 gcc/testsuite/g++.dg/modules/enum-13_b.C diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc index 66ef0bcaa94..29e57716297 100644 --- a/gcc/cp/module.cc +++ b/gcc/cp/module.cc @@ -13547,9 +13547,9 @@ depset::hash::find_dependencies (module_state *module) /* Turn the Sneakoscope on when depending the decl. */ sneakoscope = true; walker.decl_value (decl, current); - sneakoscope = false; if (current->has_defn ()) walker.write_definition (decl); + sneakoscope = false; } walker.end (); diff --git a/gcc/testsuite/g++.dg/modules/enum-13_a.C b/gcc/testsuite/g++.dg/modules/enum-13_a.C new file mode 100644 index 00000000000..2e570c6c4fb --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/enum-13_a.C @@ -0,0 +1,23 @@ +// PR c++/104919 +// PR c++/106009 +// { dg-additional-options -fmodules-ts } +// { dg-module-cmi Enum13 } + +export module Enum13; + +export +constexpr int f() { + enum E { e = 42 }; + return e; +} + +template +constexpr int ft(T) { + enum blah { e = 43 }; + return e; +} + +export +constexpr int g() { + return ft(0); +} diff --git a/gcc/testsuite/g++.dg/modules/enum-13_b.C b/gcc/testsuite/g++.dg/modules/enum-13_b.C new file mode 100644 index 00000000000..16d4a7c8ac5 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/enum-13_b.C @@ -0,0 +1,8 @@ +// PR c++/104919 +// PR c++/106009 +// { dg-additional-options -fmodules-ts } + +import Enum13; + +static_assert(f() == 42); +static_assert(g() == 43); diff --git a/gcc/testsuite/g++.dg/modules/tdef-7.h b/gcc/testsuite/g++.dg/modules/tdef-7.h index 5bc21e176cb..6125f0460e2 100644 --- a/gcc/testsuite/g++.dg/modules/tdef-7.h +++ b/gcc/testsuite/g++.dg/modules/tdef-7.h @@ -1,7 +1,5 @@ constexpr void duration_cast () { - // the constexpr's body's clone merely duplicates the TYPE_DECL, it - // doesn't create a kosher typedef typedef int __to_rep; } diff --git a/gcc/testsuite/g++.dg/modules/tdef-7_b.C b/gcc/testsuite/g++.dg/modules/tdef-7_b.C index c526ca8dd4f..ea76458715b 100644 --- a/gcc/testsuite/g++.dg/modules/tdef-7_b.C +++ b/gcc/testsuite/g++.dg/modules/tdef-7_b.C @@ -5,5 +5,5 @@ import "tdef-7_a.H"; // { dg-final { scan-lang-dump-times {merge key \(matched\) function_decl:'::duration_cast} 1 module } } // { dg-final { scan-lang-dump-not {merge key \(new\)} module } } -// { dg-final { scan-lang-dump-times {merge key \(unique\) type_decl:'#null#'} 2 module } } +// { dg-final { scan-lang-dump-times {merge key \(unique\) type_decl:'#null#'} 1 module } } // { dg-final { scan-lang-dump-times {Cloned:-[0-9]* typedef integer_type:'::duration_cast::__to_rep'} 1 module } } diff --git a/gcc/tree-inline.cc b/gcc/tree-inline.cc index 75c10eb7dfc..b35760ae9f0 100644 --- a/gcc/tree-inline.cc +++ b/gcc/tree-inline.cc @@ -370,7 +370,7 @@ remap_decl (tree decl, copy_body_data *id) need this decl for TYPE_STUB_DECL. */ insert_decl_map (id, decl, t); - if (!DECL_P (t)) + if (!DECL_P (t) || t == decl) return t; /* Remap types, if necessary. */ @@ -765,7 +765,7 @@ remap_decls (tree decls, vec **nonlocalized_list, TREE_CHAIN. If we remapped this variable to the return slot, it's already declared somewhere else, so don't declare it here. */ - if (new_var == id->retvar) + if (new_var == old_var || new_var == id->retvar) ; else if (!new_var) { @@ -6610,7 +6610,15 @@ copy_fn (tree fn, tree& parms, tree& result) id.src_cfun = DECL_STRUCT_FUNCTION (fn); id.decl_map = &decl_map; - id.copy_decl = copy_decl_no_change; + id.copy_decl = [] (tree decl, copy_body_data *id) + { + if (TREE_CODE (decl) == TYPE_DECL || TREE_CODE (decl) == CONST_DECL) + /* Don't make copies of local types or enumerators, the C++ + constexpr evaluator doesn't need them and they cause problems + with modules. */ + return decl; + return copy_decl_no_change (decl, id); + }; id.transform_call_graph_edges = CB_CGE_DUPLICATE; id.transform_new_cfg = false; id.transform_return_to_modify = false;