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
