| Message ID | 55F04BBC.4020809@oracle.com |
|---|---|
| State | New |
| Headers | show |
OK, thanks. Jason
On 09/09/2015 05:09 PM, Paolo Carlini wrote: > +@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. “and the class type does not itself use the anonymous namespace or has no linkage”? > This warning is > +enabled by default. Maybe add a sentence why this is bad? I can only guess, but I suspect the reason is this: Such types are necessarily specific to a single translation unit because any definition in another translation unit would be an ODR violation, so they can be put into the anonymous namespace themselves.
Hi Florian, On 09/14/2015 09:41 PM, Florian Weimer wrote: > This warning is > +enabled by default. > Maybe add a sentence why this is bad? I can only guess, but I suspect > the reason is this: Such types are necessarily specific to a single > translation unit because any definition in another translation unit > would be an ODR violation, so they can be put into the anonymous > namespace themselves. As I probably mentioned somewhere, GCC is the only compiler I have at hand implementing something similar: frankly, I'm not sure how exactly we want to put it, concisely and neatly at the same time. If you are willing to prepare something more concrete, I'm sure Jason would be happy to review it! Thanks, Paolo.
On 09/14/2015 06:17 PM, Paolo Carlini wrote: > Hi Florian, > > On 09/14/2015 09:41 PM, Florian Weimer wrote: >> This warning is >> +enabled by default. >> Maybe add a sentence why this is bad? I can only guess, but I suspect >> the reason is this: Such types are necessarily specific to a single >> translation unit because any definition in another translation unit >> would be an ODR violation, so they can be put into the anonymous >> namespace themselves. > As I probably mentioned somewhere, GCC is the only compiler I have at > hand implementing something similar: frankly, I'm not sure how exactly > we want to put it, concisely and neatly at the same time. If you are > willing to prepare something more concrete, I'm sure Jason would be > happy to review it! Florian's summary is correct. If a type A depends on a type B with no or internal linkage, defining it in multiple translation units would be an ODR violation because the meaning of B is different in each translation unit. If A only appears in a single translation unit, the best way to silence the warning is to give it internal linkage by putting it in an anonymous namespace as well. The compiler doesn't give this warning for types defined in the main .C file, as those are unlikely to have multiple definitions. Jason
Hi, On 09/15/2015 03:11 AM, Jason Merrill wrote: > On 09/14/2015 06:17 PM, Paolo Carlini wrote: >> Hi Florian, >> >> On 09/14/2015 09:41 PM, Florian Weimer wrote: >>> This warning is >>> +enabled by default. >>> Maybe add a sentence why this is bad? I can only guess, but I suspect >>> the reason is this: Such types are necessarily specific to a single >>> translation unit because any definition in another translation unit >>> would be an ODR violation, so they can be put into the anonymous >>> namespace themselves. >> As I probably mentioned somewhere, GCC is the only compiler I have at >> hand implementing something similar: frankly, I'm not sure how exactly >> we want to put it, concisely and neatly at the same time. If you are >> willing to prepare something more concrete, I'm sure Jason would be >> happy to review it! > > Florian's summary is correct. > > If a type A depends on a type B with no or internal linkage, defining > it in multiple translation units would be an ODR violation because the > meaning of B is different in each translation unit. If A only appears > in a single translation unit, the best way to silence the warning is > to give it internal linkage by putting it in an anonymous namespace as > well. The compiler doesn't give this warning for types defined in the > main .C file, as those are unlikely to have multiple definitions. Thanks. Then I guess I'm going to simply add to invoke.texi the above as-is: if somebody thinks it's too long and sees a way to shorten it without loosing important details, please speak asap ;) Paolo.
Index: c-family/c.opt =================================================================== --- c-family/c.opt (revision 227586) +++ c-family/c.opt (working copy) @@ -944,6 +944,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 227586) +++ cp/decl2.c (working copy) @@ -2564,10 +2564,25 @@ constrain_class_visibility (tree type) if (subvis == VISIBILITY_ANON) { - if (!in_main_input_context ()) - warning (0, "\ + if (!in_main_input_context()) + { + tree nlt = no_linkage_check (ftype, /*relaxed_p=*/false); + if (nlt) + { + if (same_type_p (TREE_TYPE (t), nlt)) + warning (OPT_Wsubobject_linkage, "\ +%qT has a field %qD 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 + warning (OPT_Wsubobject_linkage, "\ %qT has a field %qD whose type uses the anonymous namespace", - type, t); + type, t); + } } else if (MAYBE_CLASS_TYPE_P (ftype) && vis < VISIBILITY_HIDDEN @@ -2585,9 +2600,24 @@ constrain_class_visibility (tree type) if (subvis == VISIBILITY_ANON) { if (!in_main_input_context()) - warning (0, "\ + { + tree nlt = no_linkage_check (TREE_TYPE (t), /*relaxed_p=*/false); + if (nlt) + { + if (same_type_p (TREE_TYPE (t), nlt)) + warning (OPT_Wsubobject_linkage, "\ +%qT has a base %qT whose type has no linkage", + type, TREE_TYPE (t)); + 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); + } + else + 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 (vis < VISIBILITY_HIDDEN && subvis >= VISIBILITY_HIDDEN) Index: doc/invoke.texi =================================================================== --- doc/invoke.texi (revision 227586) +++ doc/invoke.texi (working copy) @@ -282,7 +282,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 -Wtautological-compare -Wtrampolines -Wtrigraphs @gol -Wtype-limits -Wundef @gol @@ -4923,6 +4923,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 { };