[v2,3/6] c++/modules: Support anonymous namespaces in header units
Checks
| Context |
Check |
Description |
| linaro-tcwg-bot/tcwg_gcc_build--master-arm |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 |
success
|
Build passed
|
Commit Message
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
Comments
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;
> +}
@@ -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))
@@ -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);
}
}
new file mode 100644
@@ -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;
new file mode 100644
@@ -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;
+}