On Tue, Jul 09, 2024 at 05:43:59PM -0400, Jason Merrill wrote:
> On 7/9/24 9:44 AM, Nathaniel Shead wrote:
> > On Mon, Jul 08, 2024 at 12:26:41PM -0400, Jason Merrill wrote:
> > > For a using-decl in the same scope as the original decl, won't this
> > > replace
> > > it so only the using-decl is visible to lookup? I had expected to omit
> > > the
> > > USING_DECL in that case.
> >
> > Yup it will; I think I'd originally done that so that more recent
> > (re-)declaration would be the one referred to by diagnostics, but on
> > retrospect that seems unhelpful; fixed. (Though need to keep the
> > replacement for CONST_DECLs, because the modules handling otherwise only
> > handles them in the context of their containing enumeration type, which
> > isn't what we want here; I've added a new test for this as well.)
>
> Ah, using-25, sure. I would think we could still tell the difference by
> comparing PURVIEW/EXPORT on the CONST_DECL to those of its type?
>
> Or perhaps have add_binding_entity skip implicitly inserted enumerators, and
> instead insert them again when reading the enum, which should also save a
> bit of space.
>
> Jason
>
So maybe something like the following?
Bootstrapped and regtested on x86_64-pc-linux-gnu, can be applied either
incrementally on the previous patch or separately as you prefer.
-- >8 --
Subject: [PATCH] c++/modules: Avoid unnecessary wrapping for CONST_DECLs
Enumerators are only written when writing the type definition (at which
point they are all written); this will happen regardless of scoped vs
unscoped or whether the enum is explicitly exported. All other cases
where an enumerator needs to be written (e.g. template parameters) they
are just a backreference to the type decl and the name of the value.
'add_binding_entity' needs to explicitly write the names of unscoped
enumerators so that lazy loading will trigger when the name is found by
name lookup; it does this by pretending that the enum declarations are
always usings so that it doesn't double-write definitions. By also
checking if the enumerator was marked purview/exported we can use that
to override a non-purview/non-exported TYPE_DECL and ensure it's made
visible regardless.
When reading we should get the exported flag on the enumeration
constant, and so should properly create a binding for it. We don't need
to do anything to handle importedness as that checking is skipped for
EK_USINGs.
Some other places assume that module information for a CONST_DECL
inherits module information from its containing type. This includes:
- get_originating_module_decl, for determining if the name was imported
or has module attachment; I don't /think/ this change should affect
that, so I'm leaving this untouched.
- binding_cmp, for sorting by exportedness; since now an enumerator
could be exported without the containing decl being exported, we need
to handle this here too.
With all this in mind, we can avoid creating a new USING_DECL for a
same-scope using that reveals a CONST_DECL by ensuring that we
special-case CONST_DECLs with purview/exported flags appropriately.
gcc/cp/ChangeLog:
* module.cc (depset::hash::add_binding_entity): Handle
CONST_DECLs with different purview/exported from their enum.
(binding_cmp): Likewise.
(set_instantiating_module): Support CONST_DECLs.
* name-lookup.cc (do_nonmember_using_decl): Don't special-case
CONST_DECLs.
Signed-off-by: Nathaniel Shead <nathanielosh...@gmail.com>
---
gcc/cp/module.cc | 19 +++++++++++++++++--
gcc/cp/name-lookup.cc | 6 ++----
2 files changed, 19 insertions(+), 6 deletions(-)
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index 7187d251d1d..d385b422168 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -13129,7 +13129,10 @@ depset::hash::add_binding_entity (tree decl, WMB_Flags
flags, void *data_)
tree inner = decl;
if (TREE_CODE (inner) == CONST_DECL
- && TREE_CODE (DECL_CONTEXT (inner)) == ENUMERAL_TYPE)
+ && TREE_CODE (DECL_CONTEXT (inner)) == ENUMERAL_TYPE
+ /* A using-decl could make a CONST_DECL purview for a non-purview
+ enumeration. */
+ && (!DECL_LANG_SPECIFIC (inner) || !DECL_MODULE_PURVIEW_P (inner)))
inner = TYPE_NAME (DECL_CONTEXT (inner));
else if (TREE_CODE (inner) == TEMPLATE_DECL)
inner = DECL_TEMPLATE_RESULT (inner);
@@ -13164,7 +13167,10 @@ depset::hash::add_binding_entity (tree decl, WMB_Flags
flags, void *data_)
gcc_checking_assert (TREE_CODE (decl) == CONST_DECL);
flags = WMB_Flags (flags | WMB_Using);
- if (DECL_MODULE_EXPORT_P (TYPE_NAME (TREE_TYPE (decl))))
+ if (DECL_MODULE_EXPORT_P (TYPE_NAME (TREE_TYPE (decl)))
+ /* A using-decl can make an enum constant exported for a
+ non-exported enumeration. */
+ || (DECL_LANG_SPECIFIC (decl) && DECL_MODULE_EXPORT_P (decl)))
flags = WMB_Flags (flags | WMB_Export);
}
@@ -13736,6 +13742,10 @@ binding_cmp (const void *a_, const void *b_)
a_export = OVL_EXPORT_P (a_ent);
a_ent = OVL_FUNCTION (a_ent);
}
+ else if (TREE_CODE (a_ent) == CONST_DECL
+ && DECL_LANG_SPECIFIC (a_ent)
+ && DECL_MODULE_EXPORT_P (a_ent))
+ a_export = true;
else
a_export = DECL_MODULE_EXPORT_P (TREE_CODE (a_ent) == CONST_DECL
? TYPE_NAME (TREE_TYPE (a_ent))
@@ -13748,6 +13758,10 @@ binding_cmp (const void *a_, const void *b_)
b_export = OVL_EXPORT_P (b_ent);
b_ent = OVL_FUNCTION (b_ent);
}
+ else if (TREE_CODE (b_ent) == CONST_DECL
+ && DECL_LANG_SPECIFIC (b_ent)
+ && DECL_MODULE_EXPORT_P (b_ent))
+ b_export = true;
else
b_export = DECL_MODULE_EXPORT_P (TREE_CODE (b_ent) == CONST_DECL
? TYPE_NAME (TREE_TYPE (b_ent))
@@ -19230,6 +19244,7 @@ set_instantiating_module (tree decl)
|| TREE_CODE (decl) == TYPE_DECL
|| TREE_CODE (decl) == CONCEPT_DECL
|| TREE_CODE (decl) == TEMPLATE_DECL
+ || TREE_CODE (decl) == CONST_DECL
|| (TREE_CODE (decl) == NAMESPACE_DECL
&& DECL_NAMESPACE_ALIAS (decl)));
diff --git a/gcc/cp/name-lookup.cc b/gcc/cp/name-lookup.cc
index 5b1f3bfd510..e5f8562105e 100644
--- a/gcc/cp/name-lookup.cc
+++ b/gcc/cp/name-lookup.cc
@@ -5345,10 +5345,8 @@ do_nonmember_using_decl (name_lookup &lookup, bool
fn_scope_p,
location for the using-decl that we can use in diagnostics.
But this is unnecessary if we're just redeclaring the same decl;
- in that case we can just mark it purview or exported directly.
- Except for using-decls of enumerators, which are currently
- handled specially in modules code and so need to be wrapped. */
- if (value != lookup.value || TREE_CODE (value) == CONST_DECL)
+ in that case we can just mark it purview or exported directly. */
+ if (value != lookup.value)
{
value = build_lang_decl (USING_DECL, lookup.name, NULL_TREE);
USING_DECL_DECLS (value) = lookup.value;
--
2.43.2
