This finishes the reworking of ADL handling for modules for PR117658.
[basic.link] p18 says that we should diagnose any references to a
TU-local entity from a different TU; with our fixed handling of ADL this
is now possible to occur.
To do this we need to generate fake bindings for these decls on
stream-out so that importers know that we have a visible name in this
namespace. We don't actually need (or want) to provide full DECLs
though, as that could potentially involve streaming other TU-local
entities, so we just build a TU_LOCAL_ENTITY for the decl on receipt.
The patch also uses 'decl_as_string' to give a more descriptive name for
these decls when erroring.
We also need somewhere to store these decls. We only actually need the
decls for diagnostics; for correctness we only need to know whether any
such decls existed, so to not mess with the existing packing of bindings
or usages of the OVERLOADs this patch adds a new map to the binding
vector that can be looked up when diagnostics need to be generated.
Finally, as specified this diagnostic is a pretty broad hammer, as any
internal-linkage purview function will stop ADL in exported templates
from working with that name. So this patch just makes it a pedwarn and
provides an option to disable if needed.
PR c++/117658
gcc/c-family/ChangeLog:
* c.opt: New flag '-Wexternal-tu-local'.
* c.opt.urls: Regenerate.
gcc/cp/ChangeLog:
* cp-tree.h (TU_LOCAL_ENTITY_NAME): Clarify meaning.
* module.cc (depset::entity_kind): New enumerator, assert that
we have enough bits reserved.
(depset::disc_bits): Assert the discriminator has enough bits.
(depset::entity_kind_name): Add 'tu-local' case, assert we
have an entry for all relevant entry_kinds.
(name_for_tu_local_decl): New function.
(trees_out::tree_node): Use it.
(depset::hash::make_dependency): Validate EK_TU_LOCAL.
(depset::hash::add_binding_entity): Generate bindings for
internal purview functions.
(enum ct_bind_flags): New enum for TU-local decls.
(depset::hash::find_dependencies): Handle EK_TU_LOCAL entities.
(binding_cmp): Likewise.
(sort_cluster): Likewise.
(module_state::write_cluster): Likewise.
(module_state::read_cluster): Likewise.
* name-lookup.cc (append_imported_binding_slot): Propagate
internal decl list when growing binding vector.
(name_lookup::adl_namespace_fns): Diagnose if naming a TU-local
entity from a different TU.
(set_module_binding): Include any internal decls in binding.
* name-lookup.h (struct module_tree_map_traits): New type.
(struct tree_binding_vec): Add member 'internal_decls'.
(BINDING_VECTOR_INTERNAL_DECLS): New getter.
(MODULE_BINDING_INTERNAL_DECLS_P): New flag.
(set_module_binding): Add parameter.
gcc/ChangeLog:
* doc/invoke.texi: Document '-Wno-external-tu-local'.
gcc/testsuite/ChangeLog:
* g++.dg/modules/adl-6_c.C: Adjust diagnostics.
* g++.dg/modules/internal-14_c.C: Likewise.
* g++.dg/modules/internal-15_a.C: New test.
* g++.dg/modules/internal-15_b.C: New test.
Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
Reviewed-by: Jason Merrill <jason@redhat.com>
C ObjC C++ ObjC++ CPP(warn_expansion_to_defined) CppReason(CPP_W_EXPANSION_TO_DEFINED) Var(cpp_warn_expansion_to_defined) Init(0) Warning EnabledBy(Wextra || Wpedantic)
Warn if \"defined\" is used outside #if.
+Wexternal-tu-local
+C++ ObjC++ Var(warn_tu_local) Warning Init(1)
+Warn about naming a TU-local entity declared in another translation unit.
+
Wextra
C ObjC C++ ObjC++ Warning
; in common.opt
Wexpansion-to-defined
UrlSuffix(gcc/Warning-Options.html#index-Wexpansion-to-defined)
+Wexternal-tu-local
+UrlSuffix(gcc/C_002b_002b-Dialect-Options.html#index-Wexternal-tu-local)
+
Wextra
UrlSuffix(gcc/Warning-Options.html#index-Wextra) LangUrlSuffix_D(gdc/Warnings.html#index-Wextra) LangUrlSuffix_Fortran(gfortran/Error-and-Warning-Options.html#index-Wextra)
location_t loc;
};
-/* The name of a translation-unit-local entity. */
+/* The human-readable name of a translation-unit-local entity as
+ an IDENTIFIER_NODE. */
#define TU_LOCAL_ENTITY_NAME(NODE) \
(((struct tree_tu_local_entity *)TU_LOCAL_ENTITY_CHECK (NODE))->name)
EK_PARTIAL, /* A partial specialization. */
EK_USING, /* A using declaration (at namespace scope). */
EK_NAMESPACE, /* A namespace. */
+ EK_TU_LOCAL, /* A TU-local decl for ADL. */
EK_REDIRECT, /* Redirect to a template_decl. */
EK_EXPLICIT_HWM,
EK_BINDING = EK_EXPLICIT_HWM, /* Implicitly encoded. */
EK_BITS = 3 /* Only need to encode below EK_EXPLICIT_HWM. */
};
+ static_assert (EK_EXPLICIT_HWM < (1u << EK_BITS),
+ "not enough bits reserved for entity_kind");
private:
/* Placement of bit fields in discriminator. */
awkward. */
DB_TYPE_SPEC_BIT, /* Specialization in the type table. */
DB_FRIEND_SPEC_BIT, /* An instantiated template friend. */
+ DB_HWM,
};
+ static_assert (DB_HWM <= sizeof(discriminator) * CHAR_BIT,
+ "not enough bits in discriminator");
public:
/* The first slot is special for EK_SPECIALIZATIONS it is a
/* Same order as entity_kind. */
static const char *const names[] =
{"decl", "specialization", "partial", "using",
- "namespace", "redirect", "binding"};
+ "namespace", "tu-local", "redirect", "binding"};
+ static_assert (ARRAY_SIZE (names) == EK_EXPLICIT_HWM + 1,
+ "names must have an entry for every explicit entity_kind");
entity_kind kind = get_entity_kind ();
gcc_checking_assert (kind < ARRAY_SIZE (names));
return names[kind];
return cp_walk_tree_without_duplicates (&t, walker, this);
}
+/* Get the name for TU-local decl T to be used in diagnostics. */
+
+static tree
+name_for_tu_local_decl (tree t)
+{
+ int flags = (TFF_SCOPE | TFF_DECL_SPECIFIERS);
+ const char *str = decl_as_string (t, flags);
+ return get_identifier (str);
+}
+
/* Stream out tree node T. We automatically create local back
references, which is essentially a single pass lisp
self-referential structure pretty-printer. */
&& dump ("Writing TU-local entity:%d %C:%N",
tag, TREE_CODE (t), t);
}
- /* TODO: Get a more descriptive name? */
- tree_node (DECL_NAME (local_decl));
+ tree_node (name_for_tu_local_decl (local_decl));
if (state)
state->write_location (*this, DECL_SOURCE_LOCATION (local_decl));
goto done;
gcc_checking_assert (!is_key_order ());
if (ek == EK_USING)
gcc_checking_assert (TREE_CODE (decl) == OVERLOAD);
+ if (ek == EK_TU_LOCAL)
+ gcc_checking_assert (DECL_DECLARES_FUNCTION_P (decl));
if (TREE_CODE (decl) == TEMPLATE_DECL)
/* The template should have copied these from its result decl. */
dump (dumper::DEPEND)
&& dump ("%s on %s %C:%N found",
ek == EK_REDIRECT ? "Redirect"
- : for_binding ? "Binding" : "Dependency",
+ : (for_binding || ek == EK_TU_LOCAL) ? "Binding"
+ : "Dependency",
dep->entity_kind_name (), TREE_CODE (decl), decl);
return dep;
than trying to clear out bindings after the fact. */
return false;
+ bool internal_decl = false;
if (!header_module_p () && data->hash->is_tu_local_entity (decl))
- /* Ignore TU-local entitites. */
- return false;
+ {
+ /* A TU-local entity. For ADL we still need to create bindings
+ for internal-linkage functions attached to a named module. */
+ if (DECL_DECLARES_FUNCTION_P (inner)
+ && DECL_LANG_SPECIFIC (inner)
+ && DECL_MODULE_ATTACH_P (inner))
+ {
+ gcc_checking_assert (!DECL_MODULE_EXPORT_P (inner));
+ internal_decl = true;
+ }
+ else
+ return false;
+ }
if ((TREE_CODE (decl) == VAR_DECL
|| TREE_CODE (decl) == TYPE_DECL)
OVL_EXPORT_P (decl) = true;
}
- depset *dep = data->hash->make_dependency
- (decl, flags & WMB_Using ? EK_USING : EK_FOR_BINDING);
+ entity_kind ek = EK_FOR_BINDING;
+ if (internal_decl)
+ ek = EK_TU_LOCAL;
+ else if (flags & WMB_Using)
+ ek = EK_USING;
+
+ depset *dep = data->hash->make_dependency (decl, ek);
if (flags & WMB_Hidden)
dep->set_hidden_binding ();
data->binding->deps.safe_push (dep);
walker.begin ();
if (current->get_entity_kind () == EK_USING)
walker.tree_node (OVL_FUNCTION (decl));
+ else if (current->get_entity_kind () == EK_TU_LOCAL)
+ /* We only stream its name and location. */
+ module->note_location (DECL_SOURCE_LOCATION (decl));
else if (TREE_VISITED (decl))
/* A global tree. */;
- else if (item->get_entity_kind () == EK_NAMESPACE)
+ else if (current->get_entity_kind () == EK_NAMESPACE)
{
module->note_location (DECL_SOURCE_LOCATION (decl));
add_namespace_context (current, CP_DECL_CONTEXT (decl));
return a_implicit ? -1 : +1; /* Implicit first. */
}
+ /* TU-local before non-TU-local. */
+ bool a_internal = a->get_entity_kind () == depset::EK_TU_LOCAL;
+ bool b_internal = b->get_entity_kind () == depset::EK_TU_LOCAL;
+ if (a_internal != b_internal)
+ return a_internal ? -1 : +1; /* Internal first. */
+
/* Hidden before non-hidden. */
bool a_hidden = a->is_hidden ();
bool b_hidden = b->is_hidden ();
use_lwm--;
break;
}
- /* We must have copied a using, so move it too. */
+ /* We must have copied a using or TU-local, so move it too. */
dep = scc[ix];
- gcc_checking_assert (dep->get_entity_kind () == depset::EK_USING);
+ gcc_checking_assert
+ (dep->get_entity_kind () == depset::EK_USING
+ || dep->get_entity_kind () == depset::EK_TU_LOCAL);
/* FALLTHROUGH */
case depset::EK_USING:
+ case depset::EK_TU_LOCAL:
if (--use_lwm != ix)
{
scc[ix] = scc[use_lwm];
cbf_export = 0x1, /* An exported decl. */
cbf_hidden = 0x2, /* A hidden (friend) decl. */
cbf_using = 0x4, /* A using decl. */
+ cbf_internal = 0x8, /* A TU-local decl. */
};
/* DEP belongs to a different cluster, seed it to prevent
gcc_checking_assert (dep->is_import ()
|| TREE_VISITED (dep->get_entity ())
|| (dep->get_entity_kind ()
- == depset::EK_USING));
+ == depset::EK_USING)
+ || (dep->get_entity_kind ()
+ == depset::EK_TU_LOCAL));
}
}
break;
/* FALLTHROUGH */
case depset::EK_USING:
+ case depset::EK_TU_LOCAL:
gcc_checking_assert (!b->is_import ()
&& !b->is_unreached ());
dump (dumper::CLUSTER)
depset *dep = b->deps[jx];
tree bound = dep->get_entity ();
unsigned flags = 0;
- if (dep->get_entity_kind () == depset::EK_USING)
+ if (dep->get_entity_kind () == depset::EK_TU_LOCAL)
+ flags |= cbf_internal;
+ else if (dep->get_entity_kind () == depset::EK_USING)
{
tree ovl = bound;
bound = OVL_FUNCTION (bound);
gcc_checking_assert (DECL_P (bound));
sec.i (flags);
- sec.tree_node (bound);
+ if (flags & cbf_internal)
+ {
+ sec.tree_node (name_for_tu_local_decl (bound));
+ write_location (sec, DECL_SOURCE_LOCATION (bound));
+ }
+ else
+ sec.tree_node (bound);
}
/* Terminate the list. */
break;
case depset::EK_USING:
+ case depset::EK_TU_LOCAL:
dump () && dump ("Depset:%u %s %C:%N", ix, b->entity_kind_name (),
TREE_CODE (decl), decl);
break;
tree name = sec.tree_node ();
tree decls = NULL_TREE;
tree visible = NULL_TREE;
+ tree internal = NULL_TREE;
tree type = NULL_TREE;
bool dedup = false;
bool global_p = is_header ();
if ((flags & cbf_hidden)
&& (flags & (cbf_using | cbf_export)))
sec.set_overrun ();
+ if ((flags & cbf_internal)
+ && flags != cbf_internal)
+ sec.set_overrun ();
+
+ if (flags & cbf_internal)
+ {
+ tree name = sec.tree_node ();
+ location_t loc = read_location (sec);
+ if (sec.get_overrun ())
+ break;
+
+ tree decl = make_node (TU_LOCAL_ENTITY);
+ TU_LOCAL_ENTITY_NAME (decl) = name;
+ TU_LOCAL_ENTITY_LOCATION (decl) = loc;
+ internal = tree_cons (NULL_TREE, decl, internal);
+ continue;
+ }
tree decl = sec.tree_node ();
if (sec.get_overrun ())
}
}
- if (!decls)
+ if (!decls && !internal)
sec.set_overrun ();
if (sec.get_overrun ())
dump () && dump ("Binding of %P", ns, name);
if (!set_module_binding (ns, name, mod, global_p,
is_module () || is_partition (),
- decls, type, visible))
+ decls, type, visible, internal))
sec.set_overrun ();
}
break;
tree new_vec = make_binding_vec (name, want);
BINDING_VECTOR_NUM_CLUSTERS (new_vec) = have + 1;
+ BINDING_VECTOR_INTERNAL_DECLS (new_vec)
+ = BINDING_VECTOR_INTERNAL_DECLS (*slot);
BINDING_VECTOR_GLOBAL_DUPS_P (new_vec)
= BINDING_VECTOR_GLOBAL_DUPS_P (*slot);
BINDING_VECTOR_PARTITION_DUPS_P (new_vec)
}
/* For lookups on the instantiation path we can see any
- module-linkage declaration; otherwise we should only
- see exported decls. */
+ declarations visible at any point on the path;
+ otherwise we should only see exported decls. */
if (on_inst_path)
- bind = STAT_DECL (bind);
+ {
+ /* If there are any internal functions visible, naming
+ them outside that module is ill-formed. */
+ auto_diagnostic_group d;
+ if (MODULE_BINDING_INTERNAL_DECLS_P (bind)
+ && pedwarn (input_location, OPT_Wexternal_tu_local,
+ "overload set for argument-dependent "
+ "lookup of %<%D::%D%> in module %qs "
+ "contains TU-local entities",
+ scope, name, module_name (mod, false)))
+ {
+ tree *tu_locals
+ = BINDING_VECTOR_INTERNAL_DECLS (val)->get (mod);
+ gcc_checking_assert (tu_locals && *tu_locals);
+ for (tree t = *tu_locals; t; t = TREE_CHAIN (t))
+ {
+ tree decl = TREE_VALUE (t);
+ inform (TU_LOCAL_ENTITY_LOCATION (decl),
+ "ignoring %qD declared here "
+ "with internal linkage",
+ TU_LOCAL_ENTITY_NAME (decl));
+ }
+ }
+ bind = STAT_DECL (bind);
+ }
else
bind = STAT_VISIBLE (bind);
}
/* An import of MODULE is binding NS::NAME. There should be no
existing binding for >= MODULE. GLOBAL_P indicates whether the
bindings include global module entities. PARTITION_P is true if
- it is part of the current module. VALUE and TYPE are the value
- and type bindings. VISIBLE are the value bindings being exported. */
+ it is part of the current module. VALUE and TYPE are the value
+ and type bindings. VISIBLE are the value bindings being exported.
+ INTERNAL is a TREE_LIST of any TU-local names visible for ADL. */
bool
set_module_binding (tree ns, tree name, unsigned mod, bool global_p,
- bool partition_p, tree value, tree type, tree visible)
+ bool partition_p, tree value, tree type, tree visible,
+ tree internal)
{
- if (!value)
+ if (!value && !internal)
/* Bogus BMIs could give rise to nothing to bind. */
return false;
- gcc_assert (TREE_CODE (value) != NAMESPACE_DECL
+ gcc_assert (!value
+ || TREE_CODE (value) != NAMESPACE_DECL
|| DECL_NAMESPACE_ALIAS (value));
gcc_checking_assert (mod);
return false;
tree bind = value;
- if (type || visible != bind || partition_p || global_p)
+ if (type || visible != bind || internal || partition_p || global_p)
{
bind = stat_hack (bind, type);
STAT_VISIBLE (bind) = visible;
STAT_TYPE_VISIBLE_P (bind) = true;
}
+ /* If this has internal declarations, track them for diagnostics. */
+ if (internal)
+ {
+ if (!BINDING_VECTOR_INTERNAL_DECLS (*slot))
+ BINDING_VECTOR_INTERNAL_DECLS (*slot)
+ = module_tree_map_t::create_ggc ();
+ bool existed = BINDING_VECTOR_INTERNAL_DECLS (*slot)->put (mod, internal);
+ gcc_checking_assert (!existed);
+ MODULE_BINDING_INTERNAL_DECLS_P (bind) = true;
+ }
+
/* Note if this is this-module and/or global binding. */
if (partition_p)
MODULE_BINDING_PARTITION_P (bind) = true;
#define BINDING_VECTOR_CLUSTER(NODE,IX) \
(((tree_binding_vec *)BINDING_VECTOR_CHECK (NODE))->vec[IX])
+struct module_tree_map_traits
+ : simple_hashmap_traits<int_hash<unsigned, 0>, tree> {};
+typedef hash_map<unsigned, tree, module_tree_map_traits> module_tree_map_t;
+
struct GTY(()) tree_binding_vec {
struct tree_base base;
tree name;
+ module_tree_map_t *internal_decls;
binding_cluster GTY((length ("%h.base.u.dependence_info.base"))) vec[1];
};
/* The name of a module vector. */
#define BINDING_VECTOR_NAME(NODE) \
(((tree_binding_vec *)BINDING_VECTOR_CHECK (NODE))->name)
+/* A collection of internal functions for ADL in this binding. */
+#define BINDING_VECTOR_INTERNAL_DECLS(NODE) \
+ (((tree_binding_vec *)BINDING_VECTOR_CHECK (NODE))->internal_decls)
/* tree_binding_vec does uses base.u.dependence_info.base field for
length. It does not have lang_flag etc available! */
#define BINDING_VECTOR_PARTITION_DUPS_P(NODE) \
(BINDING_VECTOR_CHECK (NODE)->base.volatile_flag)
+/* True if the binding slot has internal-linkage functions that should
+ cause ADL to error. */
+#define MODULE_BINDING_INTERNAL_DECLS_P(NODE) \
+ (OVERLOAD_CHECK (NODE)->base.private_flag)
+
/* These two flags indicate the provenence of the bindings on this
particular vector slot. We can of course determine this from slot
number, but that's a relatively expensive lookup. This avoids
unsigned snum);
extern bool set_module_binding (tree ctx, tree name, unsigned mod,
bool global_p, bool partition_p,
- tree value, tree type, tree visible);
+ tree value, tree type, tree visible,
+ tree internal);
extern void add_module_namespace_decl (tree ns, tree decl);
enum WMB_Flags
-Woverloaded-virtual -Wno-pmf-conversions -Wself-move -Wsign-promo
-Wsized-deallocation -Wsuggest-final-methods
-Wsuggest-final-types -Wsuggest-override -Wno-template-body
--Wno-template-id-cdtor -Wtemplate-names-tu-local
+-Wno-template-id-cdtor -Wtemplate-names-tu-local -Wno-external-tu-local
-Wno-terminate -Wno-vexing-parse -Wvirtual-inheritance
-Wno-virtual-move-assign -Wvolatile}
This flag is enabled by @option{-Wextra}.
+@opindex Wexternal-tu-local
+@opindex Wno-external-tu-local
+@item -Wno-external-tu-local
+Warn when naming a TU-local entity outside of the translation unit it
+was declared in. Such declarations will be ignored during name lookup.
+This can occur when performing ADL from a template declared in the same
+TU as the internal function:
+
+@smallexample
+export module M;
+template <typename T> void foo(T t) @{
+ bar(t);
+@}
+struct S @{@} s;
+static void bar(S) @{@} // internal linkage
+
+// instantiating foo(s) from outside this TU can see ::bar,
+// but naming it there is ill-formed.
+@end smallexample
+
+This can be worked around by making @code{bar} attached to the global
+module, using @code{extern "C++"}.
+
+This warning is enabled by default, and is upgraded to an error by
+@option{-pedantic-errors}.
+
@opindex Wterminate
@opindex Wno-terminate
@item -Wno-terminate @r{(C++ and Objective-C++ only)}
// PR c++/117658
-// { dg-additional-options "-fmodules" }
+// { dg-additional-options "-fmodules -Wno-error=external-tu-local" }
import N;
apply_err(x); // error: R::g has internal linkage and cannot be used outside N
// { dg-message "here" "" { target *-*-* } .-1 }
- // { dg-error "'g'" "" { target *-*-* } 0 }
+ // { dg-warning "lookup of 'R::g'" "" { target *-*-* } 0 }
+ // { dg-error "'g' was not declared" "" { target *-*-* } 0 }
auto y = make_Y();
f(y); // OK, I::B::f and I::A::Y have matching innermost non-inline namespace
import m;
int main() {
- // { dg-error "instantiation exposes TU-local entity '(fun1|Dodgy)'" "" { target *-*-* } 0 }
+ // { dg-error "instantiation exposes TU-local entity '\[^']*(fun1|Dodgy)\[^']*'" "" { target *-*-* } 0 }
fun2(123); // { dg-message "required from here" }
}
--- /dev/null
+// { dg-additional-options "-fmodules -fdump-lang-module-graph -Wno-global-module" }
+// { dg-module-cmi A }
+
+export module A;
+
+namespace N {
+ struct A {};
+ void adl(A);
+ inline namespace inner {
+ static void adl(int);
+ }
+}
+namespace G {
+ struct B {};
+ void adl(B);
+ namespace {
+ extern "C++" void adl(int);
+ }
+}
+void adl(double);
+
+template <typename T>
+inline void h(T t) {
+ adl(t);
+}
+
+// { dg-final { scan-lang-dump {Binding on tu-local function_decl:'::N::inner::adl' found} module } }
+// { dg-final { scan-lang-dump-not {'G::_GLOBAL__N_1::adl'} module } }
--- /dev/null
+// { dg-additional-options "-fmodules -pedantic-errors" }
+
+module A;
+
+void other() {
+ adl(N::A{}); // OK, lookup occurs from here
+ h(0); // OK, doesn't consider N::inner::adl
+
+ h(N::A{}); // { dg-message "required from here" }
+ // { dg-error "TU-local" "" { target *-*-* } 0 }
+
+ h(G::B{}); // OK, G::adl is not attached to A
+}
projects / thirdparty / gcc.git / commitdiff
