| Message ID | Y1An8APGvWejfjHX@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] c++: Mitigate -Wuseless-cast with classes [PR85043] | expand |
On 10/19/22 12:38, Marek Polacek wrote: > On Wed, Oct 19, 2022 at 09:27:27AM -0400, Jason Merrill wrote: >> On 10/18/22 13:38, Marek Polacek wrote: >>> -Wuseless-cast (not part of -Wall/-Wextra) warns here: >>> >>> struct S { }; >>> void g (S&&); >>> void f (S&& arg) >>> { >>> g (S(arg)); // warning: useless cast to type 'struct S' >>> } >>> >>> which is wrong: the code will not compile without the cast because >>> "arg" is an lvalue which cannot bind to S&&. >>> >>> I'd like to disable the warning when a class object is cast to >>> a non-reference type, which seems like a minimal patch that fixes the >>> problems reported in our Bugzilla. Of course, the cast in "(int)i" >>> may not be useless, either, but I'm not changing that here. >> >> As I commented on PR 14710, "I wouldn't warn about a cast that changes the >> type or value category of an expression at all...." > > Ah, I didn't see that comment. > >> The code currently checks the value category when casting to a reference, >> but not when casting to a non-reference; that seems like the thing to fix. > > OK, how about this? The nice thing is that it handles non-classes as well, > while we still warn about truly useless casts as in "X(X{})". Thanks, > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? > > -- >8 -- > -Wuseless-cast (not part of -Wall/-Wextra) warns here: > > struct S { }; > void g (S&&); > void f (S&& arg) > { > g (S(arg)); // warning: useless cast to type 'struct S' > } > > which is wrong: the code will not compile without the cast because > "arg" is an lvalue which cannot bind to S&&. > > This patch disables the warning when an object that isn't a prvalue > is cast to a non-reference type. Therefore we still warn about the > useless cast in "X(X{})". > > PR c++/85043 > > gcc/cp/ChangeLog: > > * typeck.cc (maybe_warn_about_useless_cast): Don't warn when > a glvalue is cast to a non-reference type. > > gcc/ChangeLog: > > * doc/invoke.texi: Update documentation of -Wuseless-cast. > > gcc/testsuite/ChangeLog: > > * g++.dg/warn/Wuseless-cast.C: Remove dg-warning. > * g++.dg/warn/Wuseless-cast3.C: New test. > --- > gcc/cp/typeck.cc | 4 ++- > gcc/doc/invoke.texi | 13 +++++++++- > gcc/testsuite/g++.dg/warn/Wuseless-cast.C | 12 ++++----- > gcc/testsuite/g++.dg/warn/Wuseless-cast3.C | 29 ++++++++++++++++++++++ > 4 files changed, 50 insertions(+), 8 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/warn/Wuseless-cast3.C > > diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc > index da0e1427b97..8e1d14ee6b0 100644 > --- a/gcc/cp/typeck.cc > +++ b/gcc/cp/typeck.cc > @@ -8108,7 +8108,9 @@ maybe_warn_about_useless_cast (location_t loc, tree type, tree expr, > && (TYPE_REF_IS_RVALUE (type) > ? xvalue_p (expr) : lvalue_p (expr)) > && same_type_p (TREE_TYPE (expr), TREE_TYPE (type))) > - || same_type_p (TREE_TYPE (expr), type)) > + /* Don't warn when converting a class object to a non-reference type, > + because that's a common way to create a temporary. */ > + || (!glvalue_p (expr) && same_type_p (TREE_TYPE (expr), type))) Might use ?: instead of || so we check this only when !TYPE_REF_P. OK with that change. > warning_at (loc, OPT_Wuseless_cast, > "useless cast to type %q#T", type); > } > diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi > index c176e2dc646..cd4d3c1d72c 100644 > --- a/gcc/doc/invoke.texi > +++ b/gcc/doc/invoke.texi > @@ -4551,7 +4551,18 @@ pointers after reallocation. > @item -Wuseless-cast @r{(C++ and Objective-C++ only)} > @opindex Wuseless-cast > @opindex Wno-useless-cast > -Warn when an expression is casted to its own type. > +Warn when an expression is cast to its own type. This warning does not > +occur when a class object is converted to a non-reference type as that > +is a way to create a temporary: > + > +@smallexample > +struct S @{ @}; > +void g (S&&); > +void f (S&& arg) > +@{ > + g (S(arg)); // make arg prvalue so that it can bind to S&& > +@} > +@end smallexample > > @item -Wno-conversion-null @r{(C++ and Objective-C++ only)} > @opindex Wconversion-null > diff --git a/gcc/testsuite/g++.dg/warn/Wuseless-cast.C b/gcc/testsuite/g++.dg/warn/Wuseless-cast.C > index 2fd6bc45102..d7cb89930a6 100644 > --- a/gcc/testsuite/g++.dg/warn/Wuseless-cast.C > +++ b/gcc/testsuite/g++.dg/warn/Wuseless-cast.C > @@ -62,11 +62,11 @@ A prvalue(); > > void f() > { > - int n; > + int n; > > - (int)(n); // { dg-warning "3:useless cast" } > - static_cast<int>(n); // { dg-warning "3:useless cast" } > - reinterpret_cast<int>(n); // { dg-warning "3:useless cast" } > + (int)(n); > + static_cast<int>(n); > + reinterpret_cast<int>(n); > > (int*)(&n); // { dg-warning "3:useless cast" } > const_cast<int*>(&n); // { dg-warning "3:useless cast" } > @@ -100,8 +100,8 @@ void f() > > A a; > > - (A)(a); // { dg-warning "3:useless cast" } > - static_cast<A>(a); // { dg-warning "3:useless cast" } > + (A)(a); > + static_cast<A>(a); > > (A*)(&a); // { dg-warning "3:useless cast" } > const_cast<A*>(&a); // { dg-warning "3:useless cast" } > diff --git a/gcc/testsuite/g++.dg/warn/Wuseless-cast3.C b/gcc/testsuite/g++.dg/warn/Wuseless-cast3.C > new file mode 100644 > index 00000000000..43dd2ebfaea > --- /dev/null > +++ b/gcc/testsuite/g++.dg/warn/Wuseless-cast3.C > @@ -0,0 +1,29 @@ > +// PR c++/85043 > +// { dg-do compile { target c++11 } } > +// { dg-options "-Wuseless-cast" } > + > +struct S { int s; void bump () { s++; } }; > + > +void > +foo () > +{ > + S s = { 1 }; > + s.bump (); > + S (s).bump (); // { dg-bogus "useless" } > + ((S) s).bump (); // { dg-bogus "useless" } > + static_cast<S> (s).bump (); // { dg-bogus "useless" } > +} > + > +struct X { }; > +void g(X&&); > + > +void > +f (X&& arg) > +{ > + g(X(arg)); // { dg-bogus "useless" } > + g(X(X{})); // { dg-warning "useless" } > + g(static_cast<X&&>(arg)); > + > + int i = (int) 1; // { dg-warning "useless" } > + const int &r = (int) i; // { dg-bogus "useless" } > +} > > base-commit: 69a233610f6b27cd4283561569d8ce0f35044dc4
On Wed, Oct 19, 2022 at 02:37:23PM -0400, Jason Merrill wrote: > On 10/19/22 12:38, Marek Polacek wrote: > > On Wed, Oct 19, 2022 at 09:27:27AM -0400, Jason Merrill wrote: > > > On 10/18/22 13:38, Marek Polacek wrote: > > > > -Wuseless-cast (not part of -Wall/-Wextra) warns here: > > > > > > > > struct S { }; > > > > void g (S&&); > > > > void f (S&& arg) > > > > { > > > > g (S(arg)); // warning: useless cast to type 'struct S' > > > > } > > > > > > > > which is wrong: the code will not compile without the cast because > > > > "arg" is an lvalue which cannot bind to S&&. > > > > > > > > I'd like to disable the warning when a class object is cast to > > > > a non-reference type, which seems like a minimal patch that fixes the > > > > problems reported in our Bugzilla. Of course, the cast in "(int)i" > > > > may not be useless, either, but I'm not changing that here. > > > > > > As I commented on PR 14710, "I wouldn't warn about a cast that changes the > > > type or value category of an expression at all...." > > > > Ah, I didn't see that comment. > > > > > The code currently checks the value category when casting to a reference, > > > but not when casting to a non-reference; that seems like the thing to fix. > > > > OK, how about this? The nice thing is that it handles non-classes as well, > > while we still warn about truly useless casts as in "X(X{})". Thanks, > > > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? > > > > -- >8 -- > > -Wuseless-cast (not part of -Wall/-Wextra) warns here: > > > > struct S { }; > > void g (S&&); > > void f (S&& arg) > > { > > g (S(arg)); // warning: useless cast to type 'struct S' > > } > > > > which is wrong: the code will not compile without the cast because > > "arg" is an lvalue which cannot bind to S&&. > > > > This patch disables the warning when an object that isn't a prvalue > > is cast to a non-reference type. Therefore we still warn about the > > useless cast in "X(X{})". > > > > PR c++/85043 > > > > gcc/cp/ChangeLog: > > > > * typeck.cc (maybe_warn_about_useless_cast): Don't warn when > > a glvalue is cast to a non-reference type. > > > > gcc/ChangeLog: > > > > * doc/invoke.texi: Update documentation of -Wuseless-cast. > > > > gcc/testsuite/ChangeLog: > > > > * g++.dg/warn/Wuseless-cast.C: Remove dg-warning. > > * g++.dg/warn/Wuseless-cast3.C: New test. > > --- > > gcc/cp/typeck.cc | 4 ++- > > gcc/doc/invoke.texi | 13 +++++++++- > > gcc/testsuite/g++.dg/warn/Wuseless-cast.C | 12 ++++----- > > gcc/testsuite/g++.dg/warn/Wuseless-cast3.C | 29 ++++++++++++++++++++++ > > 4 files changed, 50 insertions(+), 8 deletions(-) > > create mode 100644 gcc/testsuite/g++.dg/warn/Wuseless-cast3.C > > > > diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc > > index da0e1427b97..8e1d14ee6b0 100644 > > --- a/gcc/cp/typeck.cc > > +++ b/gcc/cp/typeck.cc > > @@ -8108,7 +8108,9 @@ maybe_warn_about_useless_cast (location_t loc, tree type, tree expr, > > && (TYPE_REF_IS_RVALUE (type) > > ? xvalue_p (expr) : lvalue_p (expr)) > > && same_type_p (TREE_TYPE (expr), TREE_TYPE (type))) > > - || same_type_p (TREE_TYPE (expr), type)) > > + /* Don't warn when converting a class object to a non-reference type, > > + because that's a common way to create a temporary. */ > > + || (!glvalue_p (expr) && same_type_p (TREE_TYPE (expr), type))) > > Might use ?: instead of || so we check this only when !TYPE_REF_P. OK with > that change. Ah nice, pushed with this instead of the ||: if (TYPE_REF_P (type) ? ((TYPE_REF_IS_RVALUE (type) ? xvalue_p (expr) : lvalue_p (expr)) && same_type_p (TREE_TYPE (expr), TREE_TYPE (type))) /* Don't warn when converting a class object to a non-reference type, because that's a common way to create a temporary. */ : (!glvalue_p (expr) && same_type_p (TREE_TYPE (expr), type))) Thanks, Marek
diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc index da0e1427b97..8e1d14ee6b0 100644 --- a/gcc/cp/typeck.cc +++ b/gcc/cp/typeck.cc @@ -8108,7 +8108,9 @@ maybe_warn_about_useless_cast (location_t loc, tree type, tree expr, && (TYPE_REF_IS_RVALUE (type) ? xvalue_p (expr) : lvalue_p (expr)) && same_type_p (TREE_TYPE (expr), TREE_TYPE (type))) - || same_type_p (TREE_TYPE (expr), type)) + /* Don't warn when converting a class object to a non-reference type, + because that's a common way to create a temporary. */ + || (!glvalue_p (expr) && same_type_p (TREE_TYPE (expr), type))) warning_at (loc, OPT_Wuseless_cast, "useless cast to type %q#T", type); } diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index c176e2dc646..cd4d3c1d72c 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -4551,7 +4551,18 @@ pointers after reallocation. @item -Wuseless-cast @r{(C++ and Objective-C++ only)} @opindex Wuseless-cast @opindex Wno-useless-cast -Warn when an expression is casted to its own type. +Warn when an expression is cast to its own type. This warning does not +occur when a class object is converted to a non-reference type as that +is a way to create a temporary: + +@smallexample +struct S @{ @}; +void g (S&&); +void f (S&& arg) +@{ + g (S(arg)); // make arg prvalue so that it can bind to S&& +@} +@end smallexample @item -Wno-conversion-null @r{(C++ and Objective-C++ only)} @opindex Wconversion-null diff --git a/gcc/testsuite/g++.dg/warn/Wuseless-cast.C b/gcc/testsuite/g++.dg/warn/Wuseless-cast.C index 2fd6bc45102..d7cb89930a6 100644 --- a/gcc/testsuite/g++.dg/warn/Wuseless-cast.C +++ b/gcc/testsuite/g++.dg/warn/Wuseless-cast.C @@ -62,11 +62,11 @@ A prvalue(); void f() { - int n; + int n; - (int)(n); // { dg-warning "3:useless cast" } - static_cast<int>(n); // { dg-warning "3:useless cast" } - reinterpret_cast<int>(n); // { dg-warning "3:useless cast" } + (int)(n); + static_cast<int>(n); + reinterpret_cast<int>(n); (int*)(&n); // { dg-warning "3:useless cast" } const_cast<int*>(&n); // { dg-warning "3:useless cast" } @@ -100,8 +100,8 @@ void f() A a; - (A)(a); // { dg-warning "3:useless cast" } - static_cast<A>(a); // { dg-warning "3:useless cast" } + (A)(a); + static_cast<A>(a); (A*)(&a); // { dg-warning "3:useless cast" } const_cast<A*>(&a); // { dg-warning "3:useless cast" } diff --git a/gcc/testsuite/g++.dg/warn/Wuseless-cast3.C b/gcc/testsuite/g++.dg/warn/Wuseless-cast3.C new file mode 100644 index 00000000000..43dd2ebfaea --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Wuseless-cast3.C @@ -0,0 +1,29 @@ +// PR c++/85043 +// { dg-do compile { target c++11 } } +// { dg-options "-Wuseless-cast" } + +struct S { int s; void bump () { s++; } }; + +void +foo () +{ + S s = { 1 }; + s.bump (); + S (s).bump (); // { dg-bogus "useless" } + ((S) s).bump (); // { dg-bogus "useless" } + static_cast<S> (s).bump (); // { dg-bogus "useless" } +} + +struct X { }; +void g(X&&); + +void +f (X&& arg) +{ + g(X(arg)); // { dg-bogus "useless" } + g(X(X{})); // { dg-warning "useless" } + g(static_cast<X&&>(arg)); + + int i = (int) 1; // { dg-warning "useless" } + const int &r = (int) i; // { dg-bogus "useless" } +}