On Thu, Dec 19, 2024 at 10:38:09AM -0500, Jason Merrill wrote:
> 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.
>
Makes sense, thanks.
> Incidentally, I now notice that your patch 1 regresses namespace-7_a.C,
> which works again after patch 4.
>
> Jason
Yup thanks, I realised that I'd made a mistake with specifying the
linkage of namespace aliases initially that I only incidentally fixed in
patch 4; I'll move the relevant hunk to patch 1:
--- a/gcc/cp/name-lookup.cc
+++ b/gcc/cp/name-lookup.cc
@@ -6620,7 +6620,7 @@ do_namespace_alias (tree alias, tree name_space)
DECL_NAMESPACE_ALIAS (alias) = name_space;
DECL_EXTERNAL (alias) = 1;
DECL_CONTEXT (alias) = FROB_CONTEXT (current_scope ());
- TREE_PUBLIC (alias) = TREE_PUBLIC (DECL_CONTEXT (alias));
+ TREE_PUBLIC (alias) = TREE_PUBLIC (CP_DECL_CONTEXT (alias));
alias = pushdecl (alias);
The new patch applies cleanly and modules.exp now passes again; I'll do
a full bootstrap+regtest to make sure but otherwise below is what I
intend to push tomorrow after squashing in your changes, if there are no
other concerns.
Nathaniel
-- >8 --
Currently, the modules streaming code implements some checks for
declarations in the CMI that reference (some kinds of) internal-linkage
entities, and errors if so. This patch expands on that support to
implement the logic for exposures of TU-local entities as defined in
[basic.link] since P1815.
This will cause some code that previously errored in modules to start
compiling; for instance, template specialisations of internal linkage
functions.
However, some code that previously appeared valid will with this patch
no longer compile, notably some kinds of usages of internal linkage
functions included from the GMF. This appears to be related to P2808
and FR-025, however as yet there doesn't appear to be consensus for
changing these rules so I've implemented them as-is.
This patch leaves a couple of things out. In particular, a couple of
the rules for what is a TU-local entity currently seem to me to be
redundant; I've left them as FIXMEs to be handled once I can find
testcases that aren't adequately supported by the other logic here.
Additionally, there are some exceptions for when naming a TU-local
entity is not always an exposure; I've left support for this to a
follow-up patch for easier review, as it has broader implications for
streaming.
TU-local lambdas are also not yet properly implemented, due to other
bugs with regards to LAMBDA_TYPE_EXTRA_SCOPE not being set in all cases
that it probably should be (see also PR c++/116568). We can revisit
this once that issue has been fixed.
Finally, this patch makes a couple of small adjustments to the modules
streaming logic to prune any leftover TU-local deps (that aren't
erroneous exposures). This is required for this patch to ensure that
later stages don't get confused by any leftover TU-local entities
floating around.
gcc/cp/ChangeLog:
* tree.cc (decl_linkage): Treat DECL_SELF_REFERENCE_P like
DECL_IMPLICIT_TYPEDEF_P.
* name-lookup.cc (do_namespace_alias): Fix linkage.
* module.cc (DB_IS_INTERNAL_BIT): Rename to...
(DB_TU_LOCAL_BIT): ...this.
(DB_REFS_INTERNAL_BIT): Rename to...
(DB_EXPOSURE_BIT): ...this.
(depset::hash::is_internal): Rename to...
(depset::hash::is_tu_local): ...this.
(depset::hash::refs_internal): Rename to...
(depset::hash::is_exposure): ...this.
(depset::hash::is_tu_local_entity): New function.
(depset::hash::has_tu_local_tmpl_arg): New function.
(depset::hash::is_tu_local_value): New function.
(depset::hash::make_dependency): Check for TU-local entities.
(depset::hash::add_dependency): Make current an exposure
whenever it references a TU-local entity.
(depset::hash::add_binding_entity): Don't create bindings for
any TU-local entity.
(depset::hash::finalize_dependencies): Rename flags and adjust
diagnostic messages to report exposures of TU-local entities.
(depset::tarjan::connect): Don't include any TU-local depsets.
(depset::hash::connect): Likewise.
gcc/testsuite/ChangeLog:
* g++.dg/modules/block-decl-2.C: Adjust messages.
* g++.dg/modules/internal-1.C: Adjust messages, remove XFAILs.
* g++.dg/modules/linkage-2.C: Adjust messages, remove XFAILS.
* g++.dg/modules/internal-3.C: New test.
* g++.dg/modules/internal-4_a.H: New test.
* g++.dg/modules/internal-4_b.C: New test.
Signed-off-by: Nathaniel Shead <nathanielosh...@gmail.com>
Reviewed-by: Jason Merrill <ja...@redhat.com>
---
gcc/cp/module.cc | 382 +++++++++++++++++---
gcc/cp/name-lookup.cc | 2 +-
gcc/cp/tree.cc | 14 +-
gcc/testsuite/g++.dg/modules/block-decl-2.C | 2 +-
gcc/testsuite/g++.dg/modules/internal-1.C | 15 +-
gcc/testsuite/g++.dg/modules/internal-3.C | 18 +
gcc/testsuite/g++.dg/modules/internal-4_a.H | 4 +
gcc/testsuite/g++.dg/modules/internal-4_b.C | 128 +++++++
gcc/testsuite/g++.dg/modules/linkage-2.C | 5 +-
9 files changed, 493 insertions(+), 77 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/modules/internal-3.C
create mode 100644 gcc/testsuite/g++.dg/modules/internal-4_a.H
create mode 100644 gcc/testsuite/g++.dg/modules/internal-4_b.C
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index f2a4fb16c07..09158b0f0e6 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -2352,10 +2352,8 @@ private:
DB_KIND_BITS = EK_BITS,
DB_DEFN_BIT = DB_KIND_BIT + DB_KIND_BITS,
DB_IS_PENDING_BIT, /* Is a maybe-pending entity. */
- DB_IS_INTERNAL_BIT, /* It is an (erroneous)
- internal-linkage entity. */
- DB_REFS_INTERNAL_BIT, /* Refers to an internal-linkage
- entity. */
+ DB_TU_LOCAL_BIT, /* It is a TU-local entity. */
+ DB_EXPOSURE_BIT, /* Exposes a TU-local entity. */
DB_IMPORTED_BIT, /* An imported entity. */
DB_UNREACHED_BIT, /* A yet-to-be reached entity. */
DB_MAYBE_RECURSIVE_BIT, /* An entity maybe in a recursive cluster. */
@@ -2441,13 +2439,13 @@ public:
&& get_flag_bit<DB_IS_PENDING_BIT> ()));
}
public:
- bool is_internal () const
+ bool is_tu_local () const
{
- return get_flag_bit<DB_IS_INTERNAL_BIT> ();
+ return get_flag_bit<DB_TU_LOCAL_BIT> ();
}
- bool refs_internal () const
+ bool is_exposure () const
{
- return get_flag_bit<DB_REFS_INTERNAL_BIT> ();
+ return get_flag_bit<DB_EXPOSURE_BIT> ();
}
bool is_import () const
{
@@ -2616,6 +2614,11 @@ public:
depset *add_dependency (tree decl, entity_kind);
void add_namespace_context (depset *, tree ns);
+ private:
+ bool has_tu_local_tmpl_arg (tree decl, tree args, bool explain);
+ bool is_tu_local_entity (tree decl, bool explain = false);
+ bool is_tu_local_value (tree decl, tree expr, bool explain = false);
+
private:
static bool add_binding_entity (tree, WMB_Flags, void *);
@@ -13041,6 +13044,248 @@ depset::hash::find_binding (tree ctx, tree name)
return slot ? *slot : NULL;
}
+/* Returns true if DECL is a TU-local entity, as defined by [basic.link].
+ If EXPLAIN is true, emit an informative note about why DECL is TU-local. */
+
+bool
+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.
+ Neither are the built-in declarations of 'int' and such. */
+ if (TREE_CODE (decl) == TYPE_DECL
+ && !DECL_SELF_REFERENCE_P (decl)
+ && !DECL_IMPLICIT_TYPEDEF_P (decl))
+ return false;
+
+ location_t loc = DECL_SOURCE_LOCATION (decl);
+ tree type = TREE_TYPE (decl);
+
+ /* Check specializations first for slightly better explanations. */
+ int use_tpl = -1;
+ tree ti = node_template_info (decl, use_tpl);
+ if (use_tpl > 0 && TREE_CODE (TI_TEMPLATE (ti)) == TEMPLATE_DECL)
+ {
+ /* A specialization of a TU-local template. */
+ tree tmpl = TI_TEMPLATE (ti);
+ if (is_tu_local_entity (tmpl))
+ {
+ if (explain)
+ {
+ inform (loc, "%qD is a specialization of TU-local template %qD",
+ decl, tmpl);
+ is_tu_local_entity (tmpl, /*explain=*/true);
+ }
+ return true;
+ }
+
+ /* A specialization of a template with any TU-local template argument.
*/
+ if (has_tu_local_tmpl_arg (decl, TI_ARGS (ti), explain))
+ return true;
+
+ /* FIXME A specialization of a template whose (possibly instantiated)
+ declaration is an exposure. This should always be covered by the
+ above cases?? */
+ }
+
+ /* A type, function, variable, or template with internal linkage. */
+ linkage_kind kind = decl_linkage (decl);
+ if (kind == lk_internal
+ /* But although weakrefs are marked static, don't consider them
+ to be TU-local. */
+ && !lookup_attribute ("weakref", DECL_ATTRIBUTES (decl)))
+ {
+ if (explain)
+ inform (loc, "%qD declared with internal linkage", decl);
+ return true;
+ }
+
+ /* Does not have a name with linkage and is declared, or introduced by a
+ lambda-expression, within the definition of a TU-local entity. */
+ if (kind == lk_none)
+ {
+ tree ctx = CP_DECL_CONTEXT (decl);
+ if (LAMBDA_TYPE_P (type))
+ if (tree extra = LAMBDA_TYPE_EXTRA_SCOPE (type))
+ ctx = extra;
+
+ if (TREE_CODE (ctx) == NAMESPACE_DECL)
+ {
+ if (!TREE_PUBLIC (ctx))
+ {
+ if (explain)
+ inform (loc, "%qD has no linkage and is declared in an "
+ "anonymous namespace", decl);
+ return true;
+ }
+ }
+ else if (TYPE_P (ctx))
+ {
+ tree ctx_decl = TYPE_MAIN_DECL (ctx);
+ if (is_tu_local_entity (ctx_decl))
+ {
+ if (explain)
+ {
+ inform (loc, "%qD has no linkage and is declared within "
+ "TU-local entity %qT", decl, ctx);
+ is_tu_local_entity (ctx_decl, /*explain=*/true);
+ }
+ return true;
+ }
+ }
+ else if (is_tu_local_entity (ctx))
+ {
+ if (explain)
+ {
+ inform (loc, "%qD has no linkage and is declared within "
+ "TU-local entity %qD", decl, ctx);
+ is_tu_local_entity (ctx, /*explain=*/true);
+ }
+ return true;
+ }
+ }
+
+ /* A type with no name that is defined outside a class-specifier, function
+ body, or initializer; or is introduced by a defining-type-specifier that
+ is used to declare only TU-local entities.
+
+ We consider types with names for linkage purposes as having names, since
+ 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)))
+ {
+ tree main_decl = TYPE_MAIN_DECL (type);
+ if (!DECL_CLASS_SCOPE_P (main_decl)
+ && !decl_function_context (main_decl)
+ /* FIXME: Lambdas defined outside initializers. We'll need to more
+ thoroughly set LAMBDA_TYPE_EXTRA_SCOPE to check this. */
+ && !LAMBDA_TYPE_P (type))
+ {
+ if (explain)
+ inform (loc, "%qT has no name and is not defined within a class, "
+ "function, or initializer", type);
+ return true;
+ }
+
+ // FIXME introduced by a defining-type-specifier only declaring TU-local
+ // entities; does this refer to e.g. 'static struct {} a;"? I can't
+ // think of any cases where this isn't covered by earlier cases. */
+ }
+
+ return false;
+}
+
+/* Helper for is_tu_local_entity. Returns true if one of the ARGS of
+ DECL is TU-local. Emits an explanation if EXPLAIN is true. */
+
+bool
+depset::hash::has_tu_local_tmpl_arg (tree decl, tree args, bool explain)
+{
+ if (!args || TREE_CODE (args) != TREE_VEC)
+ return false;
+
+ for (tree a : tree_vec_range (args))
+ {
+ if (TREE_CODE (a) == TREE_VEC)
+ {
+ if (has_tu_local_tmpl_arg (decl, a, explain))
+ return true;
+ }
+ else if (!WILDCARD_TYPE_P (a))
+ {
+ if (DECL_P (a) && is_tu_local_entity (a))
+ {
+ if (explain)
+ {
+ inform (DECL_SOURCE_LOCATION (decl),
+ "%qD has TU-local template argument %qD",
+ decl, a);
+ is_tu_local_entity (a, /*explain=*/true);
+ }
+ return true;
+ }
+
+ if (TYPE_P (a) && TYPE_NAME (a) && is_tu_local_entity (TYPE_NAME (a)))
+ {
+ if (explain)
+ {
+ inform (DECL_SOURCE_LOCATION (decl),
+ "%qD has TU-local template argument %qT",
+ decl, a);
+ is_tu_local_entity (TYPE_NAME (a), /*explain=*/true);
+ }
+ return true;
+ }
+
+ if (EXPR_P (a) && is_tu_local_value (decl, a, explain))
+ return true;
+ }
+ }
+
+ return false;
+}
+
+/* Returns true if EXPR (part of the initializer for DECL) is a TU-local value
+ or object. Emits an explanation if EXPLAIN is true. */
+
+bool
+depset::hash::is_tu_local_value (tree decl, tree expr, bool explain)
+{
+ if (!expr)
+ return false;
+
+ tree e = expr;
+ STRIP_ANY_LOCATION_WRAPPER (e);
+ STRIP_NOPS (e);
+ if (TREE_CODE (e) == TARGET_EXPR)
+ e = TARGET_EXPR_INITIAL (e);
+ if (!e)
+ return false;
+
+ /* It is, or is a pointer to, a TU-local function or the object associated
+ with a TU-local variable. */
+ tree object = NULL_TREE;
+ if (TREE_CODE (e) == ADDR_EXPR)
+ object = TREE_OPERAND (e, 0);
+ else if (TREE_CODE (e) == PTRMEM_CST)
+ object = PTRMEM_CST_MEMBER (e);
+ else if (VAR_OR_FUNCTION_DECL_P (e))
+ object = e;
+
+ if (object
+ && VAR_OR_FUNCTION_DECL_P (object)
+ && is_tu_local_entity (object))
+ {
+ if (explain)
+ {
+ /* We've lost a lot of location information by the time we get here,
+ so let's just do our best effort. */
+ auto loc = cp_expr_loc_or_loc (expr, DECL_SOURCE_LOCATION (decl));
+ if (VAR_P (object))
+ inform (loc, "%qD refers to TU-local object %qD", decl, object);
+ else
+ inform (loc, "%qD refers to TU-local function %qD", decl, object);
+ is_tu_local_entity (object, true);
+ }
+ return true;
+ }
+
+ /* It is an object of class or array type and any of its subobjects or
+ any of the objects or functions to which its non-static data members
+ 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))
+ if (is_tu_local_value (decl, f.value, explain))
+ return true;
+
+ return false;
+}
+
/* DECL is a newly discovered dependency. Create the depset, if it
doesn't already exist. Add it to the worklist if so.
@@ -13147,6 +13392,7 @@ depset::hash::make_dependency (tree decl, entity_kind
ek)
if (ek != EK_USING)
{
tree not_tmpl = STRIP_TEMPLATE (decl);
+ bool imported_from_module_p = false;
if (DECL_LANG_SPECIFIC (not_tmpl)
&& DECL_MODULE_IMPORT_P (not_tmpl))
@@ -13162,28 +13408,36 @@ depset::hash::make_dependency (tree decl, entity_kind
ek)
dep->cluster = index - from->entity_lwm;
dep->section = from->remap;
dep->set_flag_bit<DB_IMPORTED_BIT> ();
+
+ if (!from->is_header ())
+ imported_from_module_p = true;
}
}
- if (ek == EK_DECL
- && !dep->is_import ()
- && TREE_CODE (CP_DECL_CONTEXT (decl)) == NAMESPACE_DECL
- && !(TREE_CODE (decl) == TEMPLATE_DECL
- && DECL_UNINSTANTIATED_TEMPLATE_FRIEND_P (decl)))
+ /* Check for TU-local entities. This is unnecessary in header
+ units because we can export internal-linkage decls, and
+ no declarations are exposures. Similarly, if the decl was
+ imported from a non-header module we know it cannot have
+ been TU-local. */
+ if (!header_module_p () && !imported_from_module_p)
{
- tree ctx = CP_DECL_CONTEXT (decl);
+ if (is_tu_local_entity (decl))
+ dep->set_flag_bit<DB_TU_LOCAL_BIT> ();
- if (!TREE_PUBLIC (ctx))
- /* Member of internal namespace. */
- dep->set_flag_bit<DB_IS_INTERNAL_BIT> ();
- else if (VAR_OR_FUNCTION_DECL_P (not_tmpl)
- && DECL_THIS_STATIC (not_tmpl))
+ if (VAR_P (decl)
+ && decl_maybe_constant_var_p (decl)
+ && is_tu_local_value (decl, DECL_INITIAL (decl)))
{
- /* An internal decl. This is ok in a GM entity. */
- if (!(header_module_p ()
- || !DECL_LANG_SPECIFIC (not_tmpl)
- || !DECL_MODULE_PURVIEW_P (not_tmpl)))
- dep->set_flag_bit<DB_IS_INTERNAL_BIT> ();
+ /* A potentially-constant variable initialized to a TU-local
+ value is not usable in constant expressions within other
+ translation units. We can achieve this by simply not
+ streaming the definition in such cases. */
+ dep->clear_flag_bit<DB_DEFN_BIT> ();
+
+ if (DECL_DECLARED_CONSTEXPR_P (decl))
+ /* Also, a constexpr variable initialized to a TU-local
+ value is an exposure. */
+ dep->set_flag_bit<DB_EXPOSURE_BIT> ();
}
}
@@ -13222,8 +13476,8 @@ depset::hash::add_dependency (depset *dep)
gcc_checking_assert (current && !is_key_order ());
current->deps.safe_push (dep);
- if (dep->is_internal () && !current->is_internal ())
- current->set_flag_bit<DB_REFS_INTERNAL_BIT> ();
+ if (dep->is_tu_local ())
+ current->set_flag_bit<DB_EXPOSURE_BIT> ();
if (current->get_entity_kind () == EK_USING
&& DECL_IMPLICIT_TYPEDEF_P (dep->get_entity ())
@@ -13342,13 +13596,9 @@ depset::hash::add_binding_entity (tree decl, WMB_Flags
flags, void *data_)
/* Ignore entities not within the module purview. */
return false;
- if (VAR_OR_FUNCTION_DECL_P (inner)
- && DECL_THIS_STATIC (inner))
- {
- if (!header_module_p ())
- /* Ignore internal-linkage entitites. */
- return false;
- }
+ if (!header_module_p () && data->hash->is_tu_local_entity (decl))
+ /* Ignore TU-local entitites. */
+ return false;
if ((TREE_CODE (decl) == VAR_DECL
|| TREE_CODE (decl) == TYPE_DECL)
@@ -14044,10 +14294,8 @@ bool
depset::hash::finalize_dependencies ()
{
bool ok = true;
- depset::hash::iterator end (this->end ());
- for (depset::hash::iterator iter (begin ()); iter != end; ++iter)
+ for (depset *dep : *this)
{
- depset *dep = *iter;
if (dep->is_binding ())
{
/* Keep the containing namespace dep first. */
@@ -14060,23 +14308,41 @@ depset::hash::finalize_dependencies ()
gcc_qsort (&dep->deps[1], dep->deps.length () - 1,
sizeof (dep->deps[1]), binding_cmp);
}
- else if (dep->refs_internal ())
+ else if (dep->is_exposure () && !dep->is_tu_local ())
{
- for (unsigned ix = dep->deps.length (); ix--;)
+ ok = false;
+ bool explained = false;
+ tree decl = dep->get_entity ();
+
+ for (depset *rdep : dep->deps)
+ if (!rdep->is_binding () && rdep->is_tu_local ())
+ {
+ // FIXME:QOI Better location information? We're
+ // losing, so it doesn't matter about efficiency
+ tree exposed = rdep->get_entity ();
+ auto_diagnostic_group d;
+ error_at (DECL_SOURCE_LOCATION (decl),
+ "%qD exposes TU-local entity %qD", decl, exposed);
+ bool informed = is_tu_local_entity (exposed, /*explain=*/true);
+ gcc_checking_assert (informed);
+ explained = true;
+ break;
+ }
+
+ if (!explained && VAR_P (decl) && DECL_DECLARED_CONSTEXPR_P (decl))
{
- depset *rdep = dep->deps[ix];
- if (rdep->is_internal ())
- {
- // FIXME:QOI Better location information? We're
- // losing, so it doesn't matter about efficiency
- tree decl = dep->get_entity ();
- error_at (DECL_SOURCE_LOCATION (decl),
- "%q#D references internal linkage entity %q#D",
- decl, rdep->get_entity ());
- break;
- }
+ auto_diagnostic_group d;
+ error_at (DECL_SOURCE_LOCATION (decl),
+ "%qD is declared %<constexpr%> and is initialized to "
+ "a TU-local value", decl);
+ bool informed = is_tu_local_value (decl, DECL_INITIAL (decl),
+ /*explain=*/true);
+ gcc_checking_assert (informed);
+ explained = true;
}
- ok = false;
+
+ /* We should have emitted an error above. */
+ gcc_checking_assert (explained);
}
}
@@ -14100,7 +14366,9 @@ void
depset::tarjan::connect (depset *v)
{
gcc_checking_assert (v->is_binding ()
- || !(v->is_unreached () || v->is_import ()));
+ || !(v->is_tu_local ()
+ || v->is_unreached ()
+ || v->is_import ()));
v->cluster = v->section = ++index;
stack.safe_push (v);
@@ -14110,7 +14378,8 @@ depset::tarjan::connect (depset *v)
{
depset *dep = v->deps[ix];
- if (dep->is_binding () || !dep->is_import ())
+ if (dep->is_binding ()
+ || !(dep->is_import () || dep->is_tu_local ()))
{
unsigned lwm = dep->cluster;
@@ -14326,14 +14595,12 @@ depset::hash::connect ()
tarjan connector (size ());
vec<depset *> deps;
deps.create (size ());
- iterator end (this->end ());
- for (iterator iter (begin ()); iter != end; ++iter)
+ for (depset *item : *this)
{
- depset *item = *iter;
-
entity_kind kind = item->get_entity_kind ();
if (kind == EK_BINDING
|| !(kind == EK_REDIRECT
+ || item->is_tu_local ()
|| item->is_unreached ()
|| item->is_import ()))
deps.quick_push (item);
@@ -18612,7 +18879,8 @@ module_state::write_begin (elf_out *to, cpp_reader
*reader,
note_defs = note_defs_table_t::create_ggc (1000);
#endif
- /* Determine Strongy Connected Components. */
+ /* Determine Strongly Connected Components. This will also strip any
+ unnecessary dependencies on imported or TU-local entities. */
vec<depset *> sccs = table.connect ();
vec_alloc (ool, modules->length ());
diff --git a/gcc/cp/name-lookup.cc b/gcc/cp/name-lookup.cc
index 0da0441e3a0..35e7dc81377 100644
--- a/gcc/cp/name-lookup.cc
+++ b/gcc/cp/name-lookup.cc
@@ -6620,7 +6620,7 @@ do_namespace_alias (tree alias, tree name_space)
DECL_NAMESPACE_ALIAS (alias) = name_space;
DECL_EXTERNAL (alias) = 1;
DECL_CONTEXT (alias) = FROB_CONTEXT (current_scope ());
- TREE_PUBLIC (alias) = TREE_PUBLIC (DECL_CONTEXT (alias));
+ TREE_PUBLIC (alias) = TREE_PUBLIC (CP_DECL_CONTEXT (alias));
alias = pushdecl (alias);
diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc
index 97ce1f66d2f..f70eb59be35 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))))
+ /* 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)))
+ /* Likewise for the injected-class-name. */
+ || DECL_SELF_REFERENCE_P (decl))
decl = TYPE_MAIN_DECL (TREE_TYPE (decl));
else
return lk_none;
diff --git a/gcc/testsuite/g++.dg/modules/block-decl-2.C
b/gcc/testsuite/g++.dg/modules/block-decl-2.C
index d491a18dfb1..104a98ab052 100644
--- a/gcc/testsuite/g++.dg/modules/block-decl-2.C
+++ b/gcc/testsuite/g++.dg/modules/block-decl-2.C
@@ -11,7 +11,7 @@ export extern "C++" auto foo() {
struct X {
// `foo` is not attached to a named module, and as such
// `X::f` should be implicitly `inline` here
- void f() { // { dg-error "references internal linkage entity" }
+ void f() { // { dg-error "exposes TU-local entity" }
internal();
}
};
diff --git a/gcc/testsuite/g++.dg/modules/internal-1.C
b/gcc/testsuite/g++.dg/modules/internal-1.C
index 9f7299a5fc7..3ed74c9cc42 100644
--- a/gcc/testsuite/g++.dg/modules/internal-1.C
+++ b/gcc/testsuite/g++.dg/modules/internal-1.C
@@ -3,13 +3,10 @@
export module frob;
// { dg-module-cmi !frob }
-namespace {
-// We shouldn't be complaining about members of internal linkage
-// entities
-class X // { dg-bogus "internal linkage" "" { xfail *-*-* } }
-{ // { dg-bogus "internal linkage" "" { xfail *-*-* } }
-};
-
+namespace
+{
+ // We shouldn't be complaining about members of internal linkage entities
+ class X {};
}
static int frob ()
@@ -17,5 +14,5 @@ static int frob ()
return 1;
}
-export int f (int = frob ()); // { dg-error "references internal linkage" }
-int goof (X &); // { dg-error "references internal linkage" }
+export int f (int = frob ()); // { dg-error "exposes TU-local entity" }
+int goof (X &); // { dg-error "exposes TU-local entity" }
diff --git a/gcc/testsuite/g++.dg/modules/internal-3.C
b/gcc/testsuite/g++.dg/modules/internal-3.C
new file mode 100644
index 00000000000..91aae32783f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/internal-3.C
@@ -0,0 +1,18 @@
+// { dg-additional-options "-fmodules-ts -Wno-global-module" }
+// { dg-module-cmi !M }
+// TU-local entities in the GMF can be exposed.
+
+module;
+
+static inline void foo() {}
+
+export module M;
+
+inline void bar() { // { dg-error "exposes TU-local entity" }
+ foo();
+}
+
+// OK
+void qux() {
+ foo();
+}
diff --git a/gcc/testsuite/g++.dg/modules/internal-4_a.H
b/gcc/testsuite/g++.dg/modules/internal-4_a.H
new file mode 100644
index 00000000000..04dfac8c936
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/internal-4_a.H
@@ -0,0 +1,4 @@
+// { dg-additional-options "-fmodule-header" }
+// { dg-module-cmi {} }
+
+static void header_f() {}
diff --git a/gcc/testsuite/g++.dg/modules/internal-4_b.C
b/gcc/testsuite/g++.dg/modules/internal-4_b.C
new file mode 100644
index 00000000000..86bec294fc8
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/internal-4_b.C
@@ -0,0 +1,128 @@
+// { dg-additional-options "-fmodules-ts" }
+// { dg-module-cmi !bad }
+// Test for determining various kinds of entities being marked TU-local
+
+export module bad;
+import "internal-4_a.H";
+
+
+// A type, function variable, or template with internal linkage
+namespace {
+ struct internal_t { int m; };
+ enum internal_e {};
+ void internal_f() {}
+ int internal_v;
+ template <typename T> void internal_x() {}
+ namespace internal_ns {}
+}
+
+inline void expose_type() { // { dg-error "exposes TU-local entity" }
+ internal_t x;
+}
+inline void expose_func() { // { dg-error "exposes TU-local entity" }
+ internal_f();
+}
+inline void expose_var() { // { dg-error "exposes TU-local entity" }
+ int* p = &internal_v;
+}
+inline void expose_tmpl() { // { dg-error "exposes TU-local entity" }
+ internal_x<int>();
+}
+inline void expose_header_decl() { // { dg-error "exposes TU-local entity" }
+ header_f();
+}
+
+// We additionally consider a namespace with internal linkage as TU-local
+namespace expose_ns = internal_ns; // { dg-error "exposes TU-local entity" }
+
+// But we don't consider a weakref as being TU-local, despite being
+// marked static; this is to support uses of weakrefs in header files
+// (such as via the standard library).
+static void weakref() __attribute__((weakref("target")));
+inline void expose_weakref() {
+ weakref();
+}
+
+
+// Does not have a name with linkage and is declared, or introduced by
+// a lambda-expression, within the definition of a TU-local entity
+static auto get_local_ok() {
+ return 0;
+}
+static auto get_local_type() {
+ struct no_linkage {};
+ return no_linkage();
+}
+static auto get_local_lambda() {
+ return []{};
+}
+using T = decltype(get_local_ok()); // OK
+using U = decltype(get_local_type()); // { dg-error "exposes TU-local entity"
}
+using V = decltype(get_local_lambda()); // { dg-error "exposes TU-local
entity" }
+
+static auto internal_lambda = []{ internal_f(); }; // OK
+auto expose_lambda = internal_lambda; // { dg-error "exposes TU-local entity"
}
+
+int not_in_tu_local
+ = ([]{ internal_f(); }(), // { dg-error "exposes TU-local entity" }
+ 0);
+
+
+// A type with no name that is defined outside a class-specifier, function
+// body, or initializer
+
+struct {} no_name; // { dg-error "exposes TU-local entity" }
+enum {} e; // { dg-error "exposes TU-local entity" }
+using not_an_initializer = class {}; // { dg-error "exposes TU-local entity" }
+
+class in_class_specifier { struct {} x; }; // OK
+void in_function_body() { struct {} x; } // OK
+auto in_initializer = []{}; // OK
+
+#if __cplusplus >= 202002L
+decltype([]{}) d_lambda; // { dg-error "exposes TU-local entity" "" { xfail
*-*-* } }
+
+template <typename T>
+concept in_constraint_expression = requires {
+ // Strictly by the standard this is currently ill-formed
+ // (this is a constraint-expression not an initializer)
+ // but I don't think that is intended.
+ []{}; // { dg-bogus "exposes TU-local entity" }
+};
+#endif
+
+// (But consider unnamed types with names for linkage purposes as having names)
+typedef struct {} no_name_typedef_t;
+no_name_typedef_t linkage_name_struct; // OK
+
+enum { enum_name } linkage_name_enum; // OK
+
+
+// Specialisation of a TU-local template
+template <typename T> static void f(T) {}
+template <> void f(int) {} // OK
+inline void f_use(int x) { // { dg-error "exposes TU-local entity" }
+ f(x);
+}
+
+
+// Specialisation of a template with any TU-local argument
+template <typename T> void g(T) {}
+template <> void g(internal_t) { internal_f(); } // OK
+template <> void g(internal_e) { internal_f(); } // OK
+template <> void g(decltype(no_name)) { internal_f(); } // OK
+template <> void g(decltype(get_local_lambda())) { internal_f(); } // OK
+
+template <auto X> struct h {};
+template struct h<&internal_v>;
+template <> struct h<&internal_f> { internal_t x; }; // OK
+template <> struct h<&internal_t::m> { void foo() { internal_f(); } }; // OK
+
+
+// TODO: I can't come up with testcases for these that aren't already covered
+// by one of the above cases:
+//
+// - A type with no name introduced by a defining-type-specifier that is
+// used to declare only TU-local entities
+// - A specialisation of a template whose (possibly instantiated) declaration
+// is an exposure
diff --git a/gcc/testsuite/g++.dg/modules/linkage-2.C
b/gcc/testsuite/g++.dg/modules/linkage-2.C
index 4b20411572c..97421bfad8e 100644
--- a/gcc/testsuite/g++.dg/modules/linkage-2.C
+++ b/gcc/testsuite/g++.dg/modules/linkage-2.C
@@ -25,6 +25,5 @@ export void use() {
// Additionally, unnamed types have no linkage but are also TU-local, and thus
// cannot be exposed in a module interface unit. The non-TU-local entity 's'
-// here is an exposure of this type, so this should be an error; we don't yet
-// implement this checking however.
-struct {} s; // { dg-error "TU-local" "" { xfail *-*-* } }
+// here is an exposure of this type.
+struct {} s; // { dg-error "exposes TU-local entity" }
--
2.47.0
