diff mbox

[C++,Patch/RFC] PR 53184

Message ID 5549350B.3050504@oracle.com
State New
Headers show

Commit Message

Paolo Carlini May 5, 2015, 9:24 p.m. UTC 
Hi,

per the audit trail, this issue appears to boil down to two separate issues:
- The warning doesn't appear universally useful, thus it would be nice 
to give it a name in order to enable disabling it.
- As shown by the testcase, sometimes the wording is misleading: it 
talks about 'anonymous namespace', where, as clarified by Jason in the 
trail, the issue is really about a type with no linkage, no namespace 
involved.

- The former is easy done, I picked: -Wsubobject-linkage. Makes sense?
- The latter is a little more tricky, because it doesn't seem always 
easy to tell one case from the other, in particular when templates are 
involved (eg, g++.dg/warn/anonymous-namespace-3.C) and the linkage issue 
involves template arguments. Given that the warning doesn't seem 
terribly important (as another data point, clang doesn't have it), so 
far I have conditionals which reliably figure out cases of anonymous 
namespace and cases of no linkage (per the testcase at issue, for 
example) and otherwise fall back to an 'or' wording. I hope the 
improvement is good enough. Alternately, I suppose the warning could use 
a completely different, more generic, wording, but in that case 
testcases like anonymous-namespace-3.C will need adjustment.

Tested x86_64-linux.

Thanks,
Paolo.

////////////////////////////

Comments

Paolo Carlini July 22, 2015, 5:05 p.m. UTC | #1 
Hi,

On 05/05/2015 11:24 PM, Paolo Carlini wrote:
> Hi,
>
> per the audit trail, this issue appears to boil down to two separate 
> issues:
> - The warning doesn't appear universally useful, thus it would be nice 
> to give it a name in order to enable disabling it.
> - As shown by the testcase, sometimes the wording is misleading: it 
> talks about 'anonymous namespace', where, as clarified by Jason in the 
> trail, the issue is really about a type with no linkage, no namespace 
> involved.
>
> - The former is easy done, I picked: -Wsubobject-linkage. Makes sense?
> - The latter is a little more tricky, because it doesn't seem always 
> easy to tell one case from the other, in particular when templates are 
> involved (eg, g++.dg/warn/anonymous-namespace-3.C) and the linkage 
> issue involves template arguments. Given that the warning doesn't seem 
> terribly important (as another data point, clang doesn't have it), so 
> far I have conditionals which reliably figure out cases of anonymous 
> namespace and cases of no linkage (per the testcase at issue, for 
> example) and otherwise fall back to an 'or' wording. I hope the 
> improvement is good enough. Alternately, I suppose the warning could 
> use a completely different, more generic, wording, but in that case 
> testcases like anonymous-namespace-3.C will need adjustment.
Any feedback on this?

     https://gcc.gnu.org/ml/gcc-patches/2015-05/msg00370.html

Thanks!
Paolo.
diff mbox

Patch

Index: c-family/c.opt
===================================================================
--- c-family/c.opt	(revision 222822)
+++ c-family/c.opt	(working copy)
@@ -913,6 +913,11 @@  Wuseless-cast
 C++ ObjC++ Var(warn_useless_cast) Warning
 Warn about useless casts
 
+Wsubobject-linkage
+C++ ObjC++ Var(warn_subobject_linkage) Warning Init(1)
+Warn if a class type has a base or a field whose type uses the anonymous
+namespace or depends on a type with no linkage
+
 ansi
 C ObjC C++ ObjC++
 A synonym for -std=c89 (for C) or -std=c++98 (for C++)
Index: cp/decl2.c
===================================================================
--- cp/decl2.c	(revision 222822)
+++ cp/decl2.c	(working copy)
@@ -2554,9 +2554,22 @@  constrain_class_visibility (tree type)
 	if (subvis == VISIBILITY_ANON)
 	  {
 	    if (!in_main_input_context ())
-	      warning (0, "\
+	      {
+		if (MAYBE_CLASS_TYPE_P (ftype)
+		    && decl_anon_ns_mem_p (TYPE_MAIN_DECL (ftype)))
+		  warning (OPT_Wsubobject_linkage, "\
 %qT has a field %qD whose type uses the anonymous namespace",
 		       type, t);
+		else if (no_linkage_check (ftype, /*relaxed_p=*/false))
+		  warning (OPT_Wsubobject_linkage, "\
+%qT has a field %qD whose type depends on a type with no linkage",
+		       type, t);
+		else
+		  warning (OPT_Wsubobject_linkage, "\
+%qT has a field %qD whose type uses the anonymous namespace or depends on a "
+			   "type with no linkage",
+		       type, t);
+	      }
 	  }
 	else if (MAYBE_CLASS_TYPE_P (ftype)
 		 && vis < VISIBILITY_HIDDEN
@@ -2574,9 +2587,21 @@  constrain_class_visibility (tree type)
       if (subvis == VISIBILITY_ANON)
         {
 	  if (!in_main_input_context())
-	    warning (0, "\
+	    {
+	      if (decl_anon_ns_mem_p (TYPE_MAIN_DECL (TREE_TYPE (t))))
+		warning (OPT_Wsubobject_linkage, "\
 %qT has a base %qT whose type uses the anonymous namespace",
-		     type, TREE_TYPE (t));
+			 type, TREE_TYPE (t));
+	      else if (no_linkage_check (TREE_TYPE (t), /*relaxed_p=*/false))
+		warning (OPT_Wsubobject_linkage, "\
+%qT has a base %qT whose type depends on a type with no linkage",
+			 type, TREE_TYPE (t));
+	      else
+		warning (OPT_Wsubobject_linkage, "\
+%qT has a base %qT whose type uses the anonymous namespace or depends on a "
+			 "type with no linkage",
+			 type, TREE_TYPE (t));
+	    }
 	}
       else if (vis < VISIBILITY_HIDDEN
 	       && subvis >= VISIBILITY_HIDDEN)
Index: doc/invoke.texi
===================================================================
--- doc/invoke.texi	(revision 222822)
+++ doc/invoke.texi	(working copy)
@@ -278,7 +278,7 @@  Objective-C and Objective-C++ Dialects}.
 -Wstrict-aliasing=n @gol -Wstrict-overflow -Wstrict-overflow=@var{n} @gol
 -Wsuggest-attribute=@r{[}pure@r{|}const@r{|}noreturn@r{|}format@r{]} @gol
 -Wsuggest-final-types @gol -Wsuggest-final-methods -Wsuggest-override @gol
--Wmissing-format-attribute @gol
+-Wmissing-format-attribute -Wsubobject-linkage @gol
 -Wswitch  -Wswitch-default  -Wswitch-enum -Wswitch-bool -Wsync-nand @gol
 -Wsystem-headers  -Wtrampolines  -Wtrigraphs  -Wtype-limits  -Wundef @gol
 -Wuninitialized  -Wunknown-pragmas  -Wno-pragmas @gol
@@ -4801,6 +4801,13 @@  types. @option{-Wconversion-null} is enabled by de
 Warn when a literal '0' is used as null pointer constant.  This can
 be useful to facilitate the conversion to @code{nullptr} in C++11.
 
+@item -Wsubobject-linkage @r{(C++ and Objective-C++ only)}
+@opindex Wsubobject-linkage
+@opindex Wno-subobject-linkage
+Warn if a class type has a base or a field whose type uses the anonymous
+namespace or depends on a type with no linkage.  This warning is
+enabled by default.
+
 @item -Wdate-time
 @opindex Wdate-time
 @opindex Wno-date-time
Index: testsuite/g++.dg/warn/Wsubobject-linkage-1.C
===================================================================
--- testsuite/g++.dg/warn/Wsubobject-linkage-1.C	(revision 0)
+++ testsuite/g++.dg/warn/Wsubobject-linkage-1.C	(working copy)
@@ -0,0 +1,9 @@ 
+// PR c++/53184
+
+typedef volatile struct { } Foo;
+
+#line 6 "foo.C"
+struct Bar { Foo foo; };   // { dg-warning "no linkage" }
+// { dg-bogus "anonymous namespace" "" { target *-*-* } 6 }
+struct Bar2 : Foo { };     // { dg-warning "no linkage" }
+// { dg-bogus "anonymous namespace" "" { target *-*-* } 8 }
Index: testsuite/g++.dg/warn/Wsubobject-linkage-2.C
===================================================================
--- testsuite/g++.dg/warn/Wsubobject-linkage-2.C	(revision 0)
+++ testsuite/g++.dg/warn/Wsubobject-linkage-2.C	(working copy)
@@ -0,0 +1,8 @@ 
+// PR c++/53184
+// { dg-options "-Wno-subobject-linkage" }
+
+typedef volatile struct { } Foo;
+
+#line 7 "foo.C"
+struct Bar { Foo foo; };
+struct Bar2 : Foo { };
Index: testsuite/g++.dg/warn/Wsubobject-linkage-3.C
===================================================================
--- testsuite/g++.dg/warn/Wsubobject-linkage-3.C	(revision 0)
+++ testsuite/g++.dg/warn/Wsubobject-linkage-3.C	(working copy)
@@ -0,0 +1,9 @@ 
+// PR c++/53184
+
+namespace { struct Foo { }; }
+
+#line 6 "foo.C"
+struct Bar { Foo foo; };   // { dg-warning "anonymous namespace" }
+// { dg-bogus "no linkage" "" { target *-*-* } 6 }
+struct Bar2 : Foo { };     // { dg-warning "anonymous namespace" }
+// { dg-bogus "no linkage" "" { target *-*-* } 8 }
Index: testsuite/g++.dg/warn/Wsubobject-linkage-4.C
===================================================================
--- testsuite/g++.dg/warn/Wsubobject-linkage-4.C	(revision 0)
+++ testsuite/g++.dg/warn/Wsubobject-linkage-4.C	(working copy)
@@ -0,0 +1,8 @@ 
+// PR c++/53184
+// { dg-options "-Wno-subobject-linkage" }
+
+namespace { struct Foo { }; }
+
+#line 7 "foo.C"
+struct Bar { Foo foo; };
+struct Bar2 : Foo { };