From patchwork Fri Sep 27 05:59:41 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nathaniel Shead X-Patchwork-Id: 1990104 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=G+Y+q+Cj; 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 4XFKZv1FQbz1xt8 for ; Fri, 27 Sep 2024 16:00:55 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 0423A385DDE1 for ; Fri, 27 Sep 2024 06:00:53 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mail-pj1-x1031.google.com (mail-pj1-x1031.google.com [IPv6:2607:f8b0:4864:20::1031]) by sourceware.org (Postfix) with ESMTPS id A68B5385840F for ; Fri, 27 Sep 2024 05:59:47 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org A68B5385840F 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 A68B5385840F Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::1031 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1727416790; cv=none; b=sRY1/6Hr7wLC1SLRByfJFo71y6QSLYKIHCvxln9XU1T7u3QPIF2YMDDW1EL6i9PUHLY9vJppWfEgV8Rc93M4hu4pr8cE9ykqT4s0aIniKESb65MO/aX4ts14nt/7AFad7EePvmvwMz2rXwvD5hCrSwG+Nm7WNwJxGLExBpliAzo= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1727416790; c=relaxed/simple; bh=IJkuhhANKgM4B9N0BImIse7LxhrOPOILNa4k12xMaMc=; h=DKIM-Signature:Message-ID:Date:From:To:Subject:MIME-Version; b=bn8ufj50akpJTfBf88NuxqczDGZX6ZuB6ev/GXNqK2xFLBsAATrkWoMVl8VB1W8Dcr8P0usGEb3x/c7D51n9xGWvSlLx6an1ul+4sIlc4dYbdjINYvUW6Ur2dgdmd3YtESDz+WgixyF/xBJ4blNMVPlIRcWZneWMvMpPza8n88w= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-pj1-x1031.google.com with SMTP id 98e67ed59e1d1-2e0b02ed295so186536a91.2 for ; Thu, 26 Sep 2024 22:59:47 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1727416786; x=1728021586; 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=aambrW3gmnmWZIjUF/wzTN3+gy/dfiO5jGetCRvgwzI=; b=G+Y+q+Cjyu5K33acBdYC78Dx0fw78qtQqTzV23kvyQpYfjK+UzAIT2ERfD3GEmaImJ AuJN4PwFYKj0cCkBSumkMhYKh7G3LZVeIxgMF0UQrz60D9/BrsXsDn+TlG/xoLrQ3j6u 4w1F6vQ9s+Em8yoj98xkTs6b9quh4jdrBaxwnQ5JTeTdKshCIt5msizyTbGqw6gaoT22 4u1lYOtv8yxu4hS+QZmwScBP91ivKt1Jf73kQ4DmLuXZ1C0kAgjLOCw8N40pLy0Zl46y 6fZ+U8Ky8BRN1nQngeij6pH9QOLT7vJlxKD9lB5eA0fLvujyy0jc0D1gHWde8sruNH7K EEjA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1727416786; x=1728021586; 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=aambrW3gmnmWZIjUF/wzTN3+gy/dfiO5jGetCRvgwzI=; b=AIvs1Tbnz5jw7y4eT+IzKs7Y+Jmd/tIsOsv9PvZPuvmg1YvvPm99pl695bd4gd5FAI 2eflgjt07inYWcVKKZlgFeDHY22BiZsoBZt+i+nZXXn0kHsXtS9IVlnbLYI4oUoNP5kR CMtF2WvirqNtKBGGV+XTYVgjfhXUQY6hAyri4FHiU/NjF5Zhob3rPa1xCI09oB9uxxqS xl1LgU/Dl4UllVPmlocyEm6RfTSuJlJHSDtrjpULZQQfZI0YW/BWoazoPlaqGKgd2gJB e6U3L7IBgveunORDNUNcPiTJpdl66w1WRDfqTYhkLK2qFPWNi8ZtHQ/vDJTjroIWUAxt 2WWQ== X-Gm-Message-State: AOJu0YxzmHc9lQQaYedcxX5PGbUbZho/0JUt/rUe/P1cnQon2+9YuG3P XEPkQmicbowvWe3mmlOoX5jhiBAQWK5Y5cwTE3mb9gO23a/k1zqm0CcRaA== X-Google-Smtp-Source: AGHT+IHaRKcBa9AjjwHkudvc5xOwtm82dSvWN9XBar2CNGfhDPAnw3O6iJ8Esk4hrb7R+snA7PHhjQ== X-Received: by 2002:a05:6a00:39a8:b0:710:5243:4161 with SMTP id d2e1a72fcca58-71b26079b75mr1442628b3a.5.1727416786427; Thu, 26 Sep 2024 22:59:46 -0700 (PDT) Received: from Thaum. ([163.47.68.2]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-71b264b63d9sm834110b3a.52.2024.09.26.22.59.44 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 26 Sep 2024 22:59:45 -0700 (PDT) Message-ID: <66f649d1.050a0220.ea27b.491a@mx.google.com> X-Google-Original-Message-ID: Date: Fri, 27 Sep 2024 15:59:41 +1000 From: Nathaniel Shead To: gcc-patches@gcc.gnu.org Cc: Jason Merrill , Nathan Sidwell Subject: [PATCH v2 5/6] c++/modules: Validate external linkage definitions in header units [PR116401] References: <66f64939.050a0220.261230.4b66@mx.google.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <66f64939.050a0220.261230.4b66@mx.google.com> X-Spam-Status: No, score=-12.0 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, 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 Bootstrapped and regtested on x86_64-pc-linux-gnu and aarch64-unknown-linux-gnu, OK for trunk? -- >8 -- [module.import] p6 says "A header unit shall not contain a definition of a non-inline function or variable whose name has external linkage." This patch implements this requirement, and cleans up some issues in the testsuite where this was already violated. To handle deduction guides we mark them as inline, since although we give them a definition for implementation reasons, by the standard they have no definition, and so we should not error in this case. PR c++/116401 gcc/cp/ChangeLog: * decl.cc (grokfndecl): Mark deduction guides as 'inline'. * module.cc (check_module_decl_linkage): Implement checks for non-inline external linkage definitions in headers. gcc/testsuite/ChangeLog: * g++.dg/modules/macro-4_c.H: Add missing 'inline'. * g++.dg/modules/pr106761.h: Likewise. * g++.dg/modules/pr98843_b.H: Likewise. * g++.dg/modules/pr99468.H: Likewise. * g++.dg/modules/pragma-1_a.H: Likewise. * g++.dg/modules/tpl-ary-1.h: Likewise. * g++.dg/modules/hdr-2.H: New test. Signed-off-by: Nathaniel Shead --- gcc/cp/decl.cc | 1 + gcc/cp/module.cc | 18 +++ gcc/testsuite/g++.dg/modules/hdr-2.H | 172 ++++++++++++++++++++++ gcc/testsuite/g++.dg/modules/macro-4_c.H | 2 +- gcc/testsuite/g++.dg/modules/pr106761.h | 2 +- gcc/testsuite/g++.dg/modules/pr98843_b.H | 2 +- gcc/testsuite/g++.dg/modules/pr99468.H | 2 +- gcc/testsuite/g++.dg/modules/pragma-1_a.H | 2 +- gcc/testsuite/g++.dg/modules/tpl-ary-1.h | 2 +- 9 files changed, 197 insertions(+), 6 deletions(-) create mode 100644 gcc/testsuite/g++.dg/modules/hdr-2.H diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc index 6a7ba416cf8..5ddb7eafa50 100644 --- a/gcc/cp/decl.cc +++ b/gcc/cp/decl.cc @@ -10823,6 +10823,7 @@ grokfndecl (tree ctype, have one: the restriction that you can't repeat a deduction guide makes them more like a definition anyway. */ DECL_INITIAL (decl) = void_node; + DECL_DECLARED_INLINE_P (decl) = true; break; default: break; diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc index a4343044d1a..b4030c62eec 100644 --- a/gcc/cp/module.cc +++ b/gcc/cp/module.cc @@ -19943,6 +19943,24 @@ check_module_decl_linkage (tree decl) if (!module_has_cmi_p ()) return; + /* A header unit shall not contain a definition of a non-inline function + or variable (not template) whose name has external linkage. */ + if (header_module_p () + && !processing_template_decl + && ((TREE_CODE (decl) == FUNCTION_DECL + && !DECL_DECLARED_INLINE_P (decl) + && DECL_INITIAL (decl)) + || (TREE_CODE (decl) == VAR_DECL + && !DECL_INLINE_VAR_P (decl) + && DECL_INITIALIZED_P (decl) + && !DECL_IN_AGGR_P (decl))) + && !(DECL_LANG_SPECIFIC (decl) + && DECL_TEMPLATE_INSTANTIATION (decl)) + && decl_linkage (decl) == lk_external) + error_at (DECL_SOURCE_LOCATION (decl), + "external linkage definition of %qD in header module must " + "be declared %", decl); + /* An internal-linkage declaration cannot be generally be exported. But it's OK to export any declaration from a header unit, including internal linkage declarations. */ diff --git a/gcc/testsuite/g++.dg/modules/hdr-2.H b/gcc/testsuite/g++.dg/modules/hdr-2.H new file mode 100644 index 00000000000..097546d5667 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/hdr-2.H @@ -0,0 +1,172 @@ +// { dg-additional-options "-fmodule-header" } +// { dg-module-cmi !{} } +// external linkage variables or functions in header units must +// not have non-inline definitions + +int x_err; // { dg-error "external linkage definition" } +int y_err = 123; // { dg-error "external linkage definition" } +void f_err() {} // { dg-error "external linkage definition" } + +struct Err { + Err(); + void m(); + static void s(); + static int x; + static int y; +}; +Err::Err() = default; // { dg-error "external linkage definition" } +void Err::m() {} // { dg-error "external linkage definition" } +void Err::s() {} // { dg-error "external linkage definition" } +int Err::x; // { dg-error "external linkage definition" } +int Err::y = 123; // { dg-error "external linkage definition" } + +// No definition, OK +extern int y_decl; +void f_decl(); + +template struct DeductionGuide {}; +DeductionGuide() -> DeductionGuide; + +struct NoDefStatics { + enum E { V }; + static const int x = 123; + static const E e = V; +}; + +// But these have definitions again (though the error locations aren't great) +struct YesDefStatics { + enum E { V }; + static const int x = 123; // { dg-error "external linkage definition" } + static const E e = V; // { dg-error "external linkage definition" } +}; +const int YesDefStatics::x; +const YesDefStatics::E YesDefStatics::e; + +// Inline decls are OK +inline int x_inl; +inline int y_inl = 123; +inline void f_inl() {} +constexpr void g_inl() {} +void h_inl() = delete; + +struct Inl { + void m() {} + static void s() {} + static inline int x; + static inline int y = 123; +}; + +// Internal linkage decls are OK +static int x_internal; +static int y_internal = 123; +static void f_internal() {} + +namespace { + struct Internal { + void m(); + static void s(); + static int x; + static int y; + }; + void Internal::m() {} + void Internal::s() {} + int Internal::x; + int Internal::y = 123; +} + +// Function-scope entities are OK +inline void f_static() { + static int x_static; + static int y_static = 123; + thread_local int x_thread_local; + thread_local int y_thread_local = 123; + + x_static = y_static; + x_thread_local = y_thread_local; +} + +// Templates (not functions or variables) are OK +template int x_tpl; +template int y_tpl = 123; +template void f_tpl() {} + +struct Template_Body { + template void m(); + template static void s(); + template static int x; + template static int y; +}; +template void Template_Body::m() {} +template void Template_Body::s() {} +template int Template_Body::x; +template int Template_Body::y = 123; + +template struct Template_Type { + void m(); + static void s(); + static int x; + static int y; +}; +template void Template_Type::m() {} +template void Template_Type::s() {} +template int Template_Type::x; +template int Template_Type::y = 123; + +// Implicit instantiations are OK +inline void instantiate_tmpls() { + x_tpl = y_tpl; + f_tpl(); + + Template_Body{}.m(); + Template_Body::s(); + Template_Body::x = Template_Body::y; + + using TT = Template_Type; + TT{}.m(); + TT::s(); + TT::x = TT::y; +} + +// Explicit instantiations are also OK (extern or otherwise) +template int x_tpl; +template int y_tpl; +template void f_tpl(); + +template void Template_Body::m(); +template void Template_Body::s(); +template int Template_Body::x; +template int Template_Body::y; + +template void Template_Type::m(); +template void Template_Type::s(); +template int Template_Type::x; +template int Template_Type::y; + +extern template int x_tpl; +extern template int y_tpl; +extern template void f_tpl(); + +extern template void Template_Body::m(); +extern template void Template_Body::s(); +extern template int Template_Body::x; +extern template int Template_Body::y; + +extern template void Template_Type::m(); +extern template void Template_Type::s(); +extern template int Template_Type::x; +extern template int Template_Type::y; + +// But explicit specialisations are not (though note [temp.expl.spec] p13) +template <> int x_tpl; // { dg-error "inline" } +template <> int y_tpl = 123; // { dg-error "inline" } +template <> void f_tpl() {} // { dg-error "inline" } + +template <> void Template_Body::m() {} // { dg-error "inline" } +template <> void Template_Body::s() {} // { dg-error "inline" } +template <> int Template_Body::x; // { dg-bogus "inline" "not a definition" } +template <> int Template_Body::y = 123; // { dg-error "inline" } + +template <> void Template_Type::m() {} // { dg-error "inline" } +template <> void Template_Type::s() {} // { dg-error "inline" } +template <> int Template_Type::x; // { dg-bogus "inline" "not a definition" } +template <> int Template_Type::y = 123; // { dg-error "inline" } diff --git a/gcc/testsuite/g++.dg/modules/macro-4_c.H b/gcc/testsuite/g++.dg/modules/macro-4_c.H index ec2bed91ccd..5692e8e41d2 100644 --- a/gcc/testsuite/g++.dg/modules/macro-4_c.H +++ b/gcc/testsuite/g++.dg/modules/macro-4_c.H @@ -6,7 +6,7 @@ #undef FIVE // no effect import "macro-4_a.H"; -int a; +inline int a; #undef THREE #undef FOUR #define FOUR 4c diff --git a/gcc/testsuite/g++.dg/modules/pr106761.h b/gcc/testsuite/g++.dg/modules/pr106761.h index 9f22a22a45d..5c13fc0f118 100644 --- a/gcc/testsuite/g++.dg/modules/pr106761.h +++ b/gcc/testsuite/g++.dg/modules/pr106761.h @@ -19,4 +19,4 @@ struct tuple { = typename _TupleConstraints::template __constructible; }; -tuple t; +inline tuple t; diff --git a/gcc/testsuite/g++.dg/modules/pr98843_b.H b/gcc/testsuite/g++.dg/modules/pr98843_b.H index d6734bd382d..af504a840c7 100644 --- a/gcc/testsuite/g++.dg/modules/pr98843_b.H +++ b/gcc/testsuite/g++.dg/modules/pr98843_b.H @@ -6,7 +6,7 @@ template int Fn () return I; } -template<> int Fn<1> () +template<> inline int Fn<1> () { return 0; } diff --git a/gcc/testsuite/g++.dg/modules/pr99468.H b/gcc/testsuite/g++.dg/modules/pr99468.H index d7da3a83e1c..df63c613b2c 100644 --- a/gcc/testsuite/g++.dg/modules/pr99468.H +++ b/gcc/testsuite/g++.dg/modules/pr99468.H @@ -3,4 +3,4 @@ module M; // { dg-error "module-declaration not permitted" } -int i; +inline int i; diff --git a/gcc/testsuite/g++.dg/modules/pragma-1_a.H b/gcc/testsuite/g++.dg/modules/pragma-1_a.H index 6eb0a5900a7..a69be2fd05e 100644 --- a/gcc/testsuite/g++.dg/modules/pragma-1_a.H +++ b/gcc/testsuite/g++.dg/modules/pragma-1_a.H @@ -1,4 +1,4 @@ // { dg-additional-options -fmodule-header } // { dg-module-cmi {} } -int i; +inline int i; diff --git a/gcc/testsuite/g++.dg/modules/tpl-ary-1.h b/gcc/testsuite/g++.dg/modules/tpl-ary-1.h index 2f745afba36..2f9a8106412 100644 --- a/gcc/testsuite/g++.dg/modules/tpl-ary-1.h +++ b/gcc/testsuite/g++.dg/modules/tpl-ary-1.h @@ -1,5 +1,5 @@ -int ary[4]; +inline int ary[4]; extern int unb[]; typedef int z[0];