public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jason Merrill <jason@redhat.com>
To: Nathaniel Shead <nathanieloshead@gmail.com>
Cc: gcc-patches@gcc.gnu.org, Nathan Sidwell <nathan@acm.org>
Subject: Re: [PATCH v3] c++: Fix handling of no-linkage decls for modules
Date: Mon, 18 Mar 2024 20:58:04 -0400	[thread overview]
Message-ID: <faae280c-3a1c-4208-9a7c-715f61ba3bbb@redhat.com> (raw)
In-Reply-To: <65f58152.620a0220.6c9e6.eb75@mx.google.com>

On 3/16/24 07:23, Nathaniel Shead wrote:
> On Mon, Mar 11, 2024 at 02:13:34PM -0400, Jason Merrill wrote:
>> On 3/8/24 18:18, Nathaniel Shead wrote:
>>> On Fri, Mar 08, 2024 at 10:19:52AM -0500, Jason Merrill wrote:
>>>> On 3/7/24 21:55, Nathaniel Shead wrote:
>>>>> On Mon, Nov 27, 2023 at 03:59:39PM +1100, Nathaniel Shead wrote:
>>>>>> 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?
>>>>>
>>>>> I got around to looking at this again, here's an updated version of this
>>>>> patch. Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?
>>>>>
>>>>> (I'm not sure if 'start_preparsed_function' is the right place to be
>>>>> putting this kind of logic or if it should instead be going in
>>>>> 'grokfndecl', e.g. decl.cc:10761 where the rules for making local
>>>>> functions have no linkage are initially determined, but I found this
>>>>> easier to implement: happy to move around though if preferred.)
>>>>>
>>>>> -- >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.
>>>>>
>>>>> While implementing this we discovered that block-scope local functions
>>>>> are not correctly emitted, causing link failures; this patch also
>>>>> corrects some assumptions here and ensures that they are emitted when
>>>>> needed.
>>>>>
>>>>> 	PR c++/112631
>>>>>
>>>>> gcc/cp/ChangeLog:
>>>>>
>>>>> 	* cp-tree.h (named_module_attach_p): New function.
>>>>> 	* decl.cc (start_decl): Check for attachment not purview.
>>>>> 	(grokmethod): Likewise.
>>>>
>>>> These changes are OK; the others I want to consider more.
>>>
>>> Thanks, I can commit this as a separate commit if you prefer?
>>
>> Please.
>>
> 
> Thanks, committed as r14-9501-gead3075406ece9.
> 
>>>>> +export auto n_n() {
>>>>> +  internal();
>>>>> +  struct X { void f() { internal(); } };
>>>>> +  return X{};
>>>>
>>>> Hmm, is this not a prohibited exposure?  Seems like X has no linkage because
>>>> it's at block scope, and the deduced return type names it.
>>>>
>>>> I know we try to support this "voldemort" pattern, but is that actually
>>>> correct?
>>>
>>> I had similar doubts, but this is not an especially uncommon pattern in
>>> the wild either. I also asked some other people for their thoughts and
>>> got told:
>>>
>>>     "no linkage" doesn't mean it's ill-formed to name it in other scopes.
>>>     It means a declaration in another scope cannot correspond to it
>>>
>>> And that the wording in [basic.link] p2.4 is imprecise. (Apparently they
>>> were going to raise a core issue about this too, I think?)
>>>
>>> As for whether it's an exposure, looking at [basic.link] p15, the entity
>>> 'X' doesn't actually appear to be TU-local: it doesn't have a name with
>>> internal linkage (no linkage is different) and is not declared or
>>> introduced within the definition of a TU-local entity (n_n is not
>>> TU-local).
>>
>> Hmm, I think you're right.  And this rule:
>>
>>> -    /* DR 757: A type without linkage shall not be used as the type of a
>>> -       variable or function with linkage, unless
>>> -       o the variable or function has extern "C" linkage (7.5 [dcl.link]), or
>>> -       o the variable or function is not used (3.2 [basic.def.odr]) or is
>>> -       defined in the same translation unit.
>>
>> is no longer part of the standard since C++20; the remnant of this rule is
>> the example in
>>
>> https://eel.is/c++draft/basic#def.odr-11
>>
>>> auto f() {
>>>    struct A {};
>>>    return A{};
>>> }
>>> decltype(f()) g();
>>> auto x = g();
>>
>>> A program containing this translation unit is ill-formed because g is odr-used but not defined, and cannot be defined in any other translation unit because the local class A cannot be named outside this translation unit.
>>
>> But g could be defined in another translation unit if f is inline or in a
>> module interface unit.
>>
>> So, I think no_linkage_check needs to consider module_has_cmi_p as well as
>> vague_linkage_p for relaxed mode.  And in no_linkage_error if
>> no_linkage_check returns null in relaxed mode, reduce the permerror to a
>> pedwarn before C++20, and no diagnostic at all in C++20 and above.
>>
> 
> Thanks for the pointers, I've implemented this below.
> 
>>> +      if (ctx != NULL_TREE && TREE_PUBLIC (ctx) && module_has_cmi_p ())
>>> +	{
>>> +	  /* Ensure that functions in local classes within named modules
>>> +	     have their definitions exported, in case they are (directly
>>> +	     or indirectly) used by an importer.  */
>>> +	  TREE_PUBLIC (decl1) = true;
>>> +	  if (DECL_DECLARED_INLINE_P (decl1))
>>> +	    comdat_linkage (decl1);
>>> +	  else
>>> +	    mark_needed (decl1);
>>> +	}
>>
>> Isn't the inline case handled by the comdat_linkage just above?
>>
>> Jason
>>
> 
> It wasn't, because 'TREE_PUBLIC (decl1)' wasn't yet set. But this means
> that it's ended up a lot cleaner to do this in grokfndecl instead then,
> which (along with the no_linkage_check changes) means I didn't actually
> need to change this code at all.
> 
> Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?

OK.

> -- >8 --
> 
> When testing the changes for PR c++/112631 we discovered that currently
> we don't emit definitions of block-scope function declarations if
> they're not used in the module interface TU, which causes issues if they
> are used by importers.
> 
> This patch fixes the handling of no-linkage declarations for C++20. In
> particular, a type declared in a function with vague linkage or declared
> in a module CMI could potentially be accessible outside its defining TU,
> and as such we can't assume that function declarations using that type
> can never be defined in another TU.
> 
> A complication with handling this is that we're only strictly interested
> in declarations with a module CMI, but when parsing the global module
> fragment we don't yet know whether or not this module will have a CMI
> until we reach the "export module" line (or not). Since this case is
> IFNDR anyway (by [basic.def.odr] p11) we just tentatively assume while
> parsing the GMF that this module will have a CMI; once we see (or don't
> see) an 'export module' declaration we can commit to that knowledge for
> future declarations.
> 
> gcc/cp/ChangeLog:
> 
> 	* cp-tree.h (module_maybe_has_cmi_p): New function.
> 	* decl.cc (grokfndecl): Mark block-scope functions as public if
> 	they could be visible in other TUs.
> 	* decl2.cc (no_linkage_error): Don't error for declarations that
> 	could be defined in other TUs since C++20. Suppress duplicate
> 	errors from 'check_global_declaration'.
> 	* tree.cc (no_linkage_check): In relaxed mode, don't consider
> 	types in a module CMI to have no linkage.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/cpp2a/linkage-1.C: New test.
> 	* g++.dg/modules/block-decl-3.h: New test.
> 	* g++.dg/modules/block-decl-3_a.C: New test.
> 	* g++.dg/modules/block-decl-3_b.C: New test.
> 	* g++.dg/modules/block-decl-3_c.C: New test.
> 	* g++.dg/modules/linkage-1_a.C: New test.
> 	* g++.dg/modules/linkage-1_b.C: New test.
> 	* g++.dg/modules/linkage-1_c.C: New test.
> 	* g++.dg/modules/linkage-2.C: New test.
> 
> Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
> ---
>   gcc/cp/cp-tree.h                              |   6 +
>   gcc/cp/decl.cc                                |  10 +-
>   gcc/cp/decl2.cc                               |  39 ++++-
>   gcc/cp/tree.cc                                |  21 ++-
>   gcc/testsuite/g++.dg/cpp2a/linkage-1.C        |  18 ++
>   gcc/testsuite/g++.dg/modules/block-decl-3.h   |  39 +++++
>   gcc/testsuite/g++.dg/modules/block-decl-3_a.C | 157 ++++++++++++++++++
>   gcc/testsuite/g++.dg/modules/block-decl-3_b.C |   8 +
>   gcc/testsuite/g++.dg/modules/block-decl-3_c.C |  30 ++++
>   gcc/testsuite/g++.dg/modules/linkage-1_a.C    |  15 ++
>   gcc/testsuite/g++.dg/modules/linkage-1_b.C    |   6 +
>   gcc/testsuite/g++.dg/modules/linkage-1_c.C    |   9 +
>   gcc/testsuite/g++.dg/modules/linkage-2.C      |  26 +++
>   13 files changed, 372 insertions(+), 12 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/cpp2a/linkage-1.C
>   create mode 100644 gcc/testsuite/g++.dg/modules/block-decl-3.h
>   create mode 100644 gcc/testsuite/g++.dg/modules/block-decl-3_a.C
>   create mode 100644 gcc/testsuite/g++.dg/modules/block-decl-3_b.C
>   create mode 100644 gcc/testsuite/g++.dg/modules/block-decl-3_c.C
>   create mode 100644 gcc/testsuite/g++.dg/modules/linkage-1_a.C
>   create mode 100644 gcc/testsuite/g++.dg/modules/linkage-1_b.C
>   create mode 100644 gcc/testsuite/g++.dg/modules/linkage-1_c.C
>   create mode 100644 gcc/testsuite/g++.dg/modules/linkage-2.C
> 
> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> index 05913861e06..52d53589e51 100644
> --- a/gcc/cp/cp-tree.h
> +++ b/gcc/cp/cp-tree.h
> @@ -7384,6 +7384,12 @@ inline bool named_module_purview_p ()
>   inline bool named_module_attach_p ()
>   { return named_module_p () && module_attach_p (); }
>   
> +/* We don't know if this TU will have a CMI while parsing the GMF,
> +   so tentatively assume that it might, for the purpose of determining
> +   whether no-linkage decls could be used by an importer.  */
> +inline bool module_maybe_has_cmi_p ()
> +{ return module_has_cmi_p () || (named_module_p () && !module_purview_p ()); }
> +
>   /* We're currently exporting declarations.  */
>   inline bool module_exporting_p ()
>   { return module_kind & MK_EXPORTING; }
> diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
> index 7a97b867199..65ab64885ff 100644
> --- a/gcc/cp/decl.cc
> +++ b/gcc/cp/decl.cc
> @@ -10756,9 +10756,15 @@ grokfndecl (tree ctype,
>   
>     /* Members of anonymous types and local classes have no linkage; make
>        them internal.  If a typedef is made later, this will be changed.  */
> -  if (ctype && (!TREE_PUBLIC (TYPE_MAIN_DECL (ctype))
> -		|| decl_function_context (TYPE_MAIN_DECL (ctype))))
> +  if (ctype && !TREE_PUBLIC (TYPE_MAIN_DECL (ctype)))
>       publicp = 0;
> +  else if (ctype && decl_function_context (TYPE_MAIN_DECL (ctype)))
> +    /* But members of local classes in a module CMI should have their
> +       definitions exported, in case they are (directly or indirectly)
> +       used by an importer.  We don't just use module_has_cmi_p here
> +       because for entities in the GMF we don't yet know whether this
> +       module will have a CMI, so we'll conservatively assume it might.  */
> +    publicp = module_maybe_has_cmi_p ();
>   
>     if (publicp && cxx_dialect == cxx98)
>       {
> diff --git a/gcc/cp/decl2.cc b/gcc/cp/decl2.cc
> index 6c9fd415d40..2562d8aeff6 100644
> --- a/gcc/cp/decl2.cc
> +++ b/gcc/cp/decl2.cc
> @@ -4696,8 +4696,19 @@ no_linkage_error (tree decl)
>         bool d = false;
>         auto_diagnostic_group grp;
>         if (cxx_dialect >= cxx11)
> -	d = permerror (DECL_SOURCE_LOCATION (decl), "%q#D, declared using "
> -		       "unnamed type, is used but never defined", decl);
> +	{
> +	  /* If t is declared in a module CMI, then decl could actually
> +	     be defined in a different TU, so don't warn since C++20.  */
> +	  tree relaxed = no_linkage_check (t, /*relaxed_p=*/true);
> +	  if (relaxed != NULL_TREE)
> +	    d = permerror (DECL_SOURCE_LOCATION (decl),
> +			   "%q#D, declared using an unnamed type, "
> +			   "is used but never defined", decl);
> +	  else if (cxx_dialect < cxx20)
> +	    d = pedwarn (DECL_SOURCE_LOCATION (decl), OPT_Wc__20_extensions,
> +			 "%q#D, declared using an unnamed type, "
> +			 "is used but not defined", decl);
> +	}
>         else if (DECL_EXTERN_C_P (decl))
>   	/* Allow this; it's pretty common in C.  */;
>         else if (VAR_P (decl))
> @@ -4716,13 +4727,31 @@ no_linkage_error (tree decl)
>   	inform (DECL_SOURCE_LOCATION (TYPE_NAME (t)), "%q#D does not refer "
>   		"to the unqualified type, so it is not used for linkage",
>   		TYPE_NAME (t));
> +      /* Suppress warning from check_global_declaration if needed.  */
> +      if (d)
> +	suppress_warning (decl, OPT_Wunused);
>       }
>     else if (cxx_dialect >= cxx11)
>       {
>         if (VAR_P (decl) || !DECL_PURE_VIRTUAL_P (decl))
> -	permerror (DECL_SOURCE_LOCATION (decl),
> -		   "%q#D, declared using local type "
> -		   "%qT, is used but never defined", decl, t);
> +	{
> +	  /* Similarly for local types in a function with vague linkage or
> +	     defined in a module CMI, then decl could actually be defined
> +	     in a different TU, so don't warn since C++20.  */
> +	  bool d = false;
> +	  tree relaxed = no_linkage_check (t, /*relaxed_p=*/true);
> +	  if (relaxed != NULL_TREE)
> +	    d = permerror (DECL_SOURCE_LOCATION (decl),
> +			   "%q#D, declared using local type "
> +			   "%qT, is used but never defined", decl, t);
> +	  else if (cxx_dialect < cxx20)
> +	    d = pedwarn (DECL_SOURCE_LOCATION (decl), OPT_Wc__20_extensions,
> +			 "%q#D, declared using local type "
> +			 "%qT, is used but not defined here", decl, t);
> +	  /* Suppress warning from check_global_declaration if needed.  */
> +	  if (d)
> +	    suppress_warning (decl, OPT_Wunused);
> +	}
>       }
>     else if (VAR_P (decl))
>       warning_at (DECL_SOURCE_LOCATION (decl), 0, "type %qT with no linkage "
> diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc
> index e75be9a4e66..f1a23ffe817 100644
> --- a/gcc/cp/tree.cc
> +++ b/gcc/cp/tree.cc
> @@ -2971,7 +2971,8 @@ verify_stmt_tree (tree t)
>   
>   /* Check if the type T depends on a type with no linkage and if so,
>      return it.  If RELAXED_P then do not consider a class type declared
> -   within a vague-linkage function to have no linkage.  Remember:
> +   within a vague-linkage function or in a module CMI to have no linkage,
> +   since it can still be accessed within a different TU.  Remember:
>      no-linkage is not the same as internal-linkage.  */
>   
>   tree
> @@ -3012,7 +3013,15 @@ no_linkage_check (tree t, bool relaxed_p)
>         /* Only treat unnamed types as having no linkage if they're at
>   	 namespace scope.  This is core issue 966.  */
>         if (TYPE_UNNAMED_P (t) && TYPE_NAMESPACE_SCOPE_P (t))
> -	return t;
> +	{
> +	  if (relaxed_p
> +	      && TREE_PUBLIC (CP_TYPE_CONTEXT (t))
> +	      && module_maybe_has_cmi_p ())
> +	    /* This type could possibly be accessed outside this TU.  */
> +	    return NULL_TREE;
> +	  else
> +	    return t;
> +	}
>   
>         for (r = CP_TYPE_CONTEXT (t); ; )
>   	{
> @@ -3023,10 +3032,12 @@ no_linkage_check (tree t, bool relaxed_p)
>   	    return no_linkage_check (TYPE_CONTEXT (t), relaxed_p);
>   	  else if (TREE_CODE (r) == FUNCTION_DECL)
>   	    {
> -	      if (!relaxed_p || !vague_linkage_p (r))
> -		return t;
> -	      else
> +	      if (relaxed_p
> +		  && (vague_linkage_p (r)
> +		      || (TREE_PUBLIC (r) && module_maybe_has_cmi_p ())))
>   		r = CP_DECL_CONTEXT (r);
> +	      else
> +		return t;
>   	    }
>   	  else
>   	    break;
> diff --git a/gcc/testsuite/g++.dg/cpp2a/linkage-1.C b/gcc/testsuite/g++.dg/cpp2a/linkage-1.C
> new file mode 100644
> index 00000000000..888ed6fa5b5
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp2a/linkage-1.C
> @@ -0,0 +1,18 @@
> +// { dg-do compile { target c++11 } }
> +
> +inline auto f() {
> +  struct A {};
> +  return A{};
> +}
> +decltype(f()) a();  // { dg-error "used but not defined" "" { target c++17_down } }
> +
> +auto g() {
> +  struct A {};
> +  return A{};
> +}
> +decltype(g()) b();  // { dg-error "used but never defined" }
> +
> +int main() {
> +  a();
> +  b();
> +}
> diff --git a/gcc/testsuite/g++.dg/modules/block-decl-3.h b/gcc/testsuite/g++.dg/modules/block-decl-3.h
> new file mode 100644
> index 00000000000..4b155eb0054
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/block-decl-3.h
> @@ -0,0 +1,39 @@
> +// GMF
> +
> +// Non-inline function definitions in headers are a recipe for ODR violations,
> +// but we should probably support that anyway as its not inherently wrong
> +// if only ever included into the GMF of a single module.
> +
> +auto gmf_n_i() {
> +  struct X { void f() {} };
> +  return X{};
> +}
> +
> +inline auto gmf_i_i() {
> +  struct X { void f() {} };
> +  return X{};
> +}
> +
> +auto gmf_n_i_i() {
> +  struct X {
> +    auto f() {
> +      struct Y {
> +	void g() {}
> +      };
> +      return Y{};
> +    }
> +  };
> +  return X{};
> +}
> +
> +inline auto gmf_i_i_i() {
> +  struct X {
> +    auto f() {
> +      struct Y {
> +	void g() {}
> +      };
> +      return Y{};
> +    }
> +  };
> +  return X{};
> +}
> diff --git a/gcc/testsuite/g++.dg/modules/block-decl-3_a.C b/gcc/testsuite/g++.dg/modules/block-decl-3_a.C
> new file mode 100644
> index 00000000000..8cb4dde74d1
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/block-decl-3_a.C
> @@ -0,0 +1,157 @@
> +// { dg-additional-options "-fmodules-ts" }
> +// { dg-module-cmi mod }
> +
> +// Test that we can link various forms of local class functions.
> +// Function names use i=inline, n=non-inline, for each nesting.
> +
> +module;
> +#include "block-decl-3.h"
> +
> +export module mod;
> +
> +namespace {
> +  void internal() {}
> +}
> +
> +// singly-nested
> +
> +export auto n_n() {
> +  internal();
> +  struct X { void f() { internal(); } };
> +  return X{};
> +}
> +
> +export auto n_i() {
> +  internal();
> +  struct X { inline void f() {} };
> +  return X{};
> +}
> +
> +export inline auto i_n() {
> +  // `f` is not inline here, so this is OK
> +  struct X { void f() { internal(); } };
> +  return X{};
> +}
> +
> +export inline auto i_i() {
> +  struct X { inline void f() {} };
> +  return X{};
> +}
> +
> +
> +// doubly nested
> +
> +export auto n_n_n() {
> +  struct X {
> +    auto f() {
> +      struct Y {
> +	void g() { internal(); }
> +      };
> +      return Y{};
> +    }
> +  };
> +  return X{};
> +}
> +
> +export auto n_i_n() {
> +  struct X {
> +    inline auto f() {
> +      struct Y {
> +	void g() { internal(); }
> +      };
> +      return Y{};
> +    }
> +  };
> +  return X{};
> +}
> +
> +export inline auto i_n_i() {
> +  struct X {
> +    auto f() {
> +      struct Y {
> +	inline void g() {}
> +      };
> +      return Y {};
> +    }
> +  };
> +  return X{};
> +}
> +
> +export inline auto i_i_i() {
> +  struct X {
> +    inline auto f() {
> +      struct Y {
> +	inline void g() {}
> +      };
> +      return Y{};
> +    }
> +  };
> +  return X{};
> +}
> +
> +
> +// multiple types
> +
> +export auto multi_n_n() {
> +  struct X {
> +    void f() { internal(); }
> +  };
> +  struct Y {
> +    X x;
> +  };
> +  return Y {};
> +}
> +
> +export auto multi_n_i() {
> +  struct X {
> +    inline void f() {}
> +  };
> +  struct Y {
> +    X x;
> +  };
> +  return Y {};
> +}
> +
> +export inline auto multi_i_i() {
> +  struct X {
> +    inline void f() {}
> +  };
> +  struct Y {
> +    X x;
> +  };
> +  return Y {};
> +};
> +
> +
> +// extern "C++"
> +
> +export extern "C++" auto extern_n_i() {
> +  struct X {
> +    void f() {}  // implicitly inline
> +  };
> +  return X{};
> +};
> +
> +export extern "C++" inline auto extern_i_i() {
> +  struct X {
> +    void f() {}
> +  };
> +  return X{};
> +};
> +
> +
> +// GMF
> +
> +export using ::gmf_n_i;
> +export using ::gmf_i_i;
> +export using ::gmf_n_i_i;
> +export using ::gmf_i_i_i;
> +
> +
> +// can access from implementation unit
> +
> +auto only_used_in_impl() {
> +  struct X { void f() {} };
> +  return X{};
> +}
> +export void test_from_impl_unit();
> diff --git a/gcc/testsuite/g++.dg/modules/block-decl-3_b.C b/gcc/testsuite/g++.dg/modules/block-decl-3_b.C
> new file mode 100644
> index 00000000000..bc9b2a213f0
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/block-decl-3_b.C
> @@ -0,0 +1,8 @@
> +// { dg-additional-options "-fmodules-ts" }
> +
> +module mod;
> +
> +// Test that we can access (and link) to declarations from the interface
> +void test_from_impl_unit() {
> +  only_used_in_impl().f();
> +}
> diff --git a/gcc/testsuite/g++.dg/modules/block-decl-3_c.C b/gcc/testsuite/g++.dg/modules/block-decl-3_c.C
> new file mode 100644
> index 00000000000..5b39e038327
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/block-decl-3_c.C
> @@ -0,0 +1,30 @@
> +// { dg-module-do link }
> +// { dg-additional-options "-fmodules-ts" }
> +
> +import mod;
> +
> +int main() {
> +  n_n().f();
> +  n_i().f();
> +  i_n().f();
> +  i_i().f();
> +
> +  n_n_n().f().g();
> +  n_i_n().f().g();
> +  i_n_i().f().g();
> +  i_i_i().f().g();
> +
> +  multi_n_n().x.f();
> +  multi_n_i().x.f();
> +  multi_i_i().x.f();
> +
> +  extern_n_i().f();
> +  extern_i_i().f();
> +
> +  gmf_n_i().f();
> +  gmf_i_i().f();
> +  gmf_n_i_i().f().g();
> +  gmf_i_i_i().f().g();
> +
> +  test_from_impl_unit();
> +}
> diff --git a/gcc/testsuite/g++.dg/modules/linkage-1_a.C b/gcc/testsuite/g++.dg/modules/linkage-1_a.C
> new file mode 100644
> index 00000000000..750e31ff347
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/linkage-1_a.C
> @@ -0,0 +1,15 @@
> +// { dg-additional-options "-fmodules-ts -Wno-error=c++20-extensions" }
> +// { dg-module-cmi M }
> +
> +export module M;
> +
> +auto f() {
> +  struct A {};
> +  return A{};
> +}
> +decltype(f()) g();  // { dg-warning "used but not defined" "" { target c++17_down } }
> +export auto x = g();
> +
> +struct {} s;
> +decltype(s) h();  // { dg-warning "used but not defined" "" { target c++17_down } }
> +export auto y = h();
> diff --git a/gcc/testsuite/g++.dg/modules/linkage-1_b.C b/gcc/testsuite/g++.dg/modules/linkage-1_b.C
> new file mode 100644
> index 00000000000..f23962d76b7
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/linkage-1_b.C
> @@ -0,0 +1,6 @@
> +// { dg-additional-options "-fmodules-ts" }
> +
> +module M;
> +
> +decltype(f()) g() { return {}; }
> +decltype(s) h() { return {}; }
> diff --git a/gcc/testsuite/g++.dg/modules/linkage-1_c.C b/gcc/testsuite/g++.dg/modules/linkage-1_c.C
> new file mode 100644
> index 00000000000..f1406b99032
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/linkage-1_c.C
> @@ -0,0 +1,9 @@
> +// { dg-module-do link }
> +// { dg-additional-options "-fmodules-ts" }
> +
> +import M;
> +
> +int main() {
> +  auto a = x;
> +  auto b = y;
> +}
> diff --git a/gcc/testsuite/g++.dg/modules/linkage-2.C b/gcc/testsuite/g++.dg/modules/linkage-2.C
> new file mode 100644
> index 00000000000..eb4d7b051af
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/linkage-2.C
> @@ -0,0 +1,26 @@
> +// { dg-additional-options "-fmodules-ts" }
> +// { dg-module-cmi !M }
> +
> +export module M;
> +
> +// Same as a linkage-1 except within an anonymous namespace;
> +// now these declarations cannot possibly be defined outside this TU,
> +// so we should error.
> +
> +namespace {
> +  auto f() {
> +    struct A {};
> +    return A{};
> +  }
> +  decltype(f()) g();  // { dg-error "used but never defined" }
> +
> +  struct {} s;
> +  decltype(s) h();  // { dg-error "used but never defined" }
> +}
> +
> +export void use() {
> +  g();
> +  h();
> +}
> +
> +// { dg-prune-output "not writing module" }


      reply	other threads:[~2024-03-19  0:58 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-20  9:47 [PATCH] c++: Check module attachment instead of purview when necessary [PR112631] Nathaniel Shead
2023-11-23 20:03 ` Nathan Sidwell
2023-11-27  4:59   ` Nathaniel Shead
2024-03-08  2:55     ` [PATCH v2] c++: Check module attachment instead of just " Nathaniel Shead
2024-03-08 15:19       ` Jason Merrill
2024-03-08 23:18         ` Nathaniel Shead
2024-03-11 18:13           ` Jason Merrill
2024-03-16 11:23             ` [PATCH v3] c++: Fix handling of no-linkage decls for modules Nathaniel Shead
2024-03-19  0:58               ` Jason Merrill [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=faae280c-3a1c-4208-9a7c-715f61ba3bbb@redhat.com \
    --to=jason@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=nathan@acm.org \
    --cc=nathanieloshead@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).