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.

-- >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" }
-- 
2.42.0

Reply via email to