public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* Re: [PATCH] c++: Refer to internal linkage for -Wsubobject-linkage [PR86491]
       [not found] <c1215d18-73b0-95aa-2c02-485536696a48@redhat.com>
@ 2022-09-12 16:10 ` Jason Merrill
  0 siblings, 0 replies; 4+ messages in thread
From: Jason Merrill @ 2022-09-12 16:10 UTC (permalink / raw)
  To: gcc-patches List

[-- Attachment #1: Type: text/plain, Size: 1777 bytes --]


Oops, failed to CC the list.

-------- Forwarded Message --------
Subject: Re: [PATCH] c++: Refer to internal linkage for 
-Wsubobject-linkage [PR86491]
Date: Mon, 12 Sep 2022 12:09:38 -0400
From: Jason Merrill <jason@redhat.com>
To: Jonathan Wakely <jwakely@redhat.com>

On 7/23/22 07:31, Jonathan Wakely wrote:
> I'll try that on Monday, thanks!
> 
> On Fri, 22 Jul 2022 at 22:30, Jason Merrill <jason@redhat.com> wrote:
>>
>> On 6/30/22 12:53, Jonathan Wakely via Gcc-patches wrote:
>>> Tested powerpc64le-linux, OK for trunk?
>>>
>>> -- >8 --
>>>
>>> Since C++11 relaxed the requirement for template arguments to have
>>> external linkage, it's possible to get -Wsubobject-linkage warnings
>>> without using any anonymous namespaces. This confuses users when they
>>> get diagnostics that refer to an anonymous namespace that doesn't exist
>>> in their code.
>>>
>>> This changes the diagnostic to say "has internal linkage" for C++11 and
>>> later, which is accurate whether internal linkage is due to the 'static'
>>> specifier, or due to the use of anonymous namespaces.
>>>
>>> For C++98 template arguments declared with 'static' are ill-formed
>>> anyway, so the only way this warning can arise is via anonymous
>>> namespaces. That means the existing wording is accurate for C++98 and so
>>> we can keep it.
>>
>> I'd prefer to keep the existing wording for types that are actually in
>> an anonymous namespace.  Checking decl_anon_ns_mem_p seems like the way
>> to do that, though it would probably need to remove the existing type
>> shortcut and instead just do decl = TYPE_MAIN_DECL (decl).

I decided to follow up on this myself, and ended up renaming the current 
function as other users relied on its semantics.

Tested x86_64-pc-linux-gnu, applying to trunk.

[-- Attachment #2: 0001-c-Refer-to-internal-linkage-for-Wsubobject-linkage-P.patch --]
[-- Type: text/x-patch, Size: 8673 bytes --]

From 6cbff05579b8d1acfe953b69ba2710d6b36e830e Mon Sep 17 00:00:00 2001
From: Jonathan Wakely <jwakely@redhat.com>
Date: Thu, 30 Jun 2022 17:53:26 +0100
Subject: [PATCH] c++: Refer to internal linkage for -Wsubobject-linkage
 [PR86491]
To: gcc-patches@gcc.gnu.org

Since C++11 relaxed the requirement for template arguments to have
external linkage, it's possible to get -Wsubobject-linkage warnings
without using any anonymous namespaces. This confuses users when they
get diagnostics that refer to an anonymous namespace that doesn't exist
in their code.

This changes the diagnostic to say "has internal linkage" for C++11 and
later, if the type isn't actually a member of the anonymous namespace.
Making that distinction involved renaming the current decl_anon_ns_mem_p to
something that better expresses its semantics.

For C++98 template arguments declared with 'static' are ill-formed
anyway, so the only way this warning can arise is via anonymous
namespaces. That means the existing wording is accurate for C++98 and so
we can keep it.

	PR c++/86491

gcc/cp/ChangeLog:

	* decl2.cc (constrain_class_visibility): Adjust wording of
	-Wsubobject-linkage for cases where anonymous
	namespaces aren't used.
	* tree.cc (decl_anon_ns_mem_p): Now only true for actual anonymous
	namespace members, rename old semantics to...
	(decl_internal_context_p): ...this.
	* cp-tree.h, name-lookup.cc, pt.cc: Adjust.

gcc/testsuite/ChangeLog:

	* g++.dg/warn/anonymous-namespace-3.C: Use separate dg-warning
	directives for C++98 and everything else.
	* g++.dg/warn/Wsubobject-linkage-5.C: New test.
---
 gcc/cp/cp-tree.h                              |  3 +-
 gcc/cp/decl2.cc                               | 39 ++++++++++++-------
 gcc/cp/name-lookup.cc                         |  2 +-
 gcc/cp/pt.cc                                  |  2 +-
 gcc/cp/tree.cc                                | 12 +++++-
 .../g++.dg/warn/Wsubobject-linkage-5.C        |  7 ++++
 .../g++.dg/warn/anonymous-namespace-3.C       |  3 +-
 7 files changed, 48 insertions(+), 20 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/warn/Wsubobject-linkage-5.C

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 7b28405c3ac..e73d04f21d8 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -7874,7 +7874,8 @@ extern tree replace_placeholders		(tree, tree, bool * = NULL);
 extern bool find_placeholders			(tree);
 extern tree get_type_decl			(tree);
 extern tree decl_namespace_context		(tree);
-extern bool decl_anon_ns_mem_p			(const_tree);
+extern bool decl_anon_ns_mem_p			(tree);
+extern bool decl_internal_context_p		(const_tree);
 extern tree lvalue_type				(tree);
 extern tree error_type				(tree);
 extern int varargs_function_p			(const_tree);
diff --git a/gcc/cp/decl2.cc b/gcc/cp/decl2.cc
index cd188813bee..684a2d06dde 100644
--- a/gcc/cp/decl2.cc
+++ b/gcc/cp/decl2.cc
@@ -2851,7 +2851,7 @@ determine_visibility (tree decl)
   if (class_type)
     determine_visibility_from_class (decl, class_type);
 
-  if (decl_anon_ns_mem_p (decl))
+  if (decl_internal_context_p (decl))
     /* Names in an anonymous namespace get internal linkage.  */
     constrain_visibility (decl, VISIBILITY_ANON, false);
   else if (TREE_CODE (decl) != TYPE_DECL)
@@ -2965,16 +2965,21 @@ constrain_class_visibility (tree type)
 		  {
 		    if (same_type_p (TREE_TYPE (t), nlt))
 		      warning (OPT_Wsubobject_linkage, "\
-%qT has a field %qD whose type has no linkage",
+%qT has a field %q#D whose type has no linkage",
 			       type, t);
 		    else
 		      warning (OPT_Wsubobject_linkage, "\
 %qT has a field %qD whose type depends on the type %qT which has no linkage",
 			       type, t, nlt);
 		  }
-		else
+		else if (cxx_dialect > cxx98
+			 && !decl_anon_ns_mem_p (ftype))
 		  warning (OPT_Wsubobject_linkage, "\
-%qT has a field %qD whose type uses the anonymous namespace",
+%qT has a field %q#D whose type has internal linkage",
+			   type, t);
+		else // In C++98 this can only happen with unnamed namespaces.
+		  warning (OPT_Wsubobject_linkage, "\
+%qT has a field %q#D whose type uses the anonymous namespace",
 			   type, t);
 	      }
 	  }
@@ -2989,28 +2994,34 @@ constrain_class_visibility (tree type)
   binfo = TYPE_BINFO (type);
   for (i = 0; BINFO_BASE_ITERATE (binfo, i, t); ++i)
     {
-      int subvis = type_visibility (TREE_TYPE (t));
+      tree btype = BINFO_TYPE (t);
+      int subvis = type_visibility (btype);
 
       if (subvis == VISIBILITY_ANON)
         {
 	  if (!in_main_input_context())
 	    {
-	      tree nlt = no_linkage_check (TREE_TYPE (t), /*relaxed_p=*/false);
+	      tree nlt = no_linkage_check (btype, /*relaxed_p=*/false);
 	      if (nlt)
 		{
-		  if (same_type_p (TREE_TYPE (t), nlt))
+		  if (same_type_p (btype, nlt))
 		    warning (OPT_Wsubobject_linkage, "\
-%qT has a base %qT whose type has no linkage",
-			     type, TREE_TYPE (t));
+%qT has a base %qT which has no linkage",
+			     type, btype);
 		  else
 		    warning (OPT_Wsubobject_linkage, "\
-%qT has a base %qT whose type depends on the type %qT which has no linkage",
-			     type, TREE_TYPE (t), nlt);
+%qT has a base %qT which depends on the type %qT which has no linkage",
+			     type, btype, nlt);
 		}
-	      else
+	      else if (cxx_dialect > cxx98
+		       && !decl_anon_ns_mem_p (btype))
 		warning (OPT_Wsubobject_linkage, "\
-%qT has a base %qT whose type uses the anonymous namespace",
-			 type, TREE_TYPE (t));
+%qT has a base %qT which has internal linkage",
+			 type, btype);
+	      else // In C++98 this can only happen with unnamed namespaces.
+		warning (OPT_Wsubobject_linkage, "\
+%qT has a base %qT which uses the anonymous namespace",
+			 type, btype);
 	    }
 	}
       else if (vis < VISIBILITY_HIDDEN
diff --git a/gcc/cp/name-lookup.cc b/gcc/cp/name-lookup.cc
index f89a1dc4111..69d555ddf1f 100644
--- a/gcc/cp/name-lookup.cc
+++ b/gcc/cp/name-lookup.cc
@@ -402,7 +402,7 @@ add_decl_to_level (cp_binding_level *b, tree decl)
       && ((VAR_P (decl) && (TREE_STATIC (decl) || DECL_EXTERNAL (decl)))
 	  || (TREE_CODE (decl) == FUNCTION_DECL
 	      && (!TREE_PUBLIC (decl)
-		  || decl_anon_ns_mem_p (decl)
+		  || decl_internal_context_p (decl)
 		  || DECL_DECLARED_INLINE_P (decl)))))
     vec_safe_push (static_decls, decl);
 }
diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index c5fc0f1eab8..ad9c2f9b180 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -25025,7 +25025,7 @@ mark_decl_instantiated (tree result, int extern_p)
     return;
 
   /* For anonymous namespace we don't need to do anything.  */
-  if (decl_anon_ns_mem_p (result))
+  if (decl_internal_context_p (result))
     {
       gcc_assert (!TREE_PUBLIC (result));
       return;
diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc
index c678e3b9c4c..6de208a909b 100644
--- a/gcc/cp/tree.cc
+++ b/gcc/cp/tree.cc
@@ -2968,7 +2968,7 @@ verify_stmt_tree (tree t)
 /* Check if the type T depends on a type with no linkage and if so,
    return it.  If RELAXED_P then do not consider a class type declared
    within a vague-linkage function to have no linkage.  Remember:
-   no-linkage is not the same as internal-linkage*/
+   no-linkage is not the same as internal-linkage.  */
 
 tree
 no_linkage_check (tree t, bool relaxed_p)
@@ -3817,7 +3817,15 @@ decl_namespace_context (tree decl)
    nested, or false otherwise.  */
 
 bool
-decl_anon_ns_mem_p (const_tree decl)
+decl_anon_ns_mem_p (tree decl)
+{
+  return !TREE_PUBLIC (decl_namespace_context (decl));
+}
+
+/* Returns true if the enclosing scope of DECL has internal or no linkage.  */
+
+bool
+decl_internal_context_p (const_tree decl)
 {
   while (TREE_CODE (decl) != NAMESPACE_DECL)
     {
diff --git a/gcc/testsuite/g++.dg/warn/Wsubobject-linkage-5.C b/gcc/testsuite/g++.dg/warn/Wsubobject-linkage-5.C
new file mode 100644
index 00000000000..e2c2fd91688
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wsubobject-linkage-5.C
@@ -0,0 +1,7 @@
+// PR c++/86491
+// { dg-do compile { target c++11 } }
+
+template <int *> struct NT{};
+#line 6 "tM.C"
+static int d;
+struct D : NT<&d> {};		// { dg-warning "internal linkage" }
diff --git a/gcc/testsuite/g++.dg/warn/anonymous-namespace-3.C b/gcc/testsuite/g++.dg/warn/anonymous-namespace-3.C
index 8b72abdf5d1..ce5745b25f0 100644
--- a/gcc/testsuite/g++.dg/warn/anonymous-namespace-3.C
+++ b/gcc/testsuite/g++.dg/warn/anonymous-namespace-3.C
@@ -7,7 +7,8 @@
 struct B { std::auto_ptr<A> p; };
 
 #line 10 "foo.C"
-struct C		   // { dg-warning "uses the anonymous namespace" }
+struct C // { dg-warning "has internal linkage" "" { target c++11 } }
+// { dg-warning "uses the anonymous namespace" "" { target c++98_only } .-1 }
 {
   std::auto_ptr<A> p;
 };
-- 
2.31.1


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

* Re: [PATCH] c++: Refer to internal linkage for -Wsubobject-linkage [PR86491]
  2022-06-30 16:53 Jonathan Wakely
  2022-07-22 11:20 ` Jonathan Wakely
@ 2022-07-22 21:30 ` Jason Merrill
  1 sibling, 0 replies; 4+ messages in thread
From: Jason Merrill @ 2022-07-22 21:30 UTC (permalink / raw)
  To: Jonathan Wakely, gcc-patches

On 6/30/22 12:53, Jonathan Wakely via Gcc-patches wrote:
> Tested powerpc64le-linux, OK for trunk?
> 
> -- >8 --
> 
> Since C++11 relaxed the requirement for template arguments to have
> external linkage, it's possible to get -Wsubobject-linkage warnings
> without using any anonymous namespaces. This confuses users when they
> get diagnostics that refer to an anonymous namespace that doesn't exist
> in their code.
> 
> This changes the diagnostic to say "has internal linkage" for C++11 and
> later, which is accurate whether internal linkage is due to the 'static'
> specifier, or due to the use of anonymous namespaces.
> 
> For C++98 template arguments declared with 'static' are ill-formed
> anyway, so the only way this warning can arise is via anonymous
> namespaces. That means the existing wording is accurate for C++98 and so
> we can keep it.

I'd prefer to keep the existing wording for types that are actually in 
an anonymous namespace.  Checking decl_anon_ns_mem_p seems like the way 
to do that, though it would probably need to remove the existing type 
shortcut and instead just do decl = TYPE_MAIN_DECL (decl).

> 	PR c++/86491
> 
> gcc/cp/ChangeLog:
> 
> 	* decl2.cc (constrain_class_visibility): Adjust wording of
> 	-Wsubobject-linkage to account for cases where anonymous
> 	namespaces aren't used.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/warn/Wsubobject-linkage-3.C: Adjust for new warning.
> 	* g++.dg/warn/anonymous-namespace-1.C: Use separate dg-warning
> 	directives for C++98 and everything else.
> 	* g++.dg/warn/anonymous-namespace-2.C: Likewise.
> 	* g++.dg/warn/anonymous-namespace-3.C: Likewise.
> ---
>   gcc/cp/decl2.cc                                   | 12 ++++++++++--
>   gcc/testsuite/g++.dg/warn/Wsubobject-linkage-3.C  |  4 ++--
>   gcc/testsuite/g++.dg/warn/anonymous-namespace-1.C |  8 ++++++--
>   gcc/testsuite/g++.dg/warn/anonymous-namespace-2.C |  9 ++++++---
>   gcc/testsuite/g++.dg/warn/anonymous-namespace-3.C |  3 ++-
>   5 files changed, 26 insertions(+), 10 deletions(-)
> 
> diff --git a/gcc/cp/decl2.cc b/gcc/cp/decl2.cc
> index 3737e5f010c..de53678715e 100644
> --- a/gcc/cp/decl2.cc
> +++ b/gcc/cp/decl2.cc
> @@ -3027,7 +3027,11 @@ constrain_class_visibility (tree type)
>   %qT has a field %qD whose type depends on the type %qT which has no linkage",
>   			       type, t, nlt);
>   		  }
> -		else
> +		else if (cxx_dialect > cxx98)
> +		  warning (OPT_Wsubobject_linkage, "\
> +%qT has a field %qD whose type has internal linkage",
> +			   type, t);
> +		else // In C++98 this can only happen with unnamed namespaces.
>   		  warning (OPT_Wsubobject_linkage, "\
>   %qT has a field %qD whose type uses the anonymous namespace",
>   			   type, t);
> @@ -3062,7 +3066,11 @@ constrain_class_visibility (tree type)
>   %qT has a base %qT whose type depends on the type %qT which has no linkage",
>   			     type, TREE_TYPE (t), nlt);
>   		}
> -	      else
> +	      else if (cxx_dialect > cxx98)
> +		warning (OPT_Wsubobject_linkage, "\
> +%qT has a base %qT whose type has internal linkage",
> +			 type, TREE_TYPE (t));
> +	      else // In C++98 this can only happen with unnamed namespaces.
>   		warning (OPT_Wsubobject_linkage, "\
>   %qT has a base %qT whose type uses the anonymous namespace",
>   			 type, TREE_TYPE (t));
> diff --git a/gcc/testsuite/g++.dg/warn/Wsubobject-linkage-3.C b/gcc/testsuite/g++.dg/warn/Wsubobject-linkage-3.C
> index 95a04501441..b116fbbb186 100644
> --- a/gcc/testsuite/g++.dg/warn/Wsubobject-linkage-3.C
> +++ b/gcc/testsuite/g++.dg/warn/Wsubobject-linkage-3.C
> @@ -3,7 +3,7 @@
>   namespace { struct Foo { }; }
>   
>   #line 6 "foo.C"
> -struct Bar { Foo foo; };   // { dg-warning "anonymous namespace" }
> +struct Bar { Foo foo; };   // { dg-warning "anonymous namespace|internal linkage" }
>   // { dg-bogus "no linkage" "" { target *-*-* } .-1 }
> -struct Bar2 : Foo { };     // { dg-warning "anonymous namespace" }
> +struct Bar2 : Foo { };     // { dg-warning "anonymous namespace|internal linkage" }
>   // { dg-bogus "no linkage" "" { target *-*-* } .-1 }
> diff --git a/gcc/testsuite/g++.dg/warn/anonymous-namespace-1.C b/gcc/testsuite/g++.dg/warn/anonymous-namespace-1.C
> index cf193e0cba5..eed3818c5cf 100644
> --- a/gcc/testsuite/g++.dg/warn/anonymous-namespace-1.C
> +++ b/gcc/testsuite/g++.dg/warn/anonymous-namespace-1.C
> @@ -14,5 +14,9 @@ class foobar1
>   };
>   
>   #line 17 "foo.C"
> -class foobar : public bad { }; // { dg-warning "uses the anonymous namespace" }
> -class foobar2 { bad b; }; // { dg-warning "uses the anonymous namespace" }
> +class foobar : public bad { };
> +// { dg-warning "has internal linkage" "" { target c++11 } .-1 }
> +// { dg-warning "uses the anonymous namespace" "" { target c++98_only } .-2 }
> +class foobar2 { bad b; };
> +// { dg-warning "has internal linkage" "" { target c++11 } .-1 }
> +// { dg-warning "uses the anonymous namespace" "" { target c++98_only } .-2 }
> diff --git a/gcc/testsuite/g++.dg/warn/anonymous-namespace-2.C b/gcc/testsuite/g++.dg/warn/anonymous-namespace-2.C
> index 4048f3959df..f2ca5915278 100644
> --- a/gcc/testsuite/g++.dg/warn/anonymous-namespace-2.C
> +++ b/gcc/testsuite/g++.dg/warn/anonymous-namespace-2.C
> @@ -18,12 +18,15 @@ struct g3 {
>   };
>   
>   #line 21 "foo.C"
> -struct b1 { // { dg-warning "uses the anonymous namespace" }
> +struct b1 { // { dg-warning "has internal linkage" "" { target c++11 } }
> +// { dg-warning "uses the anonymous namespace" "" { target c++98_only } .-1 }
>       bad * B;
>   };
> -struct b2 { // { dg-warning "uses the anonymous namespace" }
> +struct b2 { // { dg-warning "has internal linkage" "" { target c++11 } }
> +// { dg-warning "uses the anonymous namespace" "" { target c++98_only } .-1 }
>       bad * B[1];
>   };
> -struct b3 { // { dg-warning "uses the anonymous namespace" }
> +struct b3 { // { dg-warning "has internal linkage" "" { target c++11 } }
> +// { dg-warning "uses the anonymous namespace" "" { target c++98_only } .-1 }
>       bad (*B)[1];
>   };
> diff --git a/gcc/testsuite/g++.dg/warn/anonymous-namespace-3.C b/gcc/testsuite/g++.dg/warn/anonymous-namespace-3.C
> index 8b72abdf5d1..ce5745b25f0 100644
> --- a/gcc/testsuite/g++.dg/warn/anonymous-namespace-3.C
> +++ b/gcc/testsuite/g++.dg/warn/anonymous-namespace-3.C
> @@ -7,7 +7,8 @@
>   struct B { std::auto_ptr<A> p; };
>   
>   #line 10 "foo.C"
> -struct C		   // { dg-warning "uses the anonymous namespace" }
> +struct C // { dg-warning "has internal linkage" "" { target c++11 } }
> +// { dg-warning "uses the anonymous namespace" "" { target c++98_only } .-1 }
>   {
>     std::auto_ptr<A> p;
>   };


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

* Re: [PATCH] c++: Refer to internal linkage for -Wsubobject-linkage [PR86491]
  2022-06-30 16:53 Jonathan Wakely
@ 2022-07-22 11:20 ` Jonathan Wakely
  2022-07-22 21:30 ` Jason Merrill
  1 sibling, 0 replies; 4+ messages in thread
From: Jonathan Wakely @ 2022-07-22 11:20 UTC (permalink / raw)
  To: gcc-patches

ping
https://gcc.gnu.org/pipermail/gcc-patches/2022-June/597560.html

On Thu, 30 Jun 2022 at 17:53, Jonathan Wakely <jwakely@redhat.com> wrote:
>
> Tested powerpc64le-linux, OK for trunk?
>
> -- >8 --
>
> Since C++11 relaxed the requirement for template arguments to have
> external linkage, it's possible to get -Wsubobject-linkage warnings
> without using any anonymous namespaces. This confuses users when they
> get diagnostics that refer to an anonymous namespace that doesn't exist
> in their code.
>
> This changes the diagnostic to say "has internal linkage" for C++11 and
> later, which is accurate whether internal linkage is due to the 'static'
> specifier, or due to the use of anonymous namespaces.
>
> For C++98 template arguments declared with 'static' are ill-formed
> anyway, so the only way this warning can arise is via anonymous
> namespaces. That means the existing wording is accurate for C++98 and so
> we can keep it.
>
>         PR c++/86491
>
> gcc/cp/ChangeLog:
>
>         * decl2.cc (constrain_class_visibility): Adjust wording of
>         -Wsubobject-linkage to account for cases where anonymous
>         namespaces aren't used.
>
> gcc/testsuite/ChangeLog:
>
>         * g++.dg/warn/Wsubobject-linkage-3.C: Adjust for new warning.
>         * g++.dg/warn/anonymous-namespace-1.C: Use separate dg-warning
>         directives for C++98 and everything else.
>         * g++.dg/warn/anonymous-namespace-2.C: Likewise.
>         * g++.dg/warn/anonymous-namespace-3.C: Likewise.
> ---
>  gcc/cp/decl2.cc                                   | 12 ++++++++++--
>  gcc/testsuite/g++.dg/warn/Wsubobject-linkage-3.C  |  4 ++--
>  gcc/testsuite/g++.dg/warn/anonymous-namespace-1.C |  8 ++++++--
>  gcc/testsuite/g++.dg/warn/anonymous-namespace-2.C |  9 ++++++---
>  gcc/testsuite/g++.dg/warn/anonymous-namespace-3.C |  3 ++-
>  5 files changed, 26 insertions(+), 10 deletions(-)
>
> diff --git a/gcc/cp/decl2.cc b/gcc/cp/decl2.cc
> index 3737e5f010c..de53678715e 100644
> --- a/gcc/cp/decl2.cc
> +++ b/gcc/cp/decl2.cc
> @@ -3027,7 +3027,11 @@ constrain_class_visibility (tree type)
>  %qT has a field %qD whose type depends on the type %qT which has no linkage",
>                                type, t, nlt);
>                   }
> -               else
> +               else if (cxx_dialect > cxx98)
> +                 warning (OPT_Wsubobject_linkage, "\
> +%qT has a field %qD whose type has internal linkage",
> +                          type, t);
> +               else // In C++98 this can only happen with unnamed namespaces.
>                   warning (OPT_Wsubobject_linkage, "\
>  %qT has a field %qD whose type uses the anonymous namespace",
>                            type, t);
> @@ -3062,7 +3066,11 @@ constrain_class_visibility (tree type)
>  %qT has a base %qT whose type depends on the type %qT which has no linkage",
>                              type, TREE_TYPE (t), nlt);
>                 }
> -             else
> +             else if (cxx_dialect > cxx98)
> +               warning (OPT_Wsubobject_linkage, "\
> +%qT has a base %qT whose type has internal linkage",
> +                        type, TREE_TYPE (t));
> +             else // In C++98 this can only happen with unnamed namespaces.
>                 warning (OPT_Wsubobject_linkage, "\
>  %qT has a base %qT whose type uses the anonymous namespace",
>                          type, TREE_TYPE (t));
> diff --git a/gcc/testsuite/g++.dg/warn/Wsubobject-linkage-3.C b/gcc/testsuite/g++.dg/warn/Wsubobject-linkage-3.C
> index 95a04501441..b116fbbb186 100644
> --- a/gcc/testsuite/g++.dg/warn/Wsubobject-linkage-3.C
> +++ b/gcc/testsuite/g++.dg/warn/Wsubobject-linkage-3.C
> @@ -3,7 +3,7 @@
>  namespace { struct Foo { }; }
>
>  #line 6 "foo.C"
> -struct Bar { Foo foo; };   // { dg-warning "anonymous namespace" }
> +struct Bar { Foo foo; };   // { dg-warning "anonymous namespace|internal linkage" }
>  // { dg-bogus "no linkage" "" { target *-*-* } .-1 }
> -struct Bar2 : Foo { };     // { dg-warning "anonymous namespace" }
> +struct Bar2 : Foo { };     // { dg-warning "anonymous namespace|internal linkage" }
>  // { dg-bogus "no linkage" "" { target *-*-* } .-1 }
> diff --git a/gcc/testsuite/g++.dg/warn/anonymous-namespace-1.C b/gcc/testsuite/g++.dg/warn/anonymous-namespace-1.C
> index cf193e0cba5..eed3818c5cf 100644
> --- a/gcc/testsuite/g++.dg/warn/anonymous-namespace-1.C
> +++ b/gcc/testsuite/g++.dg/warn/anonymous-namespace-1.C
> @@ -14,5 +14,9 @@ class foobar1
>  };
>
>  #line 17 "foo.C"
> -class foobar : public bad { }; // { dg-warning "uses the anonymous namespace" }
> -class foobar2 { bad b; }; // { dg-warning "uses the anonymous namespace" }
> +class foobar : public bad { };
> +// { dg-warning "has internal linkage" "" { target c++11 } .-1 }
> +// { dg-warning "uses the anonymous namespace" "" { target c++98_only } .-2 }
> +class foobar2 { bad b; };
> +// { dg-warning "has internal linkage" "" { target c++11 } .-1 }
> +// { dg-warning "uses the anonymous namespace" "" { target c++98_only } .-2 }
> diff --git a/gcc/testsuite/g++.dg/warn/anonymous-namespace-2.C b/gcc/testsuite/g++.dg/warn/anonymous-namespace-2.C
> index 4048f3959df..f2ca5915278 100644
> --- a/gcc/testsuite/g++.dg/warn/anonymous-namespace-2.C
> +++ b/gcc/testsuite/g++.dg/warn/anonymous-namespace-2.C
> @@ -18,12 +18,15 @@ struct g3 {
>  };
>
>  #line 21 "foo.C"
> -struct b1 { // { dg-warning "uses the anonymous namespace" }
> +struct b1 { // { dg-warning "has internal linkage" "" { target c++11 } }
> +// { dg-warning "uses the anonymous namespace" "" { target c++98_only } .-1 }
>      bad * B;
>  };
> -struct b2 { // { dg-warning "uses the anonymous namespace" }
> +struct b2 { // { dg-warning "has internal linkage" "" { target c++11 } }
> +// { dg-warning "uses the anonymous namespace" "" { target c++98_only } .-1 }
>      bad * B[1];
>  };
> -struct b3 { // { dg-warning "uses the anonymous namespace" }
> +struct b3 { // { dg-warning "has internal linkage" "" { target c++11 } }
> +// { dg-warning "uses the anonymous namespace" "" { target c++98_only } .-1 }
>      bad (*B)[1];
>  };
> diff --git a/gcc/testsuite/g++.dg/warn/anonymous-namespace-3.C b/gcc/testsuite/g++.dg/warn/anonymous-namespace-3.C
> index 8b72abdf5d1..ce5745b25f0 100644
> --- a/gcc/testsuite/g++.dg/warn/anonymous-namespace-3.C
> +++ b/gcc/testsuite/g++.dg/warn/anonymous-namespace-3.C
> @@ -7,7 +7,8 @@
>  struct B { std::auto_ptr<A> p; };
>
>  #line 10 "foo.C"
> -struct C                  // { dg-warning "uses the anonymous namespace" }
> +struct C // { dg-warning "has internal linkage" "" { target c++11 } }
> +// { dg-warning "uses the anonymous namespace" "" { target c++98_only } .-1 }
>  {
>    std::auto_ptr<A> p;
>  };
> --
> 2.36.1
>


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

* [PATCH] c++: Refer to internal linkage for -Wsubobject-linkage [PR86491]
@ 2022-06-30 16:53 Jonathan Wakely
  2022-07-22 11:20 ` Jonathan Wakely
  2022-07-22 21:30 ` Jason Merrill
  0 siblings, 2 replies; 4+ messages in thread
From: Jonathan Wakely @ 2022-06-30 16:53 UTC (permalink / raw)
  To: gcc-patches

Tested powerpc64le-linux, OK for trunk?

-- >8 --

Since C++11 relaxed the requirement for template arguments to have
external linkage, it's possible to get -Wsubobject-linkage warnings
without using any anonymous namespaces. This confuses users when they
get diagnostics that refer to an anonymous namespace that doesn't exist
in their code.

This changes the diagnostic to say "has internal linkage" for C++11 and
later, which is accurate whether internal linkage is due to the 'static'
specifier, or due to the use of anonymous namespaces.

For C++98 template arguments declared with 'static' are ill-formed
anyway, so the only way this warning can arise is via anonymous
namespaces. That means the existing wording is accurate for C++98 and so
we can keep it.

	PR c++/86491

gcc/cp/ChangeLog:

	* decl2.cc (constrain_class_visibility): Adjust wording of
	-Wsubobject-linkage to account for cases where anonymous
	namespaces aren't used.

gcc/testsuite/ChangeLog:

	* g++.dg/warn/Wsubobject-linkage-3.C: Adjust for new warning.
	* g++.dg/warn/anonymous-namespace-1.C: Use separate dg-warning
	directives for C++98 and everything else.
	* g++.dg/warn/anonymous-namespace-2.C: Likewise.
	* g++.dg/warn/anonymous-namespace-3.C: Likewise.
---
 gcc/cp/decl2.cc                                   | 12 ++++++++++--
 gcc/testsuite/g++.dg/warn/Wsubobject-linkage-3.C  |  4 ++--
 gcc/testsuite/g++.dg/warn/anonymous-namespace-1.C |  8 ++++++--
 gcc/testsuite/g++.dg/warn/anonymous-namespace-2.C |  9 ++++++---
 gcc/testsuite/g++.dg/warn/anonymous-namespace-3.C |  3 ++-
 5 files changed, 26 insertions(+), 10 deletions(-)

diff --git a/gcc/cp/decl2.cc b/gcc/cp/decl2.cc
index 3737e5f010c..de53678715e 100644
--- a/gcc/cp/decl2.cc
+++ b/gcc/cp/decl2.cc
@@ -3027,7 +3027,11 @@ constrain_class_visibility (tree type)
 %qT has a field %qD whose type depends on the type %qT which has no linkage",
 			       type, t, nlt);
 		  }
-		else
+		else if (cxx_dialect > cxx98)
+		  warning (OPT_Wsubobject_linkage, "\
+%qT has a field %qD whose type has internal linkage",
+			   type, t);
+		else // In C++98 this can only happen with unnamed namespaces.
 		  warning (OPT_Wsubobject_linkage, "\
 %qT has a field %qD whose type uses the anonymous namespace",
 			   type, t);
@@ -3062,7 +3066,11 @@ constrain_class_visibility (tree type)
 %qT has a base %qT whose type depends on the type %qT which has no linkage",
 			     type, TREE_TYPE (t), nlt);
 		}
-	      else
+	      else if (cxx_dialect > cxx98)
+		warning (OPT_Wsubobject_linkage, "\
+%qT has a base %qT whose type has internal linkage",
+			 type, TREE_TYPE (t));
+	      else // In C++98 this can only happen with unnamed namespaces.
 		warning (OPT_Wsubobject_linkage, "\
 %qT has a base %qT whose type uses the anonymous namespace",
 			 type, TREE_TYPE (t));
diff --git a/gcc/testsuite/g++.dg/warn/Wsubobject-linkage-3.C b/gcc/testsuite/g++.dg/warn/Wsubobject-linkage-3.C
index 95a04501441..b116fbbb186 100644
--- a/gcc/testsuite/g++.dg/warn/Wsubobject-linkage-3.C
+++ b/gcc/testsuite/g++.dg/warn/Wsubobject-linkage-3.C
@@ -3,7 +3,7 @@
 namespace { struct Foo { }; }
 
 #line 6 "foo.C"
-struct Bar { Foo foo; };   // { dg-warning "anonymous namespace" }
+struct Bar { Foo foo; };   // { dg-warning "anonymous namespace|internal linkage" }
 // { dg-bogus "no linkage" "" { target *-*-* } .-1 }
-struct Bar2 : Foo { };     // { dg-warning "anonymous namespace" }
+struct Bar2 : Foo { };     // { dg-warning "anonymous namespace|internal linkage" }
 // { dg-bogus "no linkage" "" { target *-*-* } .-1 }
diff --git a/gcc/testsuite/g++.dg/warn/anonymous-namespace-1.C b/gcc/testsuite/g++.dg/warn/anonymous-namespace-1.C
index cf193e0cba5..eed3818c5cf 100644
--- a/gcc/testsuite/g++.dg/warn/anonymous-namespace-1.C
+++ b/gcc/testsuite/g++.dg/warn/anonymous-namespace-1.C
@@ -14,5 +14,9 @@ class foobar1
 };
 
 #line 17 "foo.C"
-class foobar : public bad { }; // { dg-warning "uses the anonymous namespace" }
-class foobar2 { bad b; }; // { dg-warning "uses the anonymous namespace" }
+class foobar : public bad { };
+// { dg-warning "has internal linkage" "" { target c++11 } .-1 }
+// { dg-warning "uses the anonymous namespace" "" { target c++98_only } .-2 }
+class foobar2 { bad b; };
+// { dg-warning "has internal linkage" "" { target c++11 } .-1 }
+// { dg-warning "uses the anonymous namespace" "" { target c++98_only } .-2 }
diff --git a/gcc/testsuite/g++.dg/warn/anonymous-namespace-2.C b/gcc/testsuite/g++.dg/warn/anonymous-namespace-2.C
index 4048f3959df..f2ca5915278 100644
--- a/gcc/testsuite/g++.dg/warn/anonymous-namespace-2.C
+++ b/gcc/testsuite/g++.dg/warn/anonymous-namespace-2.C
@@ -18,12 +18,15 @@ struct g3 {
 };
 
 #line 21 "foo.C"
-struct b1 { // { dg-warning "uses the anonymous namespace" }
+struct b1 { // { dg-warning "has internal linkage" "" { target c++11 } }
+// { dg-warning "uses the anonymous namespace" "" { target c++98_only } .-1 }
     bad * B;
 };
-struct b2 { // { dg-warning "uses the anonymous namespace" }
+struct b2 { // { dg-warning "has internal linkage" "" { target c++11 } }
+// { dg-warning "uses the anonymous namespace" "" { target c++98_only } .-1 }
     bad * B[1];
 };
-struct b3 { // { dg-warning "uses the anonymous namespace" }
+struct b3 { // { dg-warning "has internal linkage" "" { target c++11 } }
+// { dg-warning "uses the anonymous namespace" "" { target c++98_only } .-1 }
     bad (*B)[1];
 };
diff --git a/gcc/testsuite/g++.dg/warn/anonymous-namespace-3.C b/gcc/testsuite/g++.dg/warn/anonymous-namespace-3.C
index 8b72abdf5d1..ce5745b25f0 100644
--- a/gcc/testsuite/g++.dg/warn/anonymous-namespace-3.C
+++ b/gcc/testsuite/g++.dg/warn/anonymous-namespace-3.C
@@ -7,7 +7,8 @@
 struct B { std::auto_ptr<A> p; };
 
 #line 10 "foo.C"
-struct C		   // { dg-warning "uses the anonymous namespace" }
+struct C // { dg-warning "has internal linkage" "" { target c++11 } }
+// { dg-warning "uses the anonymous namespace" "" { target c++98_only } .-1 }
 {
   std::auto_ptr<A> p;
 };
-- 
2.36.1


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

end of thread, other threads:[~2022-09-12 16:10 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <c1215d18-73b0-95aa-2c02-485536696a48@redhat.com>
2022-09-12 16:10 ` [PATCH] c++: Refer to internal linkage for -Wsubobject-linkage [PR86491] Jason Merrill
2022-06-30 16:53 Jonathan Wakely
2022-07-22 11:20 ` Jonathan Wakely
2022-07-22 21:30 ` Jason Merrill

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).