On 12/18/24 9:17 AM, Nathaniel Shead wrote:
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?
Oops, thanks! the intent of my change was that they would be considered
TU-local, as they are also names of the class. But apparently I was
missing that is_typedef_decl is true for the injected-class-name, so the
effect was roughly backwards from my intent.
So here's a revision of that patch to do less. I think the only
significant change left is removing one DECL_SELF_REFERENCE_P from
is_tu_local_entity; the decl_linkage change should have no effect since
previously it would have hit the "things in class scope" case.
Incidentally, I now notice that your patch 1 regresses namespace-7_a.C,
which works again after patch 4.
Jason
From 5019869d39806aeab15861e8331bef0ff5b965a8 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 | 6 +++---
gcc/cp/tree.cc | 10 ++++++----
2 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index f834748a579..14e5a910df4 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -13052,7 +13052,8 @@ 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))
@@ -13153,7 +13154,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)))
{
@@ -13278,7 +13278,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 97ce1f66d2f..1eef600c79a 100644
--- a/gcc/cp/tree.cc
+++ b/gcc/cp/tree.cc
@@ -5935,14 +5935,16 @@ 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))))
+ if (decl == TYPE_NAME (TYPE_MAIN_VARIANT (TREE_TYPE (decl)))
+ /* Likewise for the injected-class-name. */
+ || DECL_SELF_REFERENCE_P (decl))
decl = TYPE_MAIN_DECL (TREE_TYPE (decl));
else
return lk_none;
--
2.47.1