On 11/10/23 17:52, Nathaniel Shead wrote:
I noticed while fixing PR106849 that we don't currently check validity of exporting non-functions via using-declarations either; this patch adds those checks factored out into a new function. I also tried to make the error messages a bit more descriptive.This patch is based on [1] (with the adjustment to use STRIP_TEMPLATE Nathan mentioned), but could probably be a replacement for that patch if preferred - if so I'm happy to re-send rebased off master instead. The ICEs mentioned in the commit message are caused by code such as export module M; namespace { enum e { x }; } export using ::e; in depset::hash::finalize_dependencies when attempting to get a DECL_SOURCE_LOCATION of an OVERLOAD. I haven't fixed that in this patch though because after this patch I was no longer able to construct an example of that error, but it's maybe something to fix up later as well. [1]: https://gcc.gnu.org/pipermail/gcc-patches/2023-November/635869.html Bootstrapped and regtested on x86_64-pc-linux-gnu. I don't have write access.
ok thanks
-- >8 -- Currently only functions are directly checked for validity when exporting via a using-declaration. This patch also checks exporting non-external names of variables, types, and enumerators. This also prevents ICEs with `export using enum` for internal-linkage enums. While we're at it we also improve the error messages for these cases to provide more context about what went wrong. gcc/cp/ChangeLog: * name-lookup.cc (check_can_export_using_decl): New. (do_nonmember_using_decl): Use above to check if names can be exported. gcc/testsuite/ChangeLog: * g++.dg/modules/using-10.C: New test. * g++.dg/modules/using-enum-2.C: New test. Signed-off-by: Nathaniel Shead <nathanielosh...@gmail.com> --- gcc/cp/name-lookup.cc | 74 +++++++++++++++------ gcc/testsuite/g++.dg/modules/using-10.C | 71 ++++++++++++++++++++ gcc/testsuite/g++.dg/modules/using-enum-2.C | 23 +++++++ 3 files changed, 148 insertions(+), 20 deletions(-) create mode 100644 gcc/testsuite/g++.dg/modules/using-10.C create mode 100644 gcc/testsuite/g++.dg/modules/using-enum-2.C diff --git a/gcc/cp/name-lookup.cc b/gcc/cp/name-lookup.cc index e74084948b6..d19ea5d121c 100644 --- a/gcc/cp/name-lookup.cc +++ b/gcc/cp/name-lookup.cc @@ -4802,6 +4802,49 @@ pushdecl_outermost_localscope (tree x) return b ? do_pushdecl_with_scope (x, b) : error_mark_node; }+/* Checks if BINDING is a binding that we can export. */+ +static bool +check_can_export_using_decl (tree binding) +{ + tree decl = STRIP_TEMPLATE (binding); + + /* Linkage is determined by the owner of an enumerator. */ + if (TREE_CODE (decl) == CONST_DECL) + decl = TYPE_NAME (DECL_CONTEXT (decl)); + + /* If the using decl is exported, the things it refers + to must also be exported (or not have module attachment). */ + if (!DECL_MODULE_EXPORT_P (decl) + && (DECL_LANG_SPECIFIC (decl) + && DECL_MODULE_ATTACH_P (decl))) + { + bool internal_p = !TREE_PUBLIC (decl); + + /* A template in an anonymous namespace doesn't constrain TREE_PUBLIC + until it's instantiated, so double-check its context. */ + if (!internal_p && TREE_CODE (binding) == TEMPLATE_DECL) + internal_p = decl_internal_context_p (decl); + + auto_diagnostic_group d; + error ("exporting %q#D that does not have external linkage", + binding); + if (TREE_CODE (decl) == TYPE_DECL && !DECL_IMPLICIT_TYPEDEF_P (decl)) + /* An un-exported explicit type alias has no linkage. */ + inform (DECL_SOURCE_LOCATION (binding), + "%q#D declared here with no linkage", binding); + else if (internal_p) + inform (DECL_SOURCE_LOCATION (binding), + "%q#D declared here with internal linkage", binding); + else + inform (DECL_SOURCE_LOCATION (binding), + "%q#D declared here with module linkage", binding); + return false; + } + + return true; +} + /* Process a local-scope or namespace-scope using declaration. LOOKUP is the result of qualified lookup (both value & type are significant). FN_SCOPE_P indicates if we're at function-scope (as @@ -4845,22 +4888,7 @@ do_nonmember_using_decl (name_lookup &lookup, bool fn_scope_p, tree new_fn = *usings; bool exporting = revealing_p && module_exporting_p (); if (exporting) - { - tree decl = STRIP_TEMPLATE (new_fn); - - /* If the using decl is exported, the things it refers - to must also be exported (or not have module attachment). */ - if (!DECL_MODULE_EXPORT_P (decl) - && (DECL_LANG_SPECIFIC (decl) - && DECL_MODULE_ATTACH_P (decl))) - { - auto_diagnostic_group d; - error ("%q#D does not have external linkage", new_fn); - inform (DECL_SOURCE_LOCATION (new_fn), - "%q#D declared here", new_fn); - exporting = false; - } - } + exporting = check_can_export_using_decl (new_fn);/* [namespace.udecl] @@ -4938,20 +4966,26 @@ do_nonmember_using_decl (name_lookup &lookup, bool fn_scope_p,failed = true; } else if (insert_p) - // FIXME:what if we're newly exporting lookup.value - value = lookup.value; + { + value = lookup.value; + if (revealing_p && module_exporting_p ()) + check_can_export_using_decl (value); + }/* Now the type binding. */if (lookup.type && lookup.type != type) { - // FIXME: What if we're exporting lookup.type? if (type && !decls_match (lookup.type, type)) { diagnose_name_conflict (lookup.type, type); failed = true; } else if (insert_p) - type = lookup.type; + { + type = lookup.type; + if (revealing_p && module_exporting_p ()) + check_can_export_using_decl (type); + } }if (insert_p)diff --git a/gcc/testsuite/g++.dg/modules/using-10.C b/gcc/testsuite/g++.dg/modules/using-10.C new file mode 100644 index 00000000000..5735353ee21 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/using-10.C @@ -0,0 +1,71 @@ +// { dg-additional-options "-fmodules-ts" } +// { dg-module-cmi !bad } + +export module bad; + +// internal linkage +namespace s { + namespace { + struct a1 {}; // { dg-message "declared here with internal linkage" } + + template <typename T> + struct b1; // { dg-message "declared here with internal linkage" } + + int x1; // { dg-message "declared here with internal linkage" } + + template <typename T> + T y1; // { dg-message "declared here with internal linkage" } + + void f1(); // { dg-message "declared here with internal linkage" } + + template <typename T> + void g1(); // { dg-message "declared here with internal linkage" } + } +} + +// module linkage +namespace m { + struct a2 {}; // { dg-message "declared here with module linkage" } + + template <typename T> + struct b2; // { dg-message "declared here with module linkage" } + + int x2; // { dg-message "declared here with module linkage" } + + template <typename T> + T y2; // { dg-message "declared here with module linkage" } + + void f2(); // { dg-message "declared here with module linkage" } + + template <typename T> + void g2(); // { dg-message "declared here with module linkage" } +} + +export using s::a1; // { dg-error "does not have external linkage" } +export using s::b1; // { dg-error "does not have external linkage" } +export using s::x1; // { dg-error "does not have external linkage" } +export using s::y1; // { dg-error "does not have external linkage" } +export using s::f1; // { dg-error "does not have external linkage" } +export using s::g1; // { dg-error "does not have external linkage" } + +export using m::a2; // { dg-error "does not have external linkage" } +export using m::b2; // { dg-error "does not have external linkage" } +export using m::x2; // { dg-error "does not have external linkage" } +export using m::y2; // { dg-error "does not have external linkage" } +export using m::f2; // { dg-error "does not have external linkage" } +export using m::g2; // { dg-error "does not have external linkage" } + +namespace t { + using a = int; // { dg-message "declared here with no linkage" } + + template <typename T> + using b = int; // { dg-message "declared here with no linkage" } + + typedef int c; // { dg-message "declared here with no linkage" } +} + +export using t::a; // { dg-error "does not have external linkage" } +export using t::b; // { dg-error "does not have external linkage" } +export using t::c; // { dg-error "does not have external linkage" } + +// { dg-prune-output "not writing module" } diff --git a/gcc/testsuite/g++.dg/modules/using-enum-2.C b/gcc/testsuite/g++.dg/modules/using-enum-2.C new file mode 100644 index 00000000000..813e2f630ab --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/using-enum-2.C @@ -0,0 +1,23 @@ +// { dg-additional-options "-fmodules-ts -std=c++2a" } +// { dg-module-cmi !bad } + +export module bad; + +namespace s { + namespace { + enum e1 { x1 }; // { dg-message "declared here with internal linkage" } + enum class e2 { x2 }; // { dg-message "declared here with internal linkage" } + } +} + +namespace m { + enum e3 { x3 }; // { dg-message "declared here with module linkage" } + enum class e4 { x4 }; // { dg-message "declared here with module linkage" } +} + +export using enum s::e1; // { dg-error "does not have external linkage" } +export using enum s::e2; // { dg-error "does not have external linkage" } +export using enum m::e3; // { dg-error "does not have external linkage" } +export using enum m::e4; // { dg-error "does not have external linkage" } + +// { dg-prune-output "not writing module" }
-- Nathan Sidwell
