| Message ID | 20180919034751.GM5587@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | C++ PATCH for c++/87357, missing -Wconversion warning | expand |
On 09/18/2018 09:47 PM, Marek Polacek wrote: > As I mentioned in the PR, [class.conv.fct] says "A conversion function is never > used to convert a (possibly cv-qualified) object to the (possibly cv-qualified) > same object type (or a reference to it), to a (possibly cv-qualified) base class > of that type (or a reference to it), or to (possibly cv-qualified) void." but I > noticed we weren't warning about > > struct X { > operator X(); > }; > > (and there are no tests for this case) because comparing types directly only > works for TYPE_CANONICAL, otherwise we have to use same_type_*. > > I'm also changing the warning about 'void' since we can't form a reference to > void. Since you're changing the text of the void warning it would also be nice to add quoting around the type name if it doesn't involve adjusting too many tests. But... ... > +struct B { }; > + > +struct X : public B { > + operator X(); // { dg-warning "3:conversion to the same type will never use a type conversion operator" } Is the operator function mentioned in the warning? If not, mentioning it would make it clearer which of a set of operators is not going to be used (when there are multiple like in this test) and also make it easier to distinguish one from the others if they're on the same line (e.g., if the class is the result of macro expansion). Martin > + operator X&(); // { dg-warning "3:conversion to a reference to the same type will never use a type conversion operator" } > + operator X() const; // { dg-warning "3:conversion to the same type will never use a type conversion operator" } > + operator const X(); // { dg-warning "3:conversion to the same type will never use a type conversion operator" } > + > + operator B(); // { dg-warning "3:conversion to a base class will never use a type conversion operator" } > + operator B&(); // { dg-warning "3:conversion to a reference to a base class will never use a type conversion operator" } > + operator B() const; // { dg-warning "3:conversion to a base class will never use a type conversion operator" } > + operator const B(); // { dg-warning "3:conversion to a base class will never use a type conversion operator" } > + > + operator void(); // { dg-warning "3:conversion to void will never use a type conversion operator" } > + operator void() const; // { dg-warning "3:conversion to void will never use a type conversion operator" } > +}; >
On Wed, Sep 19, 2018 at 09:51:44AM -0600, Martin Sebor wrote: > On 09/18/2018 09:47 PM, Marek Polacek wrote: > > As I mentioned in the PR, [class.conv.fct] says "A conversion function is never > > used to convert a (possibly cv-qualified) object to the (possibly cv-qualified) > > same object type (or a reference to it), to a (possibly cv-qualified) base class > > of that type (or a reference to it), or to (possibly cv-qualified) void." but I > > noticed we weren't warning about > > > > struct X { > > operator X(); > > }; > > > > (and there are no tests for this case) because comparing types directly only > > works for TYPE_CANONICAL, otherwise we have to use same_type_*. > > > > I'm also changing the warning about 'void' since we can't form a reference to > > void. > > Since you're changing the text of the void warning it would also > be nice to add quoting around the type name if it doesn't involve > adjusting too many tests. But... Only two tests besides the new one trigger this warning, so that would be trivial. > ... > > +struct B { }; > > + > > +struct X : public B { > > + operator X(); // { dg-warning "3:conversion to the same type will never use a type conversion operator" } > > Is the operator function mentioned in the warning? Nope, just what the dg-warning says. > If not, mentioning it would make it clearer which of a set of > operators is not going to be used (when there are multiple like > in this test) and also make it easier to distinguish one from > the others if they're on the same line (e.g., if the class is > the result of macro expansion). Good idea. Maybe let's commit this first (a bugfix) and then I can add the types printing (an enhancement)? Marek
On Tue, Sep 18, 2018 at 11:47 PM, Marek Polacek <polacek@redhat.com> wrote: > As I mentioned in the PR, [class.conv.fct] says "A conversion function is never > used to convert a (possibly cv-qualified) object to the (possibly cv-qualified) > same object type (or a reference to it), to a (possibly cv-qualified) base class > of that type (or a reference to it), or to (possibly cv-qualified) void." but I > noticed we weren't warning about > > struct X { > operator X(); > }; > > (and there are no tests for this case) because comparing types directly only > works for TYPE_CANONICAL, otherwise we have to use same_type_*. > > I'm also changing the warning about 'void' since we can't form a reference to > void. > > I'll also note that I think we should warn about this by default, not only > when -Wconversion is in effect. Agreed, this should get a new flag that is on by default. > Bootstrapped/regtested on x86_64-linux, ok for trunk? OK. > 2018-09-18 Marek Polacek <polacek@redhat.com> > > PR c++/87357 - missing -Wconversion warning > * decl.c (grok_op_properties): Remove diagnostic parts mentioning > a conversion to a reference to void. Use > same_type_ignoring_top_level_qualifiers_p rather than comparing types > directly. > > * g++.dg/warn/Wconversion5.C: New test. > > diff --git gcc/cp/decl.c gcc/cp/decl.c > index 827c1720335..503b433cbd1 100644 > --- gcc/cp/decl.c > +++ gcc/cp/decl.c > @@ -13553,15 +13553,11 @@ grok_op_properties (tree decl, bool complain) > t = TYPE_MAIN_VARIANT (TREE_TYPE (t)); > > if (VOID_TYPE_P (t)) > - warning_at (loc, OPT_Wconversion, > - ref > - ? G_("conversion to a reference to void " > - "will never use a type conversion operator") > - : G_("conversion to void " > - "will never use a type conversion operator")); > + warning_at (loc, OPT_Wconversion, "conversion to void " > + "will never use a type conversion operator"); > else if (class_type) > { > - if (t == class_type) > + if (same_type_ignoring_top_level_qualifiers_p (t, class_type)) > warning_at (loc, OPT_Wconversion, > ref > ? G_("conversion to a reference to the same type " > diff --git gcc/testsuite/g++.dg/warn/Wconversion5.C gcc/testsuite/g++.dg/warn/Wconversion5.C > index e69de29bb2d..00b1ddab188 100644 > --- gcc/testsuite/g++.dg/warn/Wconversion5.C > +++ gcc/testsuite/g++.dg/warn/Wconversion5.C > @@ -0,0 +1,20 @@ > +// PR c++/87357 > +// { dg-do compile } > +// { dg-options "-Wconversion" } > + > +struct B { }; > + > +struct X : public B { > + operator X(); // { dg-warning "3:conversion to the same type will never use a type conversion operator" } > + operator X&(); // { dg-warning "3:conversion to a reference to the same type will never use a type conversion operator" } > + operator X() const; // { dg-warning "3:conversion to the same type will never use a type conversion operator" } > + operator const X(); // { dg-warning "3:conversion to the same type will never use a type conversion operator" } > + > + operator B(); // { dg-warning "3:conversion to a base class will never use a type conversion operator" } > + operator B&(); // { dg-warning "3:conversion to a reference to a base class will never use a type conversion operator" } > + operator B() const; // { dg-warning "3:conversion to a base class will never use a type conversion operator" } > + operator const B(); // { dg-warning "3:conversion to a base class will never use a type conversion operator" } > + > + operator void(); // { dg-warning "3:conversion to void will never use a type conversion operator" } > + operator void() const; // { dg-warning "3:conversion to void will never use a type conversion operator" } > +};
On 09/19/2018 10:02 AM, Marek Polacek wrote: > On Wed, Sep 19, 2018 at 09:51:44AM -0600, Martin Sebor wrote: >> On 09/18/2018 09:47 PM, Marek Polacek wrote: >>> As I mentioned in the PR, [class.conv.fct] says "A conversion function is never >>> used to convert a (possibly cv-qualified) object to the (possibly cv-qualified) >>> same object type (or a reference to it), to a (possibly cv-qualified) base class >>> of that type (or a reference to it), or to (possibly cv-qualified) void." but I >>> noticed we weren't warning about >>> >>> struct X { >>> operator X(); >>> }; >>> >>> (and there are no tests for this case) because comparing types directly only >>> works for TYPE_CANONICAL, otherwise we have to use same_type_*. >>> >>> I'm also changing the warning about 'void' since we can't form a reference to >>> void. >> >> Since you're changing the text of the void warning it would also >> be nice to add quoting around the type name if it doesn't involve >> adjusting too many tests. But... > > Only two tests besides the new one trigger this warning, so that would be > trivial. > >> ... >>> +struct B { }; >>> + >>> +struct X : public B { >>> + operator X(); // { dg-warning "3:conversion to the same type will never use a type conversion operator" } >> >> Is the operator function mentioned in the warning? > > Nope, just what the dg-warning says. > >> If not, mentioning it would make it clearer which of a set of >> operators is not going to be used (when there are multiple like >> in this test) and also make it easier to distinguish one from >> the others if they're on the same line (e.g., if the class is >> the result of macro expansion). > > Good idea. Maybe let's commit this first (a bugfix) and then I can add > the types printing (an enhancement)? Sure, whatever works for you. Martin
diff --git gcc/cp/decl.c gcc/cp/decl.c index 827c1720335..503b433cbd1 100644 --- gcc/cp/decl.c +++ gcc/cp/decl.c @@ -13553,15 +13553,11 @@ grok_op_properties (tree decl, bool complain) t = TYPE_MAIN_VARIANT (TREE_TYPE (t)); if (VOID_TYPE_P (t)) - warning_at (loc, OPT_Wconversion, - ref - ? G_("conversion to a reference to void " - "will never use a type conversion operator") - : G_("conversion to void " - "will never use a type conversion operator")); + warning_at (loc, OPT_Wconversion, "conversion to void " + "will never use a type conversion operator"); else if (class_type) { - if (t == class_type) + if (same_type_ignoring_top_level_qualifiers_p (t, class_type)) warning_at (loc, OPT_Wconversion, ref ? G_("conversion to a reference to the same type " diff --git gcc/testsuite/g++.dg/warn/Wconversion5.C gcc/testsuite/g++.dg/warn/Wconversion5.C index e69de29bb2d..00b1ddab188 100644 --- gcc/testsuite/g++.dg/warn/Wconversion5.C +++ gcc/testsuite/g++.dg/warn/Wconversion5.C @@ -0,0 +1,20 @@ +// PR c++/87357 +// { dg-do compile } +// { dg-options "-Wconversion" } + +struct B { }; + +struct X : public B { + operator X(); // { dg-warning "3:conversion to the same type will never use a type conversion operator" } + operator X&(); // { dg-warning "3:conversion to a reference to the same type will never use a type conversion operator" } + operator X() const; // { dg-warning "3:conversion to the same type will never use a type conversion operator" } + operator const X(); // { dg-warning "3:conversion to the same type will never use a type conversion operator" } + + operator B(); // { dg-warning "3:conversion to a base class will never use a type conversion operator" } + operator B&(); // { dg-warning "3:conversion to a reference to a base class will never use a type conversion operator" } + operator B() const; // { dg-warning "3:conversion to a base class will never use a type conversion operator" } + operator const B(); // { dg-warning "3:conversion to a base class will never use a type conversion operator" } + + operator void(); // { dg-warning "3:conversion to void will never use a type conversion operator" } + operator void() const; // { dg-warning "3:conversion to void will never use a type conversion operator" } +};
As I mentioned in the PR, [class.conv.fct] says "A conversion function is never used to convert a (possibly cv-qualified) object to the (possibly cv-qualified) same object type (or a reference to it), to a (possibly cv-qualified) base class of that type (or a reference to it), or to (possibly cv-qualified) void." but I noticed we weren't warning about struct X { operator X(); }; (and there are no tests for this case) because comparing types directly only works for TYPE_CANONICAL, otherwise we have to use same_type_*. I'm also changing the warning about 'void' since we can't form a reference to void. I'll also note that I think we should warn about this by default, not only when -Wconversion is in effect. Bootstrapped/regtested on x86_64-linux, ok for trunk? 2018-09-18 Marek Polacek <polacek@redhat.com> PR c++/87357 - missing -Wconversion warning * decl.c (grok_op_properties): Remove diagnostic parts mentioning a conversion to a reference to void. Use same_type_ignoring_top_level_qualifiers_p rather than comparing types directly. * g++.dg/warn/Wconversion5.C: New test.