From patchwork Fri May 17 06:14:31 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nathaniel Shead X-Patchwork-Id: 1936283 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=JblKsSSf; 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 4VgcC01bMBz20KF for ; Fri, 17 May 2024 16:15:24 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id CB7CF3849AD6 for ; Fri, 17 May 2024 06:15:21 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mail-pl1-x633.google.com (mail-pl1-x633.google.com [IPv6:2607:f8b0:4864:20::633]) by sourceware.org (Postfix) with ESMTPS id 498BE385F032 for ; Fri, 17 May 2024 06:14:37 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 498BE385F032 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 498BE385F032 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::633 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1715926480; cv=none; b=hrMLp6tDt6cJAn4iq7ZH4QBnons/ga1NogARb40e6ZmlnpkIIBX358bPIsWdaeKK7XvNPRGvZ2eGP5oHdCWVCM5sCDGr/wpnpRk3IsCD/2WsjX/b4Df2xv1uNjPd7OsaQGAZTAyEKRB826F6HRNjcdspBVnAZNYql490PJ1Tn24= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1715926480; c=relaxed/simple; bh=bIcKaCtT0lQwc+X/hT7aW+vwXdcZdmPjwlBhjQH3Sz8=; h=DKIM-Signature:Message-ID:Date:From:To:Subject:MIME-Version; b=gUQF5b0VV55clqOgWvKZ59E5hN4m3AsmFvDHIwFpuygPk9nGpzw+3rbGBGBHhb1lci4bJJkpbmZHsB+rP9quUxGwakLL31rv4ma19oLSrs9jcj6nUCy5BUi6HHvfIpriKLPRi8bS75q6fcScAzB0DjsZLH9NZNke0Bkzd9cyLHk= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-pl1-x633.google.com with SMTP id d9443c01a7336-1ee0132a6f3so2564845ad.0 for ; Thu, 16 May 2024 23:14:37 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1715926476; x=1716531276; 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=LiAHfFGMVa19CiLap4S4AhTDRW0iCfYgwEUSq9DTK28=; b=JblKsSSfcftj7tEsVKXg/GGbDpNcr7KLDHegwbSjLDWT/mA57jBRQsUzogWdGPolnB D1ZibGch8JEooxNSshq8zMc8eX+gToFsUCeuZTXzs/ilx0l6qUFUhRFPkxdUEKIRhiFW Tnavzt8P5hi5FjxlWx0rTA9ROXRKb9cjPzUHAJSev1TreHvBlSC/yInY9x7sMpNXJNH/ y2sTXw7RtYoHPLBL8DvkoxaeRgqDTWJvVQgUXYYcxuNtZ0dosGFcokn2Dk6tAHH1J2On XnNucOn29lRLRfhG5GszeCLHebTORavIwp4D+DTJcXY9TyamXJ2Ej1/liyAOUI/PEqUI zmAA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1715926476; x=1716531276; 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=LiAHfFGMVa19CiLap4S4AhTDRW0iCfYgwEUSq9DTK28=; b=L2fNbtsFnFSi1+WDBA1JqUXIe0QPnVdr3VPFK0vIl349CTHWrTbyX5OAzHl2Sy9UPF Tol7KoseNUoG1FizSLNRwdkdVweboKt6JKdqLmlv4RUVFz5vdddW8cK/CpS2P+Qkg6ew 8GJMCL8SfR+b2dGk8voxl8W9/+BRGqH8eHgeTbBn+OMDoXJR1pqQBex9TIPp6aGW5oap 9XvQAop+fjUL7bd+5n5LBDHIYLMQUSHYwRN7S62UoAS6+2UtgmiBYU4CabBLuhxy5d70 EsRpM06Lr+3X6OCuvQFhxib89ckfWh0RcUhhq9EVt4GzxagSO55jAc3kGr2UfDrpEm5R cxBg== X-Gm-Message-State: AOJu0YyG+vjj2eyiGV5GoSYuyGrd468LBlqMkftg43Xz7JzCxCh6lzVh 4US5wC9sTyyk+n/uFpLN5A6QKt0ivUtW6qng1P31NNajp2DnoEde X-Google-Smtp-Source: AGHT+IEvWW3UeIETLUCYanRN8EuOpwaNDfvXHnfqfKO5WJEqYidhtTbIb43OiL+Q/c0WfQDmrdDTvw== X-Received: by 2002:a17:903:1c1:b0:1e5:3554:d9db with SMTP id d9443c01a7336-1ef4404b961mr248501985ad.64.1715926476050; Thu, 16 May 2024 23:14:36 -0700 (PDT) Received: from Thaum. (14-200-72-253.tpgi.com.au. [14.200.72.253]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-1ef0bad86ffsm149281095ad.101.2024.05.16.23.14.33 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 16 May 2024 23:14:35 -0700 (PDT) Message-ID: <6646f5cb.170a0220.fb17d.75a5@mx.google.com> X-Google-Original-Message-ID: Date: Fri, 17 May 2024 16:14:31 +1000 From: Nathaniel Shead To: Jason Merrill Cc: gcc-patches@gcc.gnu.org, Nathan Sidwell Subject: [PATCH v2] c++/modules: Remember that header units have CMIs References: <664181e2.650a0220.dbfd8.e3d5@mx.google.com> <10d5ff7c-3d30-4317-97d9-91ea2370bbfa@redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <10d5ff7c-3d30-4317-97d9-91ea2370bbfa@redhat.com> X-Spam-Status: No, score=-11.8 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 Tue, May 14, 2024 at 06:21:48PM -0400, Jason Merrill wrote: > On 5/12/24 22:58, Nathaniel Shead wrote: > > Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk? > > OK. > I realised as I was looking over this again that I might have spoken too soon with the header unit example being supported. Doing the following: // a.H struct { int y; } s; decltype(s) f(decltype(s)); // { dg-error "used but never defined" } inline auto x = f({ 123 }); // b.C struct {} unrelated; import "a.H"; decltype(s) f(decltype(s) x) { return { 456 + x.y }; } // c.C import "linkage-3_a.H"; int main() { auto a = x.y; } Actually does fail to link, because in 'c.C' we call 'f(.anon_0)' but the definition 'b.C' is f(.anon_1). I don't think this is fixable, so I don't think this direction is workable. That said, I think that it might still be worth making header modules satisfy 'module_has_cmi_p', since that is true to the name, and will be useful in other places we currently use 'module_p ()': in which case we could instead make all the callers in 'no_linkage_check' do 'module_maybe_has_cmi_p () && !header_module_p ()'; something like the following, perhaps? But I'm not too fussed about this overall if you think this will just make things more complicated. Otherwise bootstrapped and regtested (so far just modules.exp) on x86_64-pc-linux-gnu, OK for trunk if full regtest passes? -- >8 -- This appears to be an oversight in the definition of module_has_cmi_p. This change will allow us to use the function directly in more places that need to additional work only if generating a module CMI in the future. However, we do need to change callers of 'module_maybe_has_cmi_p'; in particular header units, though having a CMI, do not provide a TU to emit names into, and thus each importer will emit their own definitions which may not match for no-linkage types. gcc/cp/ChangeLog: * cp-tree.h (module_has_cmi_p): Also true for header units. * decl.cc (grokfndecl): Disallow no-linkage names in header units. * tree.cc (no_linkage_check): Likewise. gcc/testsuite/ChangeLog: * g++.dg/modules/linkage-3.H: New test. Signed-off-by: Nathaniel Shead --- gcc/cp/cp-tree.h | 2 +- gcc/cp/decl.cc | 2 +- gcc/cp/tree.cc | 13 +++++++----- gcc/testsuite/g++.dg/modules/linkage-3.H | 25 ++++++++++++++++++++++++ 4 files changed, 35 insertions(+), 7 deletions(-) create mode 100644 gcc/testsuite/g++.dg/modules/linkage-3.H diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index ba9e848c177..ac55b5579a1 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -7381,7 +7381,7 @@ inline bool module_interface_p () inline bool module_partition_p () { return module_kind & MK_PARTITION; } inline bool module_has_cmi_p () -{ return module_kind & (MK_INTERFACE | MK_PARTITION); } +{ return module_kind & (MK_INTERFACE | MK_PARTITION | MK_HEADER); } inline bool module_purview_p () { return module_kind & MK_PURVIEW; } diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc index 6fcab615d55..f89a7df30b7 100644 --- a/gcc/cp/decl.cc +++ b/gcc/cp/decl.cc @@ -10802,7 +10802,7 @@ grokfndecl (tree ctype, used by an importer. We don't just use module_has_cmi_p here because for entities in the GMF we don't yet know whether this module will have a CMI, so we'll conservatively assume it might. */ - publicp = module_maybe_has_cmi_p (); + publicp = module_maybe_has_cmi_p () && !header_module_p (); if (publicp && cxx_dialect == cxx98) { diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc index 9d37d255d8d..00c50e3130d 100644 --- a/gcc/cp/tree.cc +++ b/gcc/cp/tree.cc @@ -2974,9 +2974,9 @@ verify_stmt_tree (tree t) /* Check if the type T depends on a type with no linkage and if so, return it. If RELAXED_P then do not consider a class type declared - within a vague-linkage function or in a module CMI to have no linkage, - since it can still be accessed within a different TU. Remember: - no-linkage is not the same as internal-linkage. */ + within a vague-linkage function or in a non-header module CMI to + have no linkage, since it can still be accessed within a different TU. + Remember: no-linkage is not the same as internal-linkage. */ tree no_linkage_check (tree t, bool relaxed_p) @@ -3019,7 +3019,8 @@ no_linkage_check (tree t, bool relaxed_p) { if (relaxed_p && TREE_PUBLIC (CP_TYPE_CONTEXT (t)) - && module_maybe_has_cmi_p ()) + && module_maybe_has_cmi_p () + && !header_module_p ()) /* This type could possibly be accessed outside this TU. */ return NULL_TREE; else @@ -3037,7 +3038,9 @@ no_linkage_check (tree t, bool relaxed_p) { if (relaxed_p && (vague_linkage_p (r) - || (TREE_PUBLIC (r) && module_maybe_has_cmi_p ()))) + || (TREE_PUBLIC (r) + && module_maybe_has_cmi_p () + && !header_module_p ()))) r = CP_DECL_CONTEXT (r); else return t; diff --git a/gcc/testsuite/g++.dg/modules/linkage-3.H b/gcc/testsuite/g++.dg/modules/linkage-3.H new file mode 100644 index 00000000000..a34ff084eaf --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/linkage-3.H @@ -0,0 +1,25 @@ +// { dg-additional-options "-fmodule-header" } +// { dg-module-cmi !{} } + +// Like linkage-1, but for header units. + +// External linkage definitions must be declared as 'inline' to satisfy +// [module.import] p6, so we don't need to care about voldemort types in +// function definitions. However, we still care about anonymous types like +// this: because a header unit is not a TU, it's up to each importer to export +// the name declared here, and depending on what other anonymous types they +// declare they could give each declaration different mangled names. +// So we should still complain about this because in general it's not safe +// to assume that the declaration can be provided in another TU; this is OK +// to do by [module.import] p5. + +inline auto f() { + struct A {}; + return A{}; +} +decltype(f()) g(); // OK, vague linkage function +auto x = g(); + +struct { int y; } s; +decltype(s) h(); // { dg-error "used but never defined" } +inline auto y = h();