public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] c++/modules: more checks for exporting names with using-declarations
@ 2023-11-10 22:52 Nathaniel Shead
  2023-11-24 22:08 ` Nathan Sidwell
  2024-01-06 22:31 ` Nathan Sidwell
  0 siblings, 2 replies; 3+ messages in thread
From: Nathaniel Shead @ 2023-11-10 22:52 UTC (permalink / raw)
  To: gcc-patches; +Cc: Nathan Sidwell, Jason Merrill

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 <nathanieloshead@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


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] c++/modules: more checks for exporting names with using-declarations
  2023-11-10 22:52 [PATCH] c++/modules: more checks for exporting names with using-declarations Nathaniel Shead
@ 2023-11-24 22:08 ` Nathan Sidwell
  2024-01-06 22:31 ` Nathan Sidwell
  1 sibling, 0 replies; 3+ messages in thread
From: Nathan Sidwell @ 2023-11-24 22:08 UTC (permalink / raw)
  To: Nathaniel Shead, gcc-patches; +Cc: Jason Merrill

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 <nathanieloshead@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


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] c++/modules: more checks for exporting names with using-declarations
  2023-11-10 22:52 [PATCH] c++/modules: more checks for exporting names with using-declarations Nathaniel Shead
  2023-11-24 22:08 ` Nathan Sidwell
@ 2024-01-06 22:31 ` Nathan Sidwell
  1 sibling, 0 replies; 3+ messages in thread
From: Nathan Sidwell @ 2024-01-06 22:31 UTC (permalink / raw)
  To: Nathaniel Shead, gcc-patches; +Cc: Jason Merrill

ok


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.
> 
> -- >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 <nathanieloshead@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


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2024-01-06 22:31 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-10 22:52 [PATCH] c++/modules: more checks for exporting names with using-declarations Nathaniel Shead
2023-11-24 22:08 ` Nathan Sidwell
2024-01-06 22:31 ` Nathan Sidwell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).