c++: Refer to internal linkage for -Wsubobject-linkage [PR86491]

Message ID 20220630165326.1253733-1-jwakely@redhat.com
State Superseded
Headers
Series c++: Refer to internal linkage for -Wsubobject-linkage [PR86491] |

Commit Message

Jonathan Wakely June 30, 2022, 4:53 p.m. UTC
  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(-)
  

Comments

Jonathan Wakely July 22, 2022, 11:20 a.m. UTC | #1
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
>
  
Jason Merrill July 22, 2022, 9:30 p.m. UTC | #2
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;
>   };
  

Patch

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;
 };