[PATCH 0/3] c++/modules: Fix some small issues with exported using-decls

[PATCH 0/3] c++/modules: Fix some small issues with exported using-decls

[Nathaniel Shead]

This patch series fixes a few issues with exported using-declarations that have
come up recently.

I was hoping to fix PR114683 and PR114685 as well, but the issues there look to
be much more involved and I'm not yet sure how to handle them.  They're
ultimately caused by using-declarations just inserting the value directly into
the given scope without any wrapping for non-functions, which means that
there's no indication of which using-decls in what scopes are exported.  I'd
hoped to try to repurpose the stat_hack machinery for this but after trying for
a while it doesn't seem to be appropriate.

Nathaniel Shead (3):
  c++: Only emit exported GMF usings [PR114600]
  c++: Propagate using decls from partitions
  c++: Propagate hidden flag on decls from partitions

 gcc/cp/module.cc                          |  6 +++++-
 gcc/cp/name-lookup.cc                     | 10 +++++-----
 gcc/testsuite/g++.dg/modules/using-14.C   | 14 ++++++++++++++
 gcc/testsuite/g++.dg/modules/using-15_a.C | 13 +++++++++++++
 gcc/testsuite/g++.dg/modules/using-15_b.C |  5 +++++
 gcc/testsuite/g++.dg/modules/using-15_c.C |  7 +++++++
 gcc/testsuite/g++.dg/modules/using-16_a.C | 11 +++++++++++
 gcc/testsuite/g++.dg/modules/using-16_b.C | 12 ++++++++++++
 gcc/testsuite/g++.dg/modules/using-16_c.C | 11 +++++++++++
 9 files changed, 83 insertions(+), 6 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/modules/using-14.C
 create mode 100644 gcc/testsuite/g++.dg/modules/using-15_a.C
 create mode 100644 gcc/testsuite/g++.dg/modules/using-15_b.C
 create mode 100644 gcc/testsuite/g++.dg/modules/using-15_c.C
 create mode 100644 gcc/testsuite/g++.dg/modules/using-16_a.C
 create mode 100644 gcc/testsuite/g++.dg/modules/using-16_b.C
 create mode 100644 gcc/testsuite/g++.dg/modules/using-16_c.C

-- 
2.43.2

[PATCH 1/3] c++/modules: Only emit exported GMF usings [PR114600]

[Nathaniel Shead]

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

-- >8 --

A typo in r14-6978 made us emit too many things. This ensures that we
don't emit using-declarations from the GMF that we don't need to.

	PR c++/114600

gcc/cp/ChangeLog:

	* module.cc (depset::hash::add_binding_entity): Require both
	WMB_Using and WMB_Export for GMF entities.

gcc/testsuite/ChangeLog:

	* g++.dg/modules/using-14.C: New test.

Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
Co-authored-by: Patrick Palka <ppalka@redhat.com>
---
 gcc/cp/module.cc                        |  2 +-
 gcc/testsuite/g++.dg/modules/using-14.C | 14 ++++++++++++++
 2 files changed, 15 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/g++.dg/modules/using-14.C

diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index 4e91fa6e052..9d054c4c792 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -12892,7 +12892,7 @@ depset::hash::add_binding_entity (tree decl, WMB_Flags flags, void *data_)
 	inner = DECL_TEMPLATE_RESULT (inner);
 
       if ((!DECL_LANG_SPECIFIC (inner) || !DECL_MODULE_PURVIEW_P (inner))
-	  && !(flags & (WMB_Using | WMB_Export)))
+	  && !((flags & WMB_Using) && (flags & WMB_Export)))
 	/* Ignore global module fragment entities unless explicitly
 	   exported with a using declaration.  */
 	return false;
diff --git a/gcc/testsuite/g++.dg/modules/using-14.C b/gcc/testsuite/g++.dg/modules/using-14.C
new file mode 100644
index 00000000000..0e15a952de5
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/using-14.C
@@ -0,0 +1,14 @@
+// PR c++/114600
+// { dg-additional-options "-fmodules-ts -Wno-global-module -fdump-lang-module" }
+// { dg-module-cmi M }
+
+module;
+namespace std {
+  template<class T> struct A { int n; };
+  template<class T> A<T> f();
+  namespace __swappable_details { using std::f; }
+}
+export module M;
+
+// The whole GMF should be discarded here
+// { dg-final { scan-lang-dump "Wrote 0 clusters" module } }
-- 
2.43.2

Re: [PATCH 1/3] c++/modules: Only emit exported GMF usings [PR114600]

[Jason Merrill]

On 4/11/24 20:40, Nathaniel Shead wrote:
> Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?

OK.

> -- >8 --
> 
> A typo in r14-6978 made us emit too many things. This ensures that we
> don't emit using-declarations from the GMF that we don't need to.
> 
> 	PR c++/114600
> 
> gcc/cp/ChangeLog:
> 
> 	* module.cc (depset::hash::add_binding_entity): Require both
> 	WMB_Using and WMB_Export for GMF entities.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/modules/using-14.C: New test.
> 
> Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
> Co-authored-by: Patrick Palka <ppalka@redhat.com>
> ---
>   gcc/cp/module.cc                        |  2 +-
>   gcc/testsuite/g++.dg/modules/using-14.C | 14 ++++++++++++++
>   2 files changed, 15 insertions(+), 1 deletion(-)
>   create mode 100644 gcc/testsuite/g++.dg/modules/using-14.C
> 
> diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> index 4e91fa6e052..9d054c4c792 100644
> --- a/gcc/cp/module.cc
> +++ b/gcc/cp/module.cc
> @@ -12892,7 +12892,7 @@ depset::hash::add_binding_entity (tree decl, WMB_Flags flags, void *data_)
>   	inner = DECL_TEMPLATE_RESULT (inner);
>   
>         if ((!DECL_LANG_SPECIFIC (inner) || !DECL_MODULE_PURVIEW_P (inner))
> -	  && !(flags & (WMB_Using | WMB_Export)))
> +	  && !((flags & WMB_Using) && (flags & WMB_Export)))
>   	/* Ignore global module fragment entities unless explicitly
>   	   exported with a using declaration.  */
>   	return false;
> diff --git a/gcc/testsuite/g++.dg/modules/using-14.C b/gcc/testsuite/g++.dg/modules/using-14.C
> new file mode 100644
> index 00000000000..0e15a952de5
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/using-14.C
> @@ -0,0 +1,14 @@
> +// PR c++/114600
> +// { dg-additional-options "-fmodules-ts -Wno-global-module -fdump-lang-module" }
> +// { dg-module-cmi M }
> +
> +module;
> +namespace std {
> +  template<class T> struct A { int n; };
> +  template<class T> A<T> f();
> +  namespace __swappable_details { using std::f; }
> +}
> +export module M;
> +
> +// The whole GMF should be discarded here
> +// { dg-final { scan-lang-dump "Wrote 0 clusters" module } }

[PATCH 2/3] c++/modules: Propagate using decls from partitions

[Nathaniel Shead]

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

-- >8 --

The modules code currently neglects to set OVL_USING_P on the dependency
created for a using-decl, which causes it not to remember that the
OVL_EXPORT_P flag had been set on it when emitted from the primary
interface unit. This patch ensures that it occurs.

gcc/cp/ChangeLog:

	* module.cc (depset::hash::add_binding_entity): Propagate
	OVL_USING_P for using-declarations.

gcc/testsuite/ChangeLog:

	* g++.dg/modules/using-15_a.C: New test.
	* g++.dg/modules/using-15_b.C: New test.
	* g++.dg/modules/using-15_c.C: New test.

Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
---
 gcc/cp/module.cc                          |  4 ++++
 gcc/testsuite/g++.dg/modules/using-15_a.C | 13 +++++++++++++
 gcc/testsuite/g++.dg/modules/using-15_b.C |  5 +++++
 gcc/testsuite/g++.dg/modules/using-15_c.C |  7 +++++++
 4 files changed, 29 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/modules/using-15_a.C
 create mode 100644 gcc/testsuite/g++.dg/modules/using-15_b.C
 create mode 100644 gcc/testsuite/g++.dg/modules/using-15_c.C

diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index 9d054c4c792..527c9046c67 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -12915,10 +12915,12 @@ depset::hash::add_binding_entity (tree decl, WMB_Flags flags, void *data_)
 	/* Ignore NTTP objects.  */
 	return false;
 
+      bool unscoped_enum_const_p = false;
       if (!(flags & WMB_Using) && CP_DECL_CONTEXT (decl) != data->ns)
 	{
 	  /* A using that lost its wrapper or an unscoped enum
 	     constant.  */
+	  unscoped_enum_const_p = (TREE_CODE (decl) == CONST_DECL);
 	  flags = WMB_Flags (flags | WMB_Using);
 	  if (DECL_MODULE_EXPORT_P (TREE_CODE (decl) == CONST_DECL
 				    ? TYPE_NAME (TREE_TYPE (decl))
@@ -12979,6 +12981,8 @@ depset::hash::add_binding_entity (tree decl, WMB_Flags flags, void *data_)
       if (flags & WMB_Using)
 	{
 	  decl = ovl_make (decl, NULL_TREE);
+	  if (!unscoped_enum_const_p)
+	    OVL_USING_P (decl) = true;
 	  if (flags & WMB_Export)
 	    OVL_EXPORT_P (decl) = true;
 	}
diff --git a/gcc/testsuite/g++.dg/modules/using-15_a.C b/gcc/testsuite/g++.dg/modules/using-15_a.C
new file mode 100644
index 00000000000..23895bd8c4a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/using-15_a.C
@@ -0,0 +1,13 @@
+// { dg-additional-options "-fmodules-ts -Wno-global-module" }
+// { dg-module-cmi M:a }
+
+module;
+namespace foo {
+  void a();
+};
+export module M:a;
+
+namespace bar {
+  // propagate usings from partitions
+  export using foo::a;
+};
diff --git a/gcc/testsuite/g++.dg/modules/using-15_b.C b/gcc/testsuite/g++.dg/modules/using-15_b.C
new file mode 100644
index 00000000000..a88f86af61f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/using-15_b.C
@@ -0,0 +1,5 @@
+// { dg-additional-options "-fmodules-ts" }
+// { dg-module-cmi M }
+
+export module M;
+export import :a;
diff --git a/gcc/testsuite/g++.dg/modules/using-15_c.C b/gcc/testsuite/g++.dg/modules/using-15_c.C
new file mode 100644
index 00000000000..0651efffc91
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/using-15_c.C
@@ -0,0 +1,7 @@
+// { dg-additional-options "-fmodules-ts" }
+import M;
+
+int main() {
+  bar::a();
+  foo::a();  // { dg-error "not been declared" }
+}
-- 
2.43.2

Re: [PATCH 2/3] c++/modules: Propagate using decls from partitions

[Jason Merrill]

On 4/11/24 20:40, Nathaniel Shead wrote:
> Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?
> 
> -- >8 --
> 
> The modules code currently neglects to set OVL_USING_P on the dependency
> created for a using-decl, which causes it not to remember that the
> OVL_EXPORT_P flag had been set on it when emitted from the primary
> interface unit. This patch ensures that it occurs.
> 
> gcc/cp/ChangeLog:
> 
> 	* module.cc (depset::hash::add_binding_entity): Propagate
> 	OVL_USING_P for using-declarations.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/modules/using-15_a.C: New test.
> 	* g++.dg/modules/using-15_b.C: New test.
> 	* g++.dg/modules/using-15_c.C: New test.
> 
> Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
> ---
>   gcc/cp/module.cc                          |  4 ++++
>   gcc/testsuite/g++.dg/modules/using-15_a.C | 13 +++++++++++++
>   gcc/testsuite/g++.dg/modules/using-15_b.C |  5 +++++
>   gcc/testsuite/g++.dg/modules/using-15_c.C |  7 +++++++
>   4 files changed, 29 insertions(+)
>   create mode 100644 gcc/testsuite/g++.dg/modules/using-15_a.C
>   create mode 100644 gcc/testsuite/g++.dg/modules/using-15_b.C
>   create mode 100644 gcc/testsuite/g++.dg/modules/using-15_c.C
> 
> diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> index 9d054c4c792..527c9046c67 100644
> --- a/gcc/cp/module.cc
> +++ b/gcc/cp/module.cc
> @@ -12915,10 +12915,12 @@ depset::hash::add_binding_entity (tree decl, WMB_Flags flags, void *data_)
>   	/* Ignore NTTP objects.  */
>   	return false;
>   
> +      bool unscoped_enum_const_p = false;
>         if (!(flags & WMB_Using) && CP_DECL_CONTEXT (decl) != data->ns)
>   	{
>   	  /* A using that lost its wrapper or an unscoped enum
>   	     constant.  */
> +	  unscoped_enum_const_p = (TREE_CODE (decl) == CONST_DECL);

How does this interact with C++20 using enum?

>   	  flags = WMB_Flags (flags | WMB_Using);
>   	  if (DECL_MODULE_EXPORT_P (TREE_CODE (decl) == CONST_DECL
>   				    ? TYPE_NAME (TREE_TYPE (decl))
> @@ -12979,6 +12981,8 @@ depset::hash::add_binding_entity (tree decl, WMB_Flags flags, void *data_)
>         if (flags & WMB_Using)
>   	{
>   	  decl = ovl_make (decl, NULL_TREE);
> +	  if (!unscoped_enum_const_p)
> +	    OVL_USING_P (decl) = true;
>   	  if (flags & WMB_Export)
>   	    OVL_EXPORT_P (decl) = true;
>   	}
> diff --git a/gcc/testsuite/g++.dg/modules/using-15_a.C b/gcc/testsuite/g++.dg/modules/using-15_a.C
> new file mode 100644
> index 00000000000..23895bd8c4a
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/using-15_a.C
> @@ -0,0 +1,13 @@
> +// { dg-additional-options "-fmodules-ts -Wno-global-module" }
> +// { dg-module-cmi M:a }
> +
> +module;
> +namespace foo {
> +  void a();
> +};
> +export module M:a;
> +
> +namespace bar {
> +  // propagate usings from partitions
> +  export using foo::a;
> +};
> diff --git a/gcc/testsuite/g++.dg/modules/using-15_b.C b/gcc/testsuite/g++.dg/modules/using-15_b.C
> new file mode 100644
> index 00000000000..a88f86af61f
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/using-15_b.C
> @@ -0,0 +1,5 @@
> +// { dg-additional-options "-fmodules-ts" }
> +// { dg-module-cmi M }
> +
> +export module M;
> +export import :a;
> diff --git a/gcc/testsuite/g++.dg/modules/using-15_c.C b/gcc/testsuite/g++.dg/modules/using-15_c.C
> new file mode 100644
> index 00000000000..0651efffc91
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/using-15_c.C
> @@ -0,0 +1,7 @@
> +// { dg-additional-options "-fmodules-ts" }
> +import M;
> +
> +int main() {
> +  bar::a();
> +  foo::a();  // { dg-error "not been declared" }
> +}

Re: [PATCH 2/3] c++/modules: Propagate using decls from partitions

[Nathaniel Shead]

On Fri, Apr 12, 2024 at 01:50:47PM -0400, Jason Merrill wrote:
> On 4/11/24 20:40, Nathaniel Shead wrote:
> > Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?
> > 
> > -- >8 --
> > 
> > The modules code currently neglects to set OVL_USING_P on the dependency
> > created for a using-decl, which causes it not to remember that the
> > OVL_EXPORT_P flag had been set on it when emitted from the primary
> > interface unit. This patch ensures that it occurs.
> > 
> > gcc/cp/ChangeLog:
> > 
> > 	* module.cc (depset::hash::add_binding_entity): Propagate
> > 	OVL_USING_P for using-declarations.
> > 
> > gcc/testsuite/ChangeLog:
> > 
> > 	* g++.dg/modules/using-15_a.C: New test.
> > 	* g++.dg/modules/using-15_b.C: New test.
> > 	* g++.dg/modules/using-15_c.C: New test.
> > 
> > Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
> > ---
> >   gcc/cp/module.cc                          |  4 ++++
> >   gcc/testsuite/g++.dg/modules/using-15_a.C | 13 +++++++++++++
> >   gcc/testsuite/g++.dg/modules/using-15_b.C |  5 +++++
> >   gcc/testsuite/g++.dg/modules/using-15_c.C |  7 +++++++
> >   4 files changed, 29 insertions(+)
> >   create mode 100644 gcc/testsuite/g++.dg/modules/using-15_a.C
> >   create mode 100644 gcc/testsuite/g++.dg/modules/using-15_b.C
> >   create mode 100644 gcc/testsuite/g++.dg/modules/using-15_c.C
> > 
> > diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> > index 9d054c4c792..527c9046c67 100644
> > --- a/gcc/cp/module.cc
> > +++ b/gcc/cp/module.cc
> > @@ -12915,10 +12915,12 @@ depset::hash::add_binding_entity (tree decl, WMB_Flags flags, void *data_)
> >   	/* Ignore NTTP objects.  */
> >   	return false;
> > +      bool unscoped_enum_const_p = false;
> >         if (!(flags & WMB_Using) && CP_DECL_CONTEXT (decl) != data->ns)
> >   	{
> >   	  /* A using that lost its wrapper or an unscoped enum
> >   	     constant.  */
> > +	  unscoped_enum_const_p = (TREE_CODE (decl) == CONST_DECL);
> 
> How does this interact with C++20 using enum?

Looks like it ignores those (so they still suffer from this error).  But
in general we don't handle usings of non-functions correctly anyway yet
(for the reasons I described in the cover letter); I just added this for
now to prevent regressing some test-cases caused by importing enum
consts wrapped in an OVERLOAD.

Otherwise happy to defer this patch until GCC 15 when I can look at
exploring what needs to be done to handle non-function using-decls
correctly, but I'll need to work out a new testcase for the followup
patch in this series (or just defer that one too, I suppose).

> >   	  flags = WMB_Flags (flags | WMB_Using);
> >   	  if (DECL_MODULE_EXPORT_P (TREE_CODE (decl) == CONST_DECL
> >   				    ? TYPE_NAME (TREE_TYPE (decl))
> > @@ -12979,6 +12981,8 @@ depset::hash::add_binding_entity (tree decl, WMB_Flags flags, void *data_)
> >         if (flags & WMB_Using)
> >   	{
> >   	  decl = ovl_make (decl, NULL_TREE);
> > +	  if (!unscoped_enum_const_p)
> > +	    OVL_USING_P (decl) = true;
> >   	  if (flags & WMB_Export)
> >   	    OVL_EXPORT_P (decl) = true;
> >   	}
> > diff --git a/gcc/testsuite/g++.dg/modules/using-15_a.C b/gcc/testsuite/g++.dg/modules/using-15_a.C
> > new file mode 100644
> > index 00000000000..23895bd8c4a
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/modules/using-15_a.C
> > @@ -0,0 +1,13 @@
> > +// { dg-additional-options "-fmodules-ts -Wno-global-module" }
> > +// { dg-module-cmi M:a }
> > +
> > +module;
> > +namespace foo {
> > +  void a();
> > +};
> > +export module M:a;
> > +
> > +namespace bar {
> > +  // propagate usings from partitions
> > +  export using foo::a;
> > +};
> > diff --git a/gcc/testsuite/g++.dg/modules/using-15_b.C b/gcc/testsuite/g++.dg/modules/using-15_b.C
> > new file mode 100644
> > index 00000000000..a88f86af61f
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/modules/using-15_b.C
> > @@ -0,0 +1,5 @@
> > +// { dg-additional-options "-fmodules-ts" }
> > +// { dg-module-cmi M }
> > +
> > +export module M;
> > +export import :a;
> > diff --git a/gcc/testsuite/g++.dg/modules/using-15_c.C b/gcc/testsuite/g++.dg/modules/using-15_c.C
> > new file mode 100644
> > index 00000000000..0651efffc91
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/modules/using-15_c.C
> > @@ -0,0 +1,7 @@
> > +// { dg-additional-options "-fmodules-ts" }
> > +import M;
> > +
> > +int main() {
> > +  bar::a();
> > +  foo::a();  // { dg-error "not been declared" }
> > +}
> 

Re: [PATCH 2/3] c++/modules: Propagate using decls from partitions

[Nathaniel Shead]

On Sun, Apr 14, 2024 at 01:40:18AM +1000, Nathaniel Shead wrote:
> On Fri, Apr 12, 2024 at 01:50:47PM -0400, Jason Merrill wrote:
> > On 4/11/24 20:40, Nathaniel Shead wrote:
> > > Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?
> > > 
> > > -- >8 --
> > > 
> > > The modules code currently neglects to set OVL_USING_P on the dependency
> > > created for a using-decl, which causes it not to remember that the
> > > OVL_EXPORT_P flag had been set on it when emitted from the primary
> > > interface unit. This patch ensures that it occurs.
> > > 
> > > gcc/cp/ChangeLog:
> > > 
> > > 	* module.cc (depset::hash::add_binding_entity): Propagate
> > > 	OVL_USING_P for using-declarations.
> > > 
> > > gcc/testsuite/ChangeLog:
> > > 
> > > 	* g++.dg/modules/using-15_a.C: New test.
> > > 	* g++.dg/modules/using-15_b.C: New test.
> > > 	* g++.dg/modules/using-15_c.C: New test.
> > > 
> > > Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
> > > ---
> > >   gcc/cp/module.cc                          |  4 ++++
> > >   gcc/testsuite/g++.dg/modules/using-15_a.C | 13 +++++++++++++
> > >   gcc/testsuite/g++.dg/modules/using-15_b.C |  5 +++++
> > >   gcc/testsuite/g++.dg/modules/using-15_c.C |  7 +++++++
> > >   4 files changed, 29 insertions(+)
> > >   create mode 100644 gcc/testsuite/g++.dg/modules/using-15_a.C
> > >   create mode 100644 gcc/testsuite/g++.dg/modules/using-15_b.C
> > >   create mode 100644 gcc/testsuite/g++.dg/modules/using-15_c.C
> > > 
> > > diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> > > index 9d054c4c792..527c9046c67 100644
> > > --- a/gcc/cp/module.cc
> > > +++ b/gcc/cp/module.cc
> > > @@ -12915,10 +12915,12 @@ depset::hash::add_binding_entity (tree decl, WMB_Flags flags, void *data_)
> > >   	/* Ignore NTTP objects.  */
> > >   	return false;
> > > +      bool unscoped_enum_const_p = false;
> > >         if (!(flags & WMB_Using) && CP_DECL_CONTEXT (decl) != data->ns)
> > >   	{
> > >   	  /* A using that lost its wrapper or an unscoped enum
> > >   	     constant.  */
> > > +	  unscoped_enum_const_p = (TREE_CODE (decl) == CONST_DECL);
> > 
> > How does this interact with C++20 using enum?
> 
> Looks like it ignores those (so they still suffer from this error).  But
> in general we don't handle usings of non-functions correctly anyway yet
> (for the reasons I described in the cover letter); I just added this for
> now to prevent regressing some test-cases caused by importing enum
> consts wrapped in an OVERLOAD.
> 
> Otherwise happy to defer this patch until GCC 15 when I can look at
> exploring what needs to be done to handle non-function using-decls
> correctly, but I'll need to work out a new testcase for the followup
> patch in this series (or just defer that one too, I suppose).
> 

Ping.  Or should I just scrap this patch for now, find a new testcase
for the followup patch, and submit it again once we have a general
solution for using-decls of non-functions?

> > >   	  flags = WMB_Flags (flags | WMB_Using);
> > >   	  if (DECL_MODULE_EXPORT_P (TREE_CODE (decl) == CONST_DECL
> > >   				    ? TYPE_NAME (TREE_TYPE (decl))
> > > @@ -12979,6 +12981,8 @@ depset::hash::add_binding_entity (tree decl, WMB_Flags flags, void *data_)
> > >         if (flags & WMB_Using)
> > >   	{
> > >   	  decl = ovl_make (decl, NULL_TREE);
> > > +	  if (!unscoped_enum_const_p)
> > > +	    OVL_USING_P (decl) = true;
> > >   	  if (flags & WMB_Export)
> > >   	    OVL_EXPORT_P (decl) = true;
> > >   	}
> > > diff --git a/gcc/testsuite/g++.dg/modules/using-15_a.C b/gcc/testsuite/g++.dg/modules/using-15_a.C
> > > new file mode 100644
> > > index 00000000000..23895bd8c4a
> > > --- /dev/null
> > > +++ b/gcc/testsuite/g++.dg/modules/using-15_a.C
> > > @@ -0,0 +1,13 @@
> > > +// { dg-additional-options "-fmodules-ts -Wno-global-module" }
> > > +// { dg-module-cmi M:a }
> > > +
> > > +module;
> > > +namespace foo {
> > > +  void a();
> > > +};
> > > +export module M:a;
> > > +
> > > +namespace bar {
> > > +  // propagate usings from partitions
> > > +  export using foo::a;
> > > +};
> > > diff --git a/gcc/testsuite/g++.dg/modules/using-15_b.C b/gcc/testsuite/g++.dg/modules/using-15_b.C
> > > new file mode 100644
> > > index 00000000000..a88f86af61f
> > > --- /dev/null
> > > +++ b/gcc/testsuite/g++.dg/modules/using-15_b.C
> > > @@ -0,0 +1,5 @@
> > > +// { dg-additional-options "-fmodules-ts" }
> > > +// { dg-module-cmi M }
> > > +
> > > +export module M;
> > > +export import :a;
> > > diff --git a/gcc/testsuite/g++.dg/modules/using-15_c.C b/gcc/testsuite/g++.dg/modules/using-15_c.C
> > > new file mode 100644
> > > index 00000000000..0651efffc91
> > > --- /dev/null
> > > +++ b/gcc/testsuite/g++.dg/modules/using-15_c.C
> > > @@ -0,0 +1,7 @@
> > > +// { dg-additional-options "-fmodules-ts" }
> > > +import M;
> > > +
> > > +int main() {
> > > +  bar::a();
> > > +  foo::a();  // { dg-error "not been declared" }
> > > +}
> > 

Re: [PATCH 2/3] c++/modules: Propagate using decls from partitions

[Jason Merrill]

On 4/30/24 00:59, Nathaniel Shead wrote:
> On Sun, Apr 14, 2024 at 01:40:18AM +1000, Nathaniel Shead wrote:
>> On Fri, Apr 12, 2024 at 01:50:47PM -0400, Jason Merrill wrote:
>>> On 4/11/24 20:40, Nathaniel Shead wrote:
>>>> Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?
>>>>
>>>> -- >8 --
>>>>
>>>> The modules code currently neglects to set OVL_USING_P on the dependency
>>>> created for a using-decl, which causes it not to remember that the
>>>> OVL_EXPORT_P flag had been set on it when emitted from the primary
>>>> interface unit. This patch ensures that it occurs.
>>>>
>>>> gcc/cp/ChangeLog:
>>>>
>>>> 	* module.cc (depset::hash::add_binding_entity): Propagate
>>>> 	OVL_USING_P for using-declarations.
>>>>
>>>> gcc/testsuite/ChangeLog:
>>>>
>>>> 	* g++.dg/modules/using-15_a.C: New test.
>>>> 	* g++.dg/modules/using-15_b.C: New test.
>>>> 	* g++.dg/modules/using-15_c.C: New test.
>>>>
>>>> Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
>>>> ---
>>>>    gcc/cp/module.cc                          |  4 ++++
>>>>    gcc/testsuite/g++.dg/modules/using-15_a.C | 13 +++++++++++++
>>>>    gcc/testsuite/g++.dg/modules/using-15_b.C |  5 +++++
>>>>    gcc/testsuite/g++.dg/modules/using-15_c.C |  7 +++++++
>>>>    4 files changed, 29 insertions(+)
>>>>    create mode 100644 gcc/testsuite/g++.dg/modules/using-15_a.C
>>>>    create mode 100644 gcc/testsuite/g++.dg/modules/using-15_b.C
>>>>    create mode 100644 gcc/testsuite/g++.dg/modules/using-15_c.C
>>>>
>>>> diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
>>>> index 9d054c4c792..527c9046c67 100644
>>>> --- a/gcc/cp/module.cc
>>>> +++ b/gcc/cp/module.cc
>>>> @@ -12915,10 +12915,12 @@ depset::hash::add_binding_entity (tree decl, WMB_Flags flags, void *data_)
>>>>    	/* Ignore NTTP objects.  */
>>>>    	return false;
>>>> +      bool unscoped_enum_const_p = false;
>>>>          if (!(flags & WMB_Using) && CP_DECL_CONTEXT (decl) != data->ns)
>>>>    	{
>>>>    	  /* A using that lost its wrapper or an unscoped enum
>>>>    	     constant.  */
>>>> +	  unscoped_enum_const_p = (TREE_CODE (decl) == CONST_DECL);
>>>
>>> How does this interact with C++20 using enum?
>>
>> Looks like it ignores those (so they still suffer from this error).  But
>> in general we don't handle usings of non-functions correctly anyway yet
>> (for the reasons I described in the cover letter); I just added this for
>> now to prevent regressing some test-cases caused by importing enum
>> consts wrapped in an OVERLOAD.
>>
>> Otherwise happy to defer this patch until GCC 15 when I can look at
>> exploring what needs to be done to handle non-function using-decls
>> correctly, but I'll need to work out a new testcase for the followup
>> patch in this series (or just defer that one too, I suppose).
> 
> Ping.  Or should I just scrap this patch for now, find a new testcase
> for the followup patch, and submit it again once we have a general
> solution for using-decls of non-functions?

Please add a FIXME about using enum here and a using enum testcase to 
the appropriate PR; OK for trunk and 14.2 with that change.

Jason

[PATCH 3/3] c++/modules: Propagate hidden flag on decls from partitions

[Nathaniel Shead]

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

-- >8 --

While working on some other fixes I noticed that the partition handling
code used the wrong flag to propagate OVL_HIDDEN_P on exported bindings
from partitions. This patch fixes that, and renames the flag to be
clearer.

gcc/cp/ChangeLog:

	* name-lookup.cc (walk_module_binding): Use the
	partition-specific hidden flag instead of the top-level
	decl_hidden.

gcc/testsuite/ChangeLog:

	* g++.dg/modules/using-16_a.C: New test.
	* g++.dg/modules/using-16_b.C: New test.
	* g++.dg/modules/using-16_c.C: New test.

Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
---
 gcc/cp/name-lookup.cc                     | 10 +++++-----
 gcc/testsuite/g++.dg/modules/using-16_a.C | 11 +++++++++++
 gcc/testsuite/g++.dg/modules/using-16_b.C | 12 ++++++++++++
 gcc/testsuite/g++.dg/modules/using-16_c.C | 11 +++++++++++
 4 files changed, 39 insertions(+), 5 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/modules/using-16_a.C
 create mode 100644 gcc/testsuite/g++.dg/modules/using-16_b.C
 create mode 100644 gcc/testsuite/g++.dg/modules/using-16_c.C

diff --git a/gcc/cp/name-lookup.cc b/gcc/cp/name-lookup.cc
index 7af7f00e34c..b7746938e1b 100644
--- a/gcc/cp/name-lookup.cc
+++ b/gcc/cp/name-lookup.cc
@@ -4274,19 +4274,19 @@ walk_module_binding (tree binding, bitmap partitions,
 
 			count += callback (btype, flags, data);
 		      }
-		    bool hidden = STAT_DECL_HIDDEN_P (bind);
+		    bool part_hidden = STAT_DECL_HIDDEN_P (bind);
 		    for (ovl_iterator iter (MAYBE_STAT_DECL (STAT_DECL (bind)));
 			 iter; ++iter)
 		      {
 			if (iter.hidden_p ())
-			  hidden = true;
+			  part_hidden = true;
 			gcc_checking_assert
-			  (!(hidden && DECL_IS_UNDECLARED_BUILTIN (*iter)));
+			  (!(part_hidden && DECL_IS_UNDECLARED_BUILTIN (*iter)));
 
 			WMB_Flags flags = WMB_None;
 			if (maybe_dups)
 			  flags = WMB_Flags (flags | WMB_Dups);
-			if (decl_hidden)
+			if (part_hidden)
 			  flags = WMB_Flags (flags | WMB_Hidden);
 			if (iter.using_p ())
 			  {
@@ -4295,7 +4295,7 @@ walk_module_binding (tree binding, bitmap partitions,
 			      flags = WMB_Flags (flags | WMB_Export);
 			  }
 			count += callback (*iter, flags, data);
-			hidden = false;
+			part_hidden = false;
 		      }
 		  }
 	      }
diff --git a/gcc/testsuite/g++.dg/modules/using-16_a.C b/gcc/testsuite/g++.dg/modules/using-16_a.C
new file mode 100644
index 00000000000..25d8bca5d1c
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/using-16_a.C
@@ -0,0 +1,11 @@
+// { dg-additional-options "-fmodules-ts" }
+// { dg-module-cmi M:S }
+
+export module M:S;
+
+namespace foo {
+  // propagate hidden from partitions
+  export struct S {
+    friend void f(S);
+  };
+};
diff --git a/gcc/testsuite/g++.dg/modules/using-16_b.C b/gcc/testsuite/g++.dg/modules/using-16_b.C
new file mode 100644
index 00000000000..3f704a913f4
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/using-16_b.C
@@ -0,0 +1,12 @@
+// { dg-additional-options "-fmodules-ts -Wno-global-module" }
+// { dg-module-cmi M }
+
+module;
+namespace bar {
+  void f(int);
+}
+export module M;
+export import :S;
+namespace foo {
+  export using bar::f;
+}
diff --git a/gcc/testsuite/g++.dg/modules/using-16_c.C b/gcc/testsuite/g++.dg/modules/using-16_c.C
new file mode 100644
index 00000000000..5e46cd16013
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/using-16_c.C
@@ -0,0 +1,11 @@
+// { dg-additional-options "-fmodules-ts" }
+import M;
+
+int main() {
+  // this should be hidden and fail
+  foo::f(foo::S{});  // { dg-error "cannot convert" }
+
+  // but these should be legal
+  foo::f(10);
+  f(foo::S{});
+}
-- 
2.43.2

Re: [PATCH 3/3] c++/modules: Propagate hidden flag on decls from partitions

[Jason Merrill]

On 4/11/24 20:41, Nathaniel Shead wrote:
> Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?

OK.

> -- >8 --
> 
> While working on some other fixes I noticed that the partition handling
> code used the wrong flag to propagate OVL_HIDDEN_P on exported bindings
> from partitions. This patch fixes that, and renames the flag to be
> clearer.
> 
> gcc/cp/ChangeLog:
> 
> 	* name-lookup.cc (walk_module_binding): Use the
> 	partition-specific hidden flag instead of the top-level
> 	decl_hidden.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/modules/using-16_a.C: New test.
> 	* g++.dg/modules/using-16_b.C: New test.
> 	* g++.dg/modules/using-16_c.C: New test.
> 
> Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
> ---
>   gcc/cp/name-lookup.cc                     | 10 +++++-----
>   gcc/testsuite/g++.dg/modules/using-16_a.C | 11 +++++++++++
>   gcc/testsuite/g++.dg/modules/using-16_b.C | 12 ++++++++++++
>   gcc/testsuite/g++.dg/modules/using-16_c.C | 11 +++++++++++
>   4 files changed, 39 insertions(+), 5 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/modules/using-16_a.C
>   create mode 100644 gcc/testsuite/g++.dg/modules/using-16_b.C
>   create mode 100644 gcc/testsuite/g++.dg/modules/using-16_c.C
> 
> diff --git a/gcc/cp/name-lookup.cc b/gcc/cp/name-lookup.cc
> index 7af7f00e34c..b7746938e1b 100644
> --- a/gcc/cp/name-lookup.cc
> +++ b/gcc/cp/name-lookup.cc
> @@ -4274,19 +4274,19 @@ walk_module_binding (tree binding, bitmap partitions,
>   
>   			count += callback (btype, flags, data);
>   		      }
> -		    bool hidden = STAT_DECL_HIDDEN_P (bind);
> +		    bool part_hidden = STAT_DECL_HIDDEN_P (bind);
>   		    for (ovl_iterator iter (MAYBE_STAT_DECL (STAT_DECL (bind)));
>   			 iter; ++iter)
>   		      {
>   			if (iter.hidden_p ())
> -			  hidden = true;
> +			  part_hidden = true;
>   			gcc_checking_assert
> -			  (!(hidden && DECL_IS_UNDECLARED_BUILTIN (*iter)));
> +			  (!(part_hidden && DECL_IS_UNDECLARED_BUILTIN (*iter)));
>   
>   			WMB_Flags flags = WMB_None;
>   			if (maybe_dups)
>   			  flags = WMB_Flags (flags | WMB_Dups);
> -			if (decl_hidden)
> +			if (part_hidden)
>   			  flags = WMB_Flags (flags | WMB_Hidden);
>   			if (iter.using_p ())
>   			  {
> @@ -4295,7 +4295,7 @@ walk_module_binding (tree binding, bitmap partitions,
>   			      flags = WMB_Flags (flags | WMB_Export);
>   			  }
>   			count += callback (*iter, flags, data);
> -			hidden = false;
> +			part_hidden = false;
>   		      }
>   		  }
>   	      }
> diff --git a/gcc/testsuite/g++.dg/modules/using-16_a.C b/gcc/testsuite/g++.dg/modules/using-16_a.C
> new file mode 100644
> index 00000000000..25d8bca5d1c
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/using-16_a.C
> @@ -0,0 +1,11 @@
> +// { dg-additional-options "-fmodules-ts" }
> +// { dg-module-cmi M:S }
> +
> +export module M:S;
> +
> +namespace foo {
> +  // propagate hidden from partitions
> +  export struct S {
> +    friend void f(S);
> +  };
> +};
> diff --git a/gcc/testsuite/g++.dg/modules/using-16_b.C b/gcc/testsuite/g++.dg/modules/using-16_b.C
> new file mode 100644
> index 00000000000..3f704a913f4
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/using-16_b.C
> @@ -0,0 +1,12 @@
> +// { dg-additional-options "-fmodules-ts -Wno-global-module" }
> +// { dg-module-cmi M }
> +
> +module;
> +namespace bar {
> +  void f(int);
> +}
> +export module M;
> +export import :S;
> +namespace foo {
> +  export using bar::f;
> +}
> diff --git a/gcc/testsuite/g++.dg/modules/using-16_c.C b/gcc/testsuite/g++.dg/modules/using-16_c.C
> new file mode 100644
> index 00000000000..5e46cd16013
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/using-16_c.C
> @@ -0,0 +1,11 @@
> +// { dg-additional-options "-fmodules-ts" }
> +import M;
> +
> +int main() {
> +  // this should be hidden and fail
> +  foo::f(foo::S{});  // { dg-error "cannot convert" }
> +
> +  // but these should be legal
> +  foo::f(10);
> +  f(foo::S{});
> +}