On Tue, Dec 17, 2024 at 03:58:38PM -0500, Jason Merrill wrote:
> On 11/27/24 3:53 AM, Nathaniel Shead wrote:
> > Gentle ping for this series:
> > https://gcc.gnu.org/pipermail/gcc-patches/2024-October/665108.html
> > 
> > Most of the patches no longer applied cleanly to trunk since the last
> > time I pinged this so I'm attaching newly rebased patches.
> > 
> > One slight adjustment I've included as well is a test in internal-4_b.C
> > for exposures of namespace aliases, as in:
> > 
> >    namespace { namespace internal {} }
> >    export namespace exposure = internal;
> > 
> > By the standard this appears to be well-formed; we currently error, and
> > I think this might be the desired behaviour (an easy workaround is to
> > wrap the alias in an anonymous namespace), but thought I'd at least test
> > the existing behaviour here.
> > 
> > Tested modules.exp on x86_64-pc-linux-gnu, OK for trunk if full
> > bootstrap+regtest succeeds?
> 
> I tweaked some of these patches a bit; OK with these changes, or without
> patch #2 if you'd rather not make that change.

Thanks.  I will include patch #2 (with an additional line in invoke.texi
to note that the presence of explicit instantiations will silence the
warning.)

> From 03134a00bdc0f53cb30fde284808be71811e29e7 Mon Sep 17 00:00:00 2001
> From: Jason Merrill <ja...@redhat.com>
> Date: Wed, 11 Dec 2024 11:02:34 -0500
> Subject: [PATCH] c++: adjust "Detect exposures of TU-local entities"
> To: gcc-patches@gcc.gnu.org
> 
> This patch adjusts the handling of the injected-class-name to be the same as
> the class name itself.
> 
> gcc/cp/ChangeLog:
> 
>       * tree.cc (decl_linkage): Treat DECL_SELF_REFERENCE_P like
>       DECL_IMPLICIT_TYPEDEF_P.
>       * module.cc (depset::hash::is_tu_local_entity): Likewise.
>       (depset::hash::is_tu_local_value): Fix formatting.
> ---
>  gcc/cp/module.cc | 10 +++++-----
>  gcc/cp/tree.cc   | 16 ++++++++--------
>  2 files changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> index 5d11e85f41d..823884e97f3 100644
> --- a/gcc/cp/module.cc
> +++ b/gcc/cp/module.cc
> @@ -13247,10 +13247,11 @@ depset::hash::is_tu_local_entity (tree decl, bool 
> explain/*=false*/)
>  {
>    gcc_checking_assert (DECL_P (decl));
>  
> -  /* An explicit type alias is not an entity, and so is never TU-local.  */
> +  /* An explicit type alias is not an entity, and so is never TU-local.
> +     Neither are the built-in declarations of 'int' and such.  */
>    if (TREE_CODE (decl) == TYPE_DECL
> -      && !DECL_IMPLICIT_TYPEDEF_P (decl)
> -      && !DECL_SELF_REFERENCE_P (decl))
> +      && (is_typedef_decl (decl)
> +       || !OVERLOAD_TYPE_P (TREE_TYPE (decl))))
>      return false;
>  
>    location_t loc = DECL_SOURCE_LOCATION (decl);
> @@ -13348,7 +13349,6 @@ depset::hash::is_tu_local_entity (tree decl, bool 
> explain/*=false*/)
>       these aren't really TU-local.  */
>    if (TREE_CODE (decl) == TYPE_DECL
>        && TYPE_ANON_P (type)
> -      && !DECL_SELF_REFERENCE_P (decl)
>        /* An enum with an enumerator name for linkage.  */
>        && !(UNSCOPED_ENUM_P (type) && TYPE_VALUES (type)))
>      {
> @@ -13473,7 +13473,7 @@ depset::hash::is_tu_local_value (tree decl, tree 
> expr, bool explain)
>       of reference type refer is TU-local and is usable in constant
>       expressions.  */
>    if (TREE_CODE (e) == CONSTRUCTOR && AGGREGATE_TYPE_P (TREE_TYPE (e)))
> -    for (auto& f : CONSTRUCTOR_ELTS (e))
> +    for (auto &f : CONSTRUCTOR_ELTS (e))
>        if (is_tu_local_value (decl, f.value, explain))
>       return true;
>  
> diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc
> index 939d2b060fb..260c16418a1 100644
> --- a/gcc/cp/tree.cc
> +++ b/gcc/cp/tree.cc
> @@ -5894,17 +5894,17 @@ decl_linkage (tree decl)
>       linkage first, and then transform that into a concrete
>       implementation.  */
>  
> -  /* An explicit type alias has no linkage.  */
> +  /* An explicit type alias has no linkage.  Nor do the built-in declarations
> +     of 'int' and such.  */
>    if (TREE_CODE (decl) == TYPE_DECL
> -      && !DECL_IMPLICIT_TYPEDEF_P (decl)
> -      && !DECL_SELF_REFERENCE_P (decl))
> +      && !DECL_IMPLICIT_TYPEDEF_P (decl))
>      {
> -      /* But this could be a typedef name for linkage purposes, in which
> -      case we're interested in the linkage of the main decl.  */
> -      if (decl == TYPE_NAME (TYPE_MAIN_VARIANT (TREE_TYPE (decl))))
> -     decl = TYPE_MAIN_DECL (TREE_TYPE (decl));
> -      else
> +      if (is_typedef_decl (decl)
> +       || !OVERLOAD_TYPE_P (TREE_TYPE (decl)))
>       return lk_none;
> +      /* But this could be a typedef name for linkage purposes or injected
> +      class name; look to the implicit typedef for linkage.  */
> +      decl = TYPE_MAIN_DECL (TREE_TYPE (decl));
>      }
>  
>    /* Namespace-scope entities with no name usually have no linkage.  */
> -- 
> 2.47.1
> 

Unfortunately, however, this patch regresses internal-1.C and
internal-4_b.C when applied to patch #1, though they test cleanly when
testing the whole series.

For a simple example, consider

  export module M;
  namespace { struct X {}; }
  void foo(X);

With this change, just patch #1 gives the following errors:

  test.cpp:3:6: error: ‘void foo({anonymous}::X)’ exposes TU-local entity 
‘struct {anonymous}::X’
      3 | void foo(X);
        |      ^~~
  test.cpp:2:20: note: ‘struct {anonymous}::X’ declared with internal linkage
      2 | namespace { struct X {}; }
        |                    ^
  test.cpp:2:22: error: ‘{anonymous}::X::X’ exposes TU-local entity ‘struct 
{anonymous}::X’
      2 | namespace { struct X {}; }
        |                      ^
  test.cpp:2:20: note: ‘struct {anonymous}::X’ declared with internal linkage
      2 | namespace { struct X {}; }
        |                    ^
  test.cpp:2:20: error: ‘struct {anonymous}::X::__as_base ’ exposes TU-local 
entity ‘struct {anonymous}::X’
  test.cpp:2:20: note: ‘struct {anonymous}::X’ declared with internal linkage

It seems that the self-reference and `__as_base` types no longer get
internal linkage, and they are not considered as TU-local (causing the
errors).  I think this is, strictly speaking, correct however?

Patch #2 'fixes' this, because the patch adds new logic to no longer
continue to explore members within TU-local entities (which means that
these typedefs are never checked to see if they cause exposures).  So
would it be appropriate to maybe just XFAIL the testcase in patch #1 and
remove the XFAILs in #2?

Nathaniel

Reply via email to