| Message ID | 20200221191426.207169-1-polacek@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | c++: Implement P1957R2, T* to bool should be considered narrowing. | expand |
On Fri, Feb 21, 2020 at 02:14:26PM -0500, Marek Polacek wrote: > This was approved in the Prague 2020 WG21 meeting so let's adjust the > comment. Since it's supposed to be a DR I think we should no longer > limit it to C++20. Which is what clang++ trunk does. Marek
On 2/21/20 2:14 PM, Marek Polacek wrote: > This was approved in the Prague 2020 WG21 meeting so let's adjust the > comment. Since it's supposed to be a DR I think we should no longer > limit it to C++20. I'm a bit nervous about the impact, but OK. It's easy enough to turn off -Wnarrowing if it's a problem for users. > Bootstrapped/regtested on x86_64-linux, ok for trunk? > > 2020-02-21 Marek Polacek <polacek@redhat.com> > > P1957R2 > * typeck2.c (check_narrowing): Consider T* to bool narrowing > in C++11 and up. > > * g++.dg/cpp0x/initlist92.C: Don't expect an error in C++20 only. > --- > gcc/cp/typeck2.c | 7 ++++--- > gcc/testsuite/g++.dg/cpp0x/initlist92.C | 2 +- > 2 files changed, 5 insertions(+), 4 deletions(-) > > diff --git a/gcc/cp/typeck2.c b/gcc/cp/typeck2.c > index 48920894b3b..68bc2e5c170 100644 > --- a/gcc/cp/typeck2.c > +++ b/gcc/cp/typeck2.c > @@ -1036,9 +1036,10 @@ check_narrowing (tree type, tree init, tsubst_flags_t complain, > } > else if (TREE_CODE (type) == BOOLEAN_TYPE > && (TYPE_PTR_P (ftype) || TYPE_PTRMEM_P (ftype))) > - /* This hasn't actually made it into C++20 yet, but let's add it now to get > - an idea of the impact. */ > - ok = (cxx_dialect < cxx2a); > + /* C++20 P1957R2: converting from a pointer type or a pointer-to-member > + type to bool should be considered narrowing. This is a DR so is not > + limited to C++20 only. */ > + ok = false; > > bool almost_ok = ok; > if (!ok && !CONSTANT_CLASS_P (init) && (complain & tf_warning_or_error)) > diff --git a/gcc/testsuite/g++.dg/cpp0x/initlist92.C b/gcc/testsuite/g++.dg/cpp0x/initlist92.C > index 319264ae274..213b192d441 100644 > --- a/gcc/testsuite/g++.dg/cpp0x/initlist92.C > +++ b/gcc/testsuite/g++.dg/cpp0x/initlist92.C > @@ -23,7 +23,7 @@ bool Test4(std::initializer_list<std::string>); > > int main () > { > - ( Test1({"false"}) ); // { dg-error "narrowing" "" { target c++2a } } > + ( Test1({"false"}) ); // { dg-error "narrowing" } > ( Test2({123}) ); > ( Test3({456}) ); > ( Test4({"false"}) ); > > base-commit: dbfba41e95d1d93b17e907b7f516b52ed3a3c415 >
On 2/24/20 9:58 AM, Jason Merrill wrote: > On 2/21/20 2:14 PM, Marek Polacek wrote: >> This was approved in the Prague 2020 WG21 meeting so let's adjust the >> comment. Since it's supposed to be a DR I think we should no longer >> limit it to C++20. > > I'm a bit nervous about the impact, but OK. It's easy enough to turn > off -Wnarrowing if it's a problem for users. Hmm, have you tried doing a Fedora build with this change? >> Bootstrapped/regtested on x86_64-linux, ok for trunk? >> >> 2020-02-21 Marek Polacek <polacek@redhat.com> >> >> P1957R2 >> * typeck2.c (check_narrowing): Consider T* to bool narrowing >> in C++11 and up. >> >> * g++.dg/cpp0x/initlist92.C: Don't expect an error in C++20 only. >> --- >> gcc/cp/typeck2.c | 7 ++++--- >> gcc/testsuite/g++.dg/cpp0x/initlist92.C | 2 +- >> 2 files changed, 5 insertions(+), 4 deletions(-) >> >> diff --git a/gcc/cp/typeck2.c b/gcc/cp/typeck2.c >> index 48920894b3b..68bc2e5c170 100644 >> --- a/gcc/cp/typeck2.c >> +++ b/gcc/cp/typeck2.c >> @@ -1036,9 +1036,10 @@ check_narrowing (tree type, tree init, >> tsubst_flags_t complain, >> } >> else if (TREE_CODE (type) == BOOLEAN_TYPE >> && (TYPE_PTR_P (ftype) || TYPE_PTRMEM_P (ftype))) >> - /* This hasn't actually made it into C++20 yet, but let's add it >> now to get >> - an idea of the impact. */ >> - ok = (cxx_dialect < cxx2a); >> + /* C++20 P1957R2: converting from a pointer type or a >> pointer-to-member >> + type to bool should be considered narrowing. This is a DR so >> is not >> + limited to C++20 only. */ >> + ok = false; >> bool almost_ok = ok; >> if (!ok && !CONSTANT_CLASS_P (init) && (complain & >> tf_warning_or_error)) >> diff --git a/gcc/testsuite/g++.dg/cpp0x/initlist92.C >> b/gcc/testsuite/g++.dg/cpp0x/initlist92.C >> index 319264ae274..213b192d441 100644 >> --- a/gcc/testsuite/g++.dg/cpp0x/initlist92.C >> +++ b/gcc/testsuite/g++.dg/cpp0x/initlist92.C >> @@ -23,7 +23,7 @@ bool Test4(std::initializer_list<std::string>); >> int main () >> { >> - ( Test1({"false"}) ); // { dg-error "narrowing" "" { target >> c++2a } } >> + ( Test1({"false"}) ); // { dg-error "narrowing" } >> ( Test2({123}) ); >> ( Test3({456}) ); >> ( Test4({"false"}) ); >> >> base-commit: dbfba41e95d1d93b17e907b7f516b52ed3a3c415 >> >
On Mon, Feb 24, 2020 at 09:59:51AM -0500, Jason Merrill wrote: > On 2/24/20 9:58 AM, Jason Merrill wrote: > > On 2/21/20 2:14 PM, Marek Polacek wrote: > > > This was approved in the Prague 2020 WG21 meeting so let's adjust the > > > comment. Since it's supposed to be a DR I think we should no longer > > > limit it to C++20. > > > > I'm a bit nervous about the impact, but OK. It's easy enough to turn > > off -Wnarrowing if it's a problem for users. > > Hmm, have you tried doing a Fedora build with this change? I'll admit I have not. Jeff, would it be possible to apply this patch onto one of your testers? We'd be much more comfortable going with this change then. > > > Bootstrapped/regtested on x86_64-linux, ok for trunk? > > > > > > 2020-02-21 Marek Polacek <polacek@redhat.com> > > > > > > P1957R2 > > > * typeck2.c (check_narrowing): Consider T* to bool narrowing > > > in C++11 and up. > > > > > > * g++.dg/cpp0x/initlist92.C: Don't expect an error in C++20 only. > > > --- > > > gcc/cp/typeck2.c | 7 ++++--- > > > gcc/testsuite/g++.dg/cpp0x/initlist92.C | 2 +- > > > 2 files changed, 5 insertions(+), 4 deletions(-) > > > > > > diff --git a/gcc/cp/typeck2.c b/gcc/cp/typeck2.c > > > index 48920894b3b..68bc2e5c170 100644 > > > --- a/gcc/cp/typeck2.c > > > +++ b/gcc/cp/typeck2.c > > > @@ -1036,9 +1036,10 @@ check_narrowing (tree type, tree init, > > > tsubst_flags_t complain, > > > } > > > else if (TREE_CODE (type) == BOOLEAN_TYPE > > > && (TYPE_PTR_P (ftype) || TYPE_PTRMEM_P (ftype))) > > > - /* This hasn't actually made it into C++20 yet, but let's add > > > it now to get > > > - an idea of the impact. */ > > > - ok = (cxx_dialect < cxx2a); > > > + /* C++20 P1957R2: converting from a pointer type or a > > > pointer-to-member > > > + type to bool should be considered narrowing. This is a DR > > > so is not > > > + limited to C++20 only. */ > > > + ok = false; > > > bool almost_ok = ok; > > > if (!ok && !CONSTANT_CLASS_P (init) && (complain & > > > tf_warning_or_error)) > > > diff --git a/gcc/testsuite/g++.dg/cpp0x/initlist92.C > > > b/gcc/testsuite/g++.dg/cpp0x/initlist92.C > > > index 319264ae274..213b192d441 100644 > > > --- a/gcc/testsuite/g++.dg/cpp0x/initlist92.C > > > +++ b/gcc/testsuite/g++.dg/cpp0x/initlist92.C > > > @@ -23,7 +23,7 @@ bool Test4(std::initializer_list<std::string>); > > > int main () > > > { > > > - ( Test1({"false"}) ); // { dg-error "narrowing" "" { target > > > c++2a } } > > > + ( Test1({"false"}) ); // { dg-error "narrowing" } > > > ( Test2({123}) ); > > > ( Test3({456}) ); > > > ( Test4({"false"}) ); > > > > > > base-commit: dbfba41e95d1d93b17e907b7f516b52ed3a3c415 > > > > > > Marek
Ping: Jeff, see the question below. Thanks. On Mon, Feb 24, 2020 at 10:19:20AM -0500, Marek Polacek wrote: > On Mon, Feb 24, 2020 at 09:59:51AM -0500, Jason Merrill wrote: > > On 2/24/20 9:58 AM, Jason Merrill wrote: > > > On 2/21/20 2:14 PM, Marek Polacek wrote: > > > > This was approved in the Prague 2020 WG21 meeting so let's adjust the > > > > comment. Since it's supposed to be a DR I think we should no longer > > > > limit it to C++20. > > > > > > I'm a bit nervous about the impact, but OK. It's easy enough to turn > > > off -Wnarrowing if it's a problem for users. > > > > Hmm, have you tried doing a Fedora build with this change? > > I'll admit I have not. > > Jeff, would it be possible to apply this patch onto one of your testers? > We'd be much more comfortable going with this change then. > > > > > Bootstrapped/regtested on x86_64-linux, ok for trunk? > > > > > > > > 2020-02-21 Marek Polacek <polacek@redhat.com> > > > > > > > > P1957R2 > > > > * typeck2.c (check_narrowing): Consider T* to bool narrowing > > > > in C++11 and up. > > > > > > > > * g++.dg/cpp0x/initlist92.C: Don't expect an error in C++20 only. > > > > --- > > > > gcc/cp/typeck2.c | 7 ++++--- > > > > gcc/testsuite/g++.dg/cpp0x/initlist92.C | 2 +- > > > > 2 files changed, 5 insertions(+), 4 deletions(-) > > > > > > > > diff --git a/gcc/cp/typeck2.c b/gcc/cp/typeck2.c > > > > index 48920894b3b..68bc2e5c170 100644 > > > > --- a/gcc/cp/typeck2.c > > > > +++ b/gcc/cp/typeck2.c > > > > @@ -1036,9 +1036,10 @@ check_narrowing (tree type, tree init, > > > > tsubst_flags_t complain, > > > > } > > > > else if (TREE_CODE (type) == BOOLEAN_TYPE > > > > && (TYPE_PTR_P (ftype) || TYPE_PTRMEM_P (ftype))) > > > > - /* This hasn't actually made it into C++20 yet, but let's add > > > > it now to get > > > > - an idea of the impact. */ > > > > - ok = (cxx_dialect < cxx2a); > > > > + /* C++20 P1957R2: converting from a pointer type or a > > > > pointer-to-member > > > > + type to bool should be considered narrowing. This is a DR > > > > so is not > > > > + limited to C++20 only. */ > > > > + ok = false; > > > > bool almost_ok = ok; > > > > if (!ok && !CONSTANT_CLASS_P (init) && (complain & > > > > tf_warning_or_error)) > > > > diff --git a/gcc/testsuite/g++.dg/cpp0x/initlist92.C > > > > b/gcc/testsuite/g++.dg/cpp0x/initlist92.C > > > > index 319264ae274..213b192d441 100644 > > > > --- a/gcc/testsuite/g++.dg/cpp0x/initlist92.C > > > > +++ b/gcc/testsuite/g++.dg/cpp0x/initlist92.C > > > > @@ -23,7 +23,7 @@ bool Test4(std::initializer_list<std::string>); > > > > int main () > > > > { > > > > - ( Test1({"false"}) ); // { dg-error "narrowing" "" { target > > > > c++2a } } > > > > + ( Test1({"false"}) ); // { dg-error "narrowing" } > > > > ( Test2({123}) ); > > > > ( Test3({456}) ); > > > > ( Test4({"false"}) ); > > > > > > > > base-commit: dbfba41e95d1d93b17e907b7f516b52ed3a3c415 > > > > > > > > > > > Marek > Marek
On Mon, 2020-03-02 at 10:45 -0500, Marek Polacek wrote: > Ping: Jeff, see the question below. Thanks. Sorry, totally missed the question. I'm guessing you want me to run it through the Fedora build tester? jeff >
On Mon, Mar 02, 2020 at 09:25:35AM -0700, Jeff Law wrote: > On Mon, 2020-03-02 at 10:45 -0500, Marek Polacek wrote: > > Ping: Jeff, see the question below. Thanks. > Sorry, totally missed the question. I'm guessing you want me to run it through > the Fedora build tester? Yeah -- we want to allow a more strict narrow checking even in pre-C+20 modes, and we're wondering about the fallout. So re-compiling Fedora packages would be useful. Marek
On Mon, 2020-03-02 at 11:29 -0500, Marek Polacek wrote: > On Mon, Mar 02, 2020 at 09:25:35AM -0700, Jeff Law wrote: > > On Mon, 2020-03-02 at 10:45 -0500, Marek Polacek wrote: > > > Ping: Jeff, see the question below. Thanks. > > Sorry, totally missed the question. I'm guessing you want me to run it > > through > > the Fedora build tester? > > Yeah -- we want to allow a more strict narrow checking even in pre-C+20 > modes, and we're wondering about the fallout. So re-compiling Fedora > packages would be useful. Is the error easy to identify from a diagnostic? If so that would avoid having to get fresh baselines. I can just apply the patch, build gcc, then spin up a single build of everything. Jeff
On 3/2/20 11:38 AM, Jeff Law wrote: > On Mon, 2020-03-02 at 11:29 -0500, Marek Polacek wrote: >> On Mon, Mar 02, 2020 at 09:25:35AM -0700, Jeff Law wrote: >>> On Mon, 2020-03-02 at 10:45 -0500, Marek Polacek wrote: >>>> Ping: Jeff, see the question below. Thanks. >>> Sorry, totally missed the question. I'm guessing you want me to run it >>> through >>> the Fedora build tester? >> >> Yeah -- we want to allow a more strict narrow checking even in pre-C+20 >> modes, and we're wondering about the fallout. So re-compiling Fedora >> packages would be useful. > Is the error easy to identify from a diagnostic? If so that would avoid having > to get fresh baselines. I can just apply the patch, build gcc, then spin up a > single build of everything. Not always; sometimes making something ill-formed just changes overload resolution On further thought, it seems unnecessary to make this change at this point in the release cycle. Let's defer this to GCC 11. Jason
On Mon, Mar 02, 2020 at 11:59:12AM -0500, Jason Merrill wrote: > On 3/2/20 11:38 AM, Jeff Law wrote: > > On Mon, 2020-03-02 at 11:29 -0500, Marek Polacek wrote: > > > On Mon, Mar 02, 2020 at 09:25:35AM -0700, Jeff Law wrote: > > > > On Mon, 2020-03-02 at 10:45 -0500, Marek Polacek wrote: > > > > > Ping: Jeff, see the question below. Thanks. > > > > Sorry, totally missed the question. I'm guessing you want me to run it > > > > through > > > > the Fedora build tester? > > > > > > Yeah -- we want to allow a more strict narrow checking even in pre-C+20 > > > modes, and we're wondering about the fallout. So re-compiling Fedora > > > packages would be useful. > > Is the error easy to identify from a diagnostic? If so that would avoid having > > to get fresh baselines. I can just apply the patch, build gcc, then spin up a > > single build of everything. > > Not always; sometimes making something ill-formed just changes overload > resolution > > On further thought, it seems unnecessary to make this change at this point > in the release cycle. Let's defer this to GCC 11. Ack, will defer then. Marek
diff --git a/gcc/cp/typeck2.c b/gcc/cp/typeck2.c index 48920894b3b..68bc2e5c170 100644 --- a/gcc/cp/typeck2.c +++ b/gcc/cp/typeck2.c @@ -1036,9 +1036,10 @@ check_narrowing (tree type, tree init, tsubst_flags_t complain, } else if (TREE_CODE (type) == BOOLEAN_TYPE && (TYPE_PTR_P (ftype) || TYPE_PTRMEM_P (ftype))) - /* This hasn't actually made it into C++20 yet, but let's add it now to get - an idea of the impact. */ - ok = (cxx_dialect < cxx2a); + /* C++20 P1957R2: converting from a pointer type or a pointer-to-member + type to bool should be considered narrowing. This is a DR so is not + limited to C++20 only. */ + ok = false; bool almost_ok = ok; if (!ok && !CONSTANT_CLASS_P (init) && (complain & tf_warning_or_error)) diff --git a/gcc/testsuite/g++.dg/cpp0x/initlist92.C b/gcc/testsuite/g++.dg/cpp0x/initlist92.C index 319264ae274..213b192d441 100644 --- a/gcc/testsuite/g++.dg/cpp0x/initlist92.C +++ b/gcc/testsuite/g++.dg/cpp0x/initlist92.C @@ -23,7 +23,7 @@ bool Test4(std::initializer_list<std::string>); int main () { - ( Test1({"false"}) ); // { dg-error "narrowing" "" { target c++2a } } + ( Test1({"false"}) ); // { dg-error "narrowing" } ( Test2({123}) ); ( Test3({456}) ); ( Test4({"false"}) );