[v2,3/6] c++/modules: Support anonymous namespaces in header units

Bootstrapped and regtested on x86_64-pc-linux-gnu and aarch64-unknown-linux-gnu,
OK for trunk?

-- >8 --

A header unit may contain anonymous namespaces, and those declarations
are exported (as with any declaration in a header unit).  This patch
ensures that such declarations are correctly handled.

The change to 'make_namespace_finish' is required so that if an
anonymous namespace is first seen by an import it is correctly handled
within 'add_imported_namespace'.  I don't see any particular reason why
handling of anonymous namespaces here had to be handled separately
outside that function since these are the only two callers.

gcc/cp/ChangeLog:

	* module.cc (depset::hash::add_binding_entity): Also walk
	anonymous namespaces.
	(module_state::write_namespaces): Adjust assertion.
	* name-lookup.cc (push_namespace): Move anon using-directive
	handling to...
	(make_namespace_finish): ...here.

gcc/testsuite/ChangeLog:

	* g++.dg/modules/internal-8_a.H: New test.
	* g++.dg/modules/internal-8_b.C: New test.

Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
---
 gcc/cp/module.cc                            |  7 +++--
 gcc/cp/name-lookup.cc                       |  8 +++---
 gcc/testsuite/g++.dg/modules/internal-8_a.H | 28 ++++++++++++++++++++
 gcc/testsuite/g++.dg/modules/internal-8_b.C | 29 +++++++++++++++++++++
 4 files changed, 63 insertions(+), 9 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/modules/internal-8_a.H
 create mode 100644 gcc/testsuite/g++.dg/modules/internal-8_b.C

On 9/27/24 1:58 AM, Nathaniel Shead wrote:
> Bootstrapped and regtested on x86_64-pc-linux-gnu and aarch64-unknown-linux-gnu,
> OK for trunk?
> 
> -- >8 --
> 
> A header unit may contain anonymous namespaces, and those declarations
> are exported (as with any declaration in a header unit).  This patch
> ensures that such declarations are correctly handled.
> 
> The change to 'make_namespace_finish' is required so that if an
> anonymous namespace is first seen by an import it is correctly handled
> within 'add_imported_namespace'.  I don't see any particular reason why
> handling of anonymous namespaces here had to be handled separately
> outside that function since these are the only two callers.

Please add a rationale comment to that hunk; OK with that change.

> gcc/cp/ChangeLog:
> 
> 	* module.cc (depset::hash::add_binding_entity): Also walk
> 	anonymous namespaces.
> 	(module_state::write_namespaces): Adjust assertion.
> 	* name-lookup.cc (push_namespace): Move anon using-directive
> 	handling to...
> 	(make_namespace_finish): ...here.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/modules/internal-8_a.H: New test.
> 	* g++.dg/modules/internal-8_b.C: New test.
> 
> Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
> ---
>   gcc/cp/module.cc                            |  7 +++--
>   gcc/cp/name-lookup.cc                       |  8 +++---
>   gcc/testsuite/g++.dg/modules/internal-8_a.H | 28 ++++++++++++++++++++
>   gcc/testsuite/g++.dg/modules/internal-8_b.C | 29 +++++++++++++++++++++
>   4 files changed, 63 insertions(+), 9 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/modules/internal-8_a.H
>   create mode 100644 gcc/testsuite/g++.dg/modules/internal-8_b.C
> 
> diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> index 5d7f4941b2a..df407c1fd55 100644
> --- a/gcc/cp/module.cc
> +++ b/gcc/cp/module.cc
> @@ -13723,15 +13723,15 @@ depset::hash::add_binding_entity (tree decl, WMB_Flags flags, void *data_)
>         return (flags & WMB_Using
>   	      ? flags & WMB_Export : DECL_MODULE_EXPORT_P (decl));
>       }
> -  else if (DECL_NAME (decl) && !data->met_namespace)
> +  else if (!data->met_namespace)
>       {
>         /* Namespace, walk exactly once.  */
> -      gcc_checking_assert (TREE_PUBLIC (decl));
>         data->met_namespace = true;
>         if (data->hash->add_namespace_entities (decl, data->partitions))
>   	{
>   	  /* It contains an exported thing, so it is exported.  */
>   	  gcc_checking_assert (DECL_MODULE_PURVIEW_P (decl));
> +	  gcc_checking_assert (TREE_PUBLIC (decl) || header_module_p ());
>   	  DECL_MODULE_EXPORT_P (decl) = true;
>   	}
>   
> @@ -16126,8 +16126,7 @@ module_state::write_namespaces (elf_out *to, vec<depset *> spaces,
>         tree ns = b->get_entity ();
>   
>         gcc_checking_assert (TREE_CODE (ns) == NAMESPACE_DECL);
> -      /* P1815 may have something to say about this.  */
> -      gcc_checking_assert (TREE_PUBLIC (ns));
> +      gcc_checking_assert (TREE_PUBLIC (ns) || header_module_p ());
>   
>         unsigned flags = 0;
>         if (TREE_PUBLIC (ns))
> diff --git a/gcc/cp/name-lookup.cc b/gcc/cp/name-lookup.cc
> index eb365b259d9..fe2eb2e0917 100644
> --- a/gcc/cp/name-lookup.cc
> +++ b/gcc/cp/name-lookup.cc
> @@ -9098,6 +9098,9 @@ make_namespace_finish (tree ns, tree *slot, bool from_import = false)
>   
>     if (DECL_NAMESPACE_INLINE_P (ns) || !DECL_NAME (ns))
>       emit_debug_info_using_namespace (ctx, ns, true);
> +
> +  if (!DECL_NAMESPACE_INLINE_P (ns) && !DECL_NAME (ns))
> +    add_using_namespace (NAMESPACE_LEVEL (ctx)->using_directives, ns);
>   }
>   
>   /* Push into the scope of the NAME namespace.  If NAME is NULL_TREE,
> @@ -9234,11 +9237,6 @@ push_namespace (tree name, bool make_inline)
>   	      gcc_checking_assert (slot);
>   	    }
>   	  make_namespace_finish (ns, slot);
> -
> -	  /* Add the anon using-directive here, we don't do it in
> -	     make_namespace_finish.  */
> -	  if (!DECL_NAMESPACE_INLINE_P (ns) && !name)
> -	    add_using_namespace (current_binding_level->using_directives, ns);
>   	}
>       }
>   
> diff --git a/gcc/testsuite/g++.dg/modules/internal-8_a.H b/gcc/testsuite/g++.dg/modules/internal-8_a.H
> new file mode 100644
> index 00000000000..57fe60bb3c0
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/internal-8_a.H
> @@ -0,0 +1,28 @@
> +// { dg-additional-options "-fmodule-header" }
> +// { dg-module-cmi {} }
> +
> +static int x = 123;
> +static void f() {}
> +template <typename T> static void t() {}
> +
> +namespace {
> +  int y = 456;
> +  void g() {};
> +  template <typename T> void u() {}
> +
> +  namespace ns { int in_ns = 456; }
> +
> +  struct A {};
> +  template <typename T> struct B {};
> +
> +  enum E { X };
> +  enum class F { Y };
> +
> +  template <typename T> using U = int;
> +
> +#if __cplusplus >= 202002L
> +  template <typename T> concept C = true;
> +#endif
> +}
> +
> +namespace ns2 = ns;
> diff --git a/gcc/testsuite/g++.dg/modules/internal-8_b.C b/gcc/testsuite/g++.dg/modules/internal-8_b.C
> new file mode 100644
> index 00000000000..a2d74a87473
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/internal-8_b.C
> @@ -0,0 +1,29 @@
> +// { dg-additional-options "-fmodules-ts" }
> +
> +import "internal-8_a.H";
> +
> +int main() {
> +  auto x2 = x;
> +  f();
> +  t<int>();
> +
> +  auto y2 = y;
> +  g();
> +  u<int>();
> +
> +  int val1 = ns::in_ns;
> +
> +  A a;
> +  B<int> b;
> +
> +  E e = X;
> +  F f = F::Y;
> +
> +  U<int> temp;
> +
> +#if __cplusplus >= 202002L
> +  static_assert(C<int>);
> +#endif
> +
> +  int val2 = ns2::in_ns;
> +}