c++: Check module attachment instead of purview when necessary [PR112631]

Bootstrapped and regtested on x86_64-pc-linux-gnu. I don't have write
access.

-- >8 --

Block-scope declarations of functions or extern values are not allowed
when attached to a named module. Similarly, class member functions are
not inline if attached to a named module. However, in both these cases
we currently only check if the declaration is within the module purview;
it is possible for such a declaration to occur within the module purview
but not be attached to a named module (e.g. in an 'extern "C++"' block).
This patch makes the required adjustments.

	PR c++/112631

gcc/cp/ChangeLog:

	* cp-tree.h (named_module_attach_p): New function.
	* decl.cc (start_decl): Use named_module_attach_p instead of
	named_module_purview_p.
	(grokmethod): Likewise.

gcc/testsuite/ChangeLog:

	* g++.dg/modules/pr112631.C: New test.
---
 gcc/cp/cp-tree.h                        |  2 ++
 gcc/cp/decl.cc                          | 10 +++++-----
 gcc/testsuite/g++.dg/modules/pr112631.C |  8 ++++++++
 3 files changed, 15 insertions(+), 5 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/modules/pr112631.C

On 11/20/23 04:47, Nathaniel Shead wrote:
> Bootstrapped and regtested on x86_64-pc-linux-gnu. I don't have write
> access.
> 
> -- >8 --
> 
> Block-scope declarations of functions or extern values are not allowed
> when attached to a named module. Similarly, class member functions are
> not inline if attached to a named module. However, in both these cases
> we currently only check if the declaration is within the module purview;
> it is possible for such a declaration to occur within the module purview
> but not be attached to a named module (e.g. in an 'extern "C++"' block).
> This patch makes the required adjustments.


Ah I'd been puzzling over the default inlinedness of  member-fns of block-scope 
structs.  Could you augment the testcase to make sure that's right too?

Something like:

// dg-module-do link
export module Mod;

export auto Get () {
   struct X { void Fn () {} };
   return X();
}


///
import Mod
void Frob () { Get().Fn(); }

> 
> 	PR c++/112631
> 
> gcc/cp/ChangeLog:
> 
> 	* cp-tree.h (named_module_attach_p): New function.
> 	* decl.cc (start_decl): Use named_module_attach_p instead of
> 	named_module_purview_p.
> 	(grokmethod): Likewise.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/modules/pr112631.C: New test.
> ---
>   gcc/cp/cp-tree.h                        |  2 ++
>   gcc/cp/decl.cc                          | 10 +++++-----
>   gcc/testsuite/g++.dg/modules/pr112631.C |  8 ++++++++
>   3 files changed, 15 insertions(+), 5 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/modules/pr112631.C
> 
> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> index 7b0b7c6a17e..9a3981cef58 100644
> --- a/gcc/cp/cp-tree.h
> +++ b/gcc/cp/cp-tree.h
> @@ -7315,6 +7315,8 @@ inline bool module_attach_p ()
>   
>   inline bool named_module_purview_p ()
>   { return named_module_p () && module_purview_p (); }
> +inline bool named_module_attach_p ()
> +{ return named_module_p () && module_attach_p (); }
>   
>   /* We're currently exporting declarations.  */
>   inline bool module_exporting_p ()
> diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
> index e6f75d771e0..395f108aec7 100644
> --- a/gcc/cp/decl.cc
> +++ b/gcc/cp/decl.cc
> @@ -5917,10 +5917,10 @@ start_decl (const cp_declarator *declarator,
>       {
>         /* A function-scope decl of some namespace-scope decl.  */
>         DECL_LOCAL_DECL_P (decl) = true;
> -      if (named_module_purview_p ())
> +      if (named_module_attach_p ())
>   	error_at (declarator->id_loc,
> -		  "block-scope extern declaration %q#D not permitted"
> -		  " in module purview", decl);
> +		  "block-scope extern declaration %q#D must not be"
> +		  " attached to a named module", decl);
>       }
>   
>     /* Enter this declaration into the symbol table.  Don't push the plain
> @@ -18513,10 +18513,10 @@ grokmethod (cp_decl_specifier_seq *declspecs,
>     check_template_shadow (fndecl);
>   
>     /* p1779 ABI-Isolation makes inline not a default for in-class
> -     definitions in named module purview.  If the user explicitly
> +     definitions attached to a named module.  If the user explicitly
>        made it inline, grokdeclarator will already have done the right
>        things.  */
> -  if ((!named_module_purview_p ()
> +  if ((!named_module_attach_p ()
>          || flag_module_implicit_inline
>         /* Lambda's operator function remains inline.  */
>          || LAMBDA_TYPE_P (DECL_CONTEXT (fndecl)))
> diff --git a/gcc/testsuite/g++.dg/modules/pr112631.C b/gcc/testsuite/g++.dg/modules/pr112631.C
> new file mode 100644
> index 00000000000..b5e81a1041b
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/pr112631.C
> @@ -0,0 +1,8 @@
> +// { dg-additional-options "-fmodules-ts" }
> +// { dg-module-cmi bla }
> +
> +export module bla;
> +
> +extern "C++" inline void fun() {
> +  void oops();  // { dg-bogus "block-scope extern declaration" }
> +}

On Thu, Nov 23, 2023 at 03:03:37PM -0500, Nathan Sidwell wrote:
> On 11/20/23 04:47, Nathaniel Shead wrote:
> > Bootstrapped and regtested on x86_64-pc-linux-gnu. I don't have write
> > access.
> > 
> > -- >8 --
> > 
> > Block-scope declarations of functions or extern values are not allowed
> > when attached to a named module. Similarly, class member functions are
> > not inline if attached to a named module. However, in both these cases
> > we currently only check if the declaration is within the module purview;
> > it is possible for such a declaration to occur within the module purview
> > but not be attached to a named module (e.g. in an 'extern "C++"' block).
> > This patch makes the required adjustments.
> 
> 
> Ah I'd been puzzling over the default inlinedness of  member-fns of
> block-scope structs.  Could you augment the testcase to make sure that's
> right too?
> 
> Something like:
> 
> // dg-module-do link
> export module Mod;
> 
> export auto Get () {
>   struct X { void Fn () {} };
>   return X();
> }
> 
> 
> ///
> import Mod
> void Frob () { Get().Fn(); }
> 

I gave this a try and it indeed doesn't work correctly; 'Fn' needs to be
marked 'inline' for this to link (whether or not 'Get' itself is
inline). I've tried tracing the code to work out what's going on but
I've been struggling to work out how all the different flags (e.g.
TREE_PUBLIC, TREE_EXTERNAL, TREE_COMDAT, DECL_NOT_REALLY_EXTERN)
interact, which flags we want to be set where, and where the decision of
what function definitions to emit is actually made.

I did find that doing 'mark_used(decl)' on all member functions in
block-scope structs seems to work however, but I wonder if that's maybe
too aggressive or if there's something else we should be doing?