On Sun, Feb 09, 2025 at 01:16:00AM +1100, Nathaniel Shead wrote:
> Tested on x86_64-pc-linux-gnu, OK for trunk if full bootstrap + regtest
> passes?
>
> -- >8 --
>
> There are two issues with no-linkage decls (e.g. explicit type aliases)
> in unnamed namespaces that this patch fixes.
>
> Firstly, we don't currently handle exporting no-linkage decls in unnamed
> namespaces. This should be ill-formed in [module.export], since having
> an exported declaration within a namespace-definition makes the
> namespace definition exported (p2), but an unnamed namespace has
> internal linkage thus violating p3.
>
> Secondly, by the standard it appears to be possible to emit unnamed
> namespaces from named modules in certain scenarios. This patch makes
> the adjustments needed to ensure we don't error in this case.
>
One thing to note with this is that it means that the following sample:
export module M;
namespace {
struct Internal {};
using Alias = Internal;
}
will still error:
test.cpp:4:9: error: ‘using {anonymous}::Alias = struct {anonymous}::Internal’
exposes TU-local entity ‘struct {anonymous}::Internal’
4 | using Alias = Internal;
| ^~~~~
test.cpp:3:10: note: ‘struct {anonymous}::Internal’ declared with internal
linkage
3 | struct Internal {};
| ^~~~~~~~
https://eel.is/c++draft/basic.link#17 is the relevant paragraph here,
but I'm not 100% sure what it says about this example; I suppose that
given Alias isn't really an entity maybe this should be OK? But either
way we definitely don't want to emit this alias.
Maybe the correct approach here is to mark an explicit type alias
TU-local iff the type it refers to is itself TU-local? So something
like this on top of this patch:
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index 21251f98a35..7aa7f93df57 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -13331,6 +13331,8 @@ bool
depset::hash::is_tu_local_entity (tree decl, bool explain/*=false*/)
{
gcc_checking_assert (DECL_P (decl));
+ location_t loc = DECL_SOURCE_LOCATION (decl);
+ tree type = TREE_TYPE (decl);
/* Only types, functions, variables, and template (specialisations)
can be TU-local. */
@@ -13340,15 +13342,22 @@ depset::hash::is_tu_local_entity (tree decl, bool
explain/*=false*/)
&& TREE_CODE (decl) != TEMPLATE_DECL)
return false;
- /* An explicit type alias is not an entity, and so is never TU-local.
- Neither are the built-in declarations of 'int' and such. */
+ /* An explicit type alias is not an entity, but rather inherits
+ whether it's TU-local from the type that it aliases.
+ The built-in declarations of 'int' and such are never TU-local. */
if (TREE_CODE (decl) == TYPE_DECL
&& !DECL_SELF_REFERENCE_P (decl)
&& !DECL_IMPLICIT_TYPEDEF_P (decl))
- return false;
-
- location_t loc = DECL_SOURCE_LOCATION (decl);
- tree type = TREE_TYPE (decl);
+ {
+ if (tree orig = DECL_ORIGINAL_TYPE (decl))
+ {
+ if (explain)
+ inform (loc, "%qD is an alias of TU-local type %qT", decl, orig);
+ return is_tu_local_entity (TYPE_NAME (orig), explain);
+ }
+ else
+ return false;
+ }
/* Check specializations first for slightly better explanations. */
int use_tpl = -1;
Thoughts?
> PR c++/118799
>
> gcc/cp/ChangeLog:
>
> * module.cc (depset::hash::is_tu_local_entity): Only types,
> functions, variables, and template (specialisations) can be
> TU-local.
> (module_state::write_namespaces): Allow unnamed namespaces in
> named modules.
> (check_module_decl_linkage): Error for all exported declarations
> in an unnamed namespace.
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/modules/export-6.C: Adjust error message, add check for
> no-linkage decls in namespace.
> * g++.dg/modules/internal-4_b.C: Allow exposing a namespace with
> internal linkage.
> * g++.dg/modules/using-30_a.C: New test.
> * g++.dg/modules/using-30_b.C: New test.
> * g++.dg/modules/using-30_c.C: New test.
>
> Signed-off-by: Nathaniel Shead <nathanielosh...@gmail.com>
> ---
> gcc/cp/module.cc | 35 ++++++++++++++++-----
> gcc/testsuite/g++.dg/modules/export-6.C | 33 ++++++++++---------
> gcc/testsuite/g++.dg/modules/internal-4_b.C | 4 +--
> gcc/testsuite/g++.dg/modules/using-30_a.C | 9 ++++++
> gcc/testsuite/g++.dg/modules/using-30_b.C | 6 ++++
> gcc/testsuite/g++.dg/modules/using-30_c.C | 13 ++++++++
> 6 files changed, 77 insertions(+), 23 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/modules/using-30_a.C
> create mode 100644 gcc/testsuite/g++.dg/modules/using-30_b.C
> create mode 100644 gcc/testsuite/g++.dg/modules/using-30_c.C
>
> diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> index 59716e1873e..21251f98a35 100644
> --- a/gcc/cp/module.cc
> +++ b/gcc/cp/module.cc
> @@ -13332,6 +13332,14 @@ depset::hash::is_tu_local_entity (tree decl, bool
> explain/*=false*/)
> {
> gcc_checking_assert (DECL_P (decl));
>
> + /* Only types, functions, variables, and template (specialisations)
> + can be TU-local. */
> + if (TREE_CODE (decl) != TYPE_DECL
> + && TREE_CODE (decl) != FUNCTION_DECL
> + && TREE_CODE (decl) != VAR_DECL
> + && TREE_CODE (decl) != TEMPLATE_DECL)
> + return false;
> +
> /* An explicit type alias is not an entity, and so is never TU-local.
> Neither are the built-in declarations of 'int' and such. */
> if (TREE_CODE (decl) == TYPE_DECL
> @@ -16433,8 +16441,9 @@ module_state::write_namespaces (elf_out *to,
> vec<depset *> spaces,
> depset *b = spaces[ix];
> tree ns = b->get_entity ();
>
> + /* This could be an anonymous namespace even for a named module,
> + since we can still emit no-linkage decls. */
> gcc_checking_assert (TREE_CODE (ns) == NAMESPACE_DECL);
> - gcc_checking_assert (TREE_PUBLIC (ns) || header_module_p ());
>
> unsigned flags = 0;
> if (TREE_PUBLIC (ns))
> @@ -20394,13 +20403,25 @@ check_module_decl_linkage (tree 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. */
> - if (!header_module_p ()
> - && DECL_MODULE_EXPORT_P (decl)
> - && decl_linkage (decl) == lk_internal)
> + if (!header_module_p () && DECL_MODULE_EXPORT_P (decl))
> {
> - error_at (DECL_SOURCE_LOCATION (decl),
> - "exporting declaration %qD with internal linkage", decl);
> - DECL_MODULE_EXPORT_P (decl) = false;
> + /* Let's additionally treat any exported declaration within an
> + internal namespace as exporting a declaration with internal
> + linkage, as this would implicitly also export the internal
> + linkage namespace. */
> + if (decl_internal_context_p (decl))
> + {
> + error_at (DECL_SOURCE_LOCATION (decl),
> + "exporting declaration %qD declared in unnamed namespace",
> + decl);
> + DECL_MODULE_EXPORT_P (decl) = false;
> + }
> + else if (decl_linkage (decl) == lk_internal)
> + {
> + error_at (DECL_SOURCE_LOCATION (decl),
> + "exporting declaration %qD with internal linkage", decl);
> + DECL_MODULE_EXPORT_P (decl) = false;
> + }
> }
> }
>
> diff --git a/gcc/testsuite/g++.dg/modules/export-6.C
> b/gcc/testsuite/g++.dg/modules/export-6.C
> index 460cdf08ea9..af54e5fbe87 100644
> --- a/gcc/testsuite/g++.dg/modules/export-6.C
> +++ b/gcc/testsuite/g++.dg/modules/export-6.C
> @@ -16,27 +16,32 @@ export static auto [d] = S{}; // { dg-error "internal
> linkage" "" { target c++2
> #endif
>
> namespace {
> - export int y = 456; // { dg-error "internal linkage" }
> - export void h(); // { dg-error "internal linkage" }
> - export void i() {} // { dg-error "internal linkage" }
> - export template <typename T> void v(); // { dg-error "internal linkage" }
> - export template <typename T> void w() {} // { dg-error "internal linkage" }
> - export auto [e] = S{}; // { dg-error "internal linkage" }
> + export int y = 456; // { dg-error "exporting" }
> + export void h(); // { dg-error "exporting" }
> + export void i() {} // { dg-error "exporting" }
> + export template <typename T> void v(); // { dg-error "exporting" }
> + export template <typename T> void w() {} // { dg-error "exporting" }
> + export auto [e] = S{}; // { dg-error "exporting" }
>
> - export namespace ns {} // { dg-error "internal linkage" }
> - export namespace alias = global; // { dg-error "internal linkage" }
> + export namespace ns {} // { dg-error "exporting" }
> + export namespace alias = global; // { dg-error "exporting" }
>
> - export struct A {}; // { dg-error "internal linkage" }
> - export template <typename T> struct B {}; // { dg-error "internal
> linkage" }
> + export struct A {}; // { dg-error "exporting" }
> + export template <typename T> struct B {}; // { dg-error "exporting" }
>
> - export enum E {}; // { dg-error "internal linkage" }
> - export enum class F {}; // { dg-error "internal linkage" }
> + export enum E {}; // { dg-error "exporting" }
> + export enum class F {}; // { dg-error "exporting" }
>
> - export template <typename T> using U = int; // { dg-error "internal
> linkage" }
> + export template <typename T> using U = int; // { dg-error "exporting" }
>
> #if __cplusplus >= 202002L
> - export template <typename T> concept C = true; // { dg-error "internal
> linkage" "" { target c++20 } }
> + export template <typename T> concept C = true; // { dg-error "exporting"
> "" { target c++20 } }
> #endif
> +
> + // Also complain about exporting no-linkage decls in an unnamed namespace
> + export typedef int T; // { dg-error "exporting" }
> + export typedef struct {} *PC; // { dg-error "exporting" }
> + export using V = int; // { dg-error "exporting" }
> }
>
> export namespace {} // { dg-error "exporting unnamed namespace" }
> diff --git a/gcc/testsuite/g++.dg/modules/internal-4_b.C
> b/gcc/testsuite/g++.dg/modules/internal-4_b.C
> index 81fc65a69bd..7e4fbfeb1fb 100644
> --- a/gcc/testsuite/g++.dg/modules/internal-4_b.C
> +++ b/gcc/testsuite/g++.dg/modules/internal-4_b.C
> @@ -32,8 +32,8 @@ inline void expose_header_decl() { // { dg-error "exposes
> TU-local entity" }
> header_f();
> }
>
> -// We additionally consider a namespace with internal linkage as TU-local
> -namespace expose_ns = internal_ns; // { dg-error "exposes TU-local entity" }
> +// A namespace with internal linkage is not inherently an exposure.
> +namespace expose_ns = internal_ns; // { dg-bogus "exposes TU-local entity" }
>
> // But we don't consider a weakref as being TU-local, despite being
> // marked static; this is to support uses of weakrefs in header files
> diff --git a/gcc/testsuite/g++.dg/modules/using-30_a.C
> b/gcc/testsuite/g++.dg/modules/using-30_a.C
> new file mode 100644
> index 00000000000..fc6a67f2bae
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/using-30_a.C
> @@ -0,0 +1,9 @@
> +// { dg-additional-options "-fmodules" }
> +// { dg-module-cmi M }
> +
> +export module M;
> +
> +namespace {
> + using A = int;
> + typedef int B;
> +}
> diff --git a/gcc/testsuite/g++.dg/modules/using-30_b.C
> b/gcc/testsuite/g++.dg/modules/using-30_b.C
> new file mode 100644
> index 00000000000..ba06c9de792
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/using-30_b.C
> @@ -0,0 +1,6 @@
> +// { dg-additional-options "-fmodules" }
> +
> +module M;
> +
> +A x;
> +B y;
> diff --git a/gcc/testsuite/g++.dg/modules/using-30_c.C
> b/gcc/testsuite/g++.dg/modules/using-30_c.C
> new file mode 100644
> index 00000000000..34dc6f2c98d
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/using-30_c.C
> @@ -0,0 +1,13 @@
> +// { dg-additional-options "-fmodules" }
> +// The different declarations in the anonymous namespace shouldn't clash with
> +// those in M.
> +
> +namespace {
> + using A = double;
> + typedef double B;
> +}
> +import M;
> +int main() {
> + A a = 1.0;
> + B b = 2.0;
> +}
> --
> 2.47.0
>
