* [PATCH] c++: Prevent warnings for value-dependent exprs [PR96742] @ 2020-10-24 22:52 Marek Polacek 2020-10-27 17:36 ` Jason Merrill 0 siblings, 1 reply; 10+ messages in thread From: Marek Polacek @ 2020-10-24 22:52 UTC (permalink / raw) To: GCC Patches, Jason Merrill Here, in r11-155, I changed the call to uses_template_parms to type_dependent_expression_p_push to avoid a crash in C++98 in value_dependent_expression_p on a non-constant expression. But that prompted a host of complaints that we now warn for value-dependent expressions in templates. Those warnings are technically valid, but people still don't want them because they're awkward to avoid. So let's partially revert my earlier fix and make sure that we don't ICE in value_dependent_expression_p by checking potential_constant_expression first. Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/10? gcc/cp/ChangeLog: PR c++/96675 PR c++/96742 * pt.c (tsubst_copy_and_build): Call uses_template_parms instead of type_dependent_expression_p_push. Only call uses_template_parms for expressions that are potential_constant_expression. gcc/testsuite/ChangeLog: PR c++/96675 PR c++/96742 * g++.dg/warn/Wdiv-by-zero-3.C: Turn dg-warning into dg-bogus. * g++.dg/warn/Wtautological-compare3.C: New test. * g++.dg/warn/Wtype-limits5.C: New test. * g++.old-deja/g++.pt/crash10.C: Remove dg-warning. --- gcc/cp/pt.c | 6 ++++-- gcc/testsuite/g++.dg/warn/Wdiv-by-zero-3.C | 6 ++++-- gcc/testsuite/g++.dg/warn/Wtautological-compare3.C | 11 +++++++++++ gcc/testsuite/g++.dg/warn/Wtype-limits5.C | 11 +++++++++++ gcc/testsuite/g++.old-deja/g++.pt/crash10.C | 1 - 5 files changed, 30 insertions(+), 5 deletions(-) create mode 100644 gcc/testsuite/g++.dg/warn/Wtautological-compare3.C create mode 100644 gcc/testsuite/g++.dg/warn/Wtype-limits5.C diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c index dc664ec3798..8aa0bc2c0d8 100644 --- a/gcc/cp/pt.c +++ b/gcc/cp/pt.c @@ -19618,8 +19618,10 @@ tsubst_copy_and_build (tree t, { /* If T was type-dependent, suppress warnings that depend on the range of the types involved. */ - bool was_dep = type_dependent_expression_p_push (t); - + ++processing_template_decl; + const bool was_dep = (!potential_constant_expression (t) + || uses_template_parms (t)); + --processing_template_decl; tree op0 = RECUR (TREE_OPERAND (t, 0)); tree op1 = RECUR (TREE_OPERAND (t, 1)); diff --git a/gcc/testsuite/g++.dg/warn/Wdiv-by-zero-3.C b/gcc/testsuite/g++.dg/warn/Wdiv-by-zero-3.C index 424eb0c3d49..01f691f2878 100644 --- a/gcc/testsuite/g++.dg/warn/Wdiv-by-zero-3.C +++ b/gcc/testsuite/g++.dg/warn/Wdiv-by-zero-3.C @@ -5,8 +5,10 @@ foo (T t, int i) { int m1 = 10 / t; int m2 = 10 / i; - int m3 = 10 / (sizeof(T) - sizeof(int)); // { dg-warning "division by" } - int m4 = 10 / N; // { dg-warning "division by" } + // People don't want to see warnings for type- or value-dependent + // expressions. + int m3 = 10 / (sizeof(T) - sizeof(int)); // { dg-bogus "division by" } + int m4 = 10 / N; // { dg-bogus "division by" } return m1 + m2 + m3 + m4; } diff --git a/gcc/testsuite/g++.dg/warn/Wtautological-compare3.C b/gcc/testsuite/g++.dg/warn/Wtautological-compare3.C new file mode 100644 index 00000000000..89bf1b619a6 --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Wtautological-compare3.C @@ -0,0 +1,11 @@ +// PR c++/96675 +// { dg-do compile { target c++11 } } +// { dg-additional-options "-Wtautological-compare" } + +template<char c> +constexpr bool f(char d) { + return 'a' <= c && c <= 'z' ? (d | 0x20) == c : + 'A' <= c && c <= 'Z' ? (d & ~0x20) == c : + d == c; +} +static_assert(f<'p'>('P'), ""); diff --git a/gcc/testsuite/g++.dg/warn/Wtype-limits5.C b/gcc/testsuite/g++.dg/warn/Wtype-limits5.C new file mode 100644 index 00000000000..5e79123b622 --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Wtype-limits5.C @@ -0,0 +1,11 @@ +// PR c++/96742 +// { dg-additional-options "-Wtype-limits" } + +template <unsigned N> +bool f(unsigned x) { + return unsigned(x < N); +} + +int main() { + f<0>(1); +} diff --git a/gcc/testsuite/g++.old-deja/g++.pt/crash10.C b/gcc/testsuite/g++.old-deja/g++.pt/crash10.C index 012e3d0c11b..a84b19004ee 100644 --- a/gcc/testsuite/g++.old-deja/g++.pt/crash10.C +++ b/gcc/testsuite/g++.old-deja/g++.pt/crash10.C @@ -6,7 +6,6 @@ public: enum { val = (N == 0) ? M : GCD<N, M % N>::val }; // { dg-error "constant expression" "valid" { target *-*-* } .-1 } // { dg-message "template argument" "valid" { target *-*-* } .-2 } -// { dg-warning "division by" "" { target *-*-* } .-3 } }; int main() { base-commit: 1aeb7d7d67d167297ca2f4a97ef20f68e7546b4c -- 2.26.2 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] c++: Prevent warnings for value-dependent exprs [PR96742] 2020-10-24 22:52 [PATCH] c++: Prevent warnings for value-dependent exprs [PR96742] Marek Polacek @ 2020-10-27 17:36 ` Jason Merrill 2020-10-28 18:00 ` [PATCH v2] " Marek Polacek 0 siblings, 1 reply; 10+ messages in thread From: Jason Merrill @ 2020-10-27 17:36 UTC (permalink / raw) To: Marek Polacek, GCC Patches On 10/24/20 6:52 PM, Marek Polacek wrote: > Here, in r11-155, I changed the call to uses_template_parms to > type_dependent_expression_p_push to avoid a crash in C++98 in > value_dependent_expression_p on a non-constant expression. But that > prompted a host of complaints that we now warn for value-dependent > expressions in templates. Those warnings are technically valid, but > people still don't want them because they're awkward to avoid. So let's > partially revert my earlier fix and make sure that we don't ICE in > value_dependent_expression_p by checking potential_constant_expression > first. > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/10? > > gcc/cp/ChangeLog: > > PR c++/96675 > PR c++/96742 > * pt.c (tsubst_copy_and_build): Call uses_template_parms instead of > type_dependent_expression_p_push. Only call uses_template_parms > for expressions that are potential_constant_expression. > > gcc/testsuite/ChangeLog: > > PR c++/96675 > PR c++/96742 > * g++.dg/warn/Wdiv-by-zero-3.C: Turn dg-warning into dg-bogus. > * g++.dg/warn/Wtautological-compare3.C: New test. > * g++.dg/warn/Wtype-limits5.C: New test. > * g++.old-deja/g++.pt/crash10.C: Remove dg-warning. > --- > gcc/cp/pt.c | 6 ++++-- > gcc/testsuite/g++.dg/warn/Wdiv-by-zero-3.C | 6 ++++-- > gcc/testsuite/g++.dg/warn/Wtautological-compare3.C | 11 +++++++++++ > gcc/testsuite/g++.dg/warn/Wtype-limits5.C | 11 +++++++++++ > gcc/testsuite/g++.old-deja/g++.pt/crash10.C | 1 - > 5 files changed, 30 insertions(+), 5 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/warn/Wtautological-compare3.C > create mode 100644 gcc/testsuite/g++.dg/warn/Wtype-limits5.C > > diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c > index dc664ec3798..8aa0bc2c0d8 100644 > --- a/gcc/cp/pt.c > +++ b/gcc/cp/pt.c > @@ -19618,8 +19618,10 @@ tsubst_copy_and_build (tree t, > { > /* If T was type-dependent, suppress warnings that depend on the range > of the types involved. */ > - bool was_dep = type_dependent_expression_p_push (t); > - > + ++processing_template_decl; > + const bool was_dep = (!potential_constant_expression (t) > + || uses_template_parms (t)); We don't want to suppress warnings for a non-constant expression that uses no template parms. So maybe potential_c_e ? value_d : type_d ? Or perhaps instantiation_dependent_expression_p. > + --processing_template_decl; > tree op0 = RECUR (TREE_OPERAND (t, 0)); > tree op1 = RECUR (TREE_OPERAND (t, 1)); > > diff --git a/gcc/testsuite/g++.dg/warn/Wdiv-by-zero-3.C b/gcc/testsuite/g++.dg/warn/Wdiv-by-zero-3.C > index 424eb0c3d49..01f691f2878 100644 > --- a/gcc/testsuite/g++.dg/warn/Wdiv-by-zero-3.C > +++ b/gcc/testsuite/g++.dg/warn/Wdiv-by-zero-3.C > @@ -5,8 +5,10 @@ foo (T t, int i) > { > int m1 = 10 / t; > int m2 = 10 / i; > - int m3 = 10 / (sizeof(T) - sizeof(int)); // { dg-warning "division by" } > - int m4 = 10 / N; // { dg-warning "division by" } > + // People don't want to see warnings for type- or value-dependent > + // expressions. > + int m3 = 10 / (sizeof(T) - sizeof(int)); // { dg-bogus "division by" } > + int m4 = 10 / N; // { dg-bogus "division by" } > return m1 + m2 + m3 + m4; > } > > diff --git a/gcc/testsuite/g++.dg/warn/Wtautological-compare3.C b/gcc/testsuite/g++.dg/warn/Wtautological-compare3.C > new file mode 100644 > index 00000000000..89bf1b619a6 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/warn/Wtautological-compare3.C > @@ -0,0 +1,11 @@ > +// PR c++/96675 > +// { dg-do compile { target c++11 } } > +// { dg-additional-options "-Wtautological-compare" } > + > +template<char c> > +constexpr bool f(char d) { > + return 'a' <= c && c <= 'z' ? (d | 0x20) == c : > + 'A' <= c && c <= 'Z' ? (d & ~0x20) == c : > + d == c; > +} > +static_assert(f<'p'>('P'), ""); > diff --git a/gcc/testsuite/g++.dg/warn/Wtype-limits5.C b/gcc/testsuite/g++.dg/warn/Wtype-limits5.C > new file mode 100644 > index 00000000000..5e79123b622 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/warn/Wtype-limits5.C > @@ -0,0 +1,11 @@ > +// PR c++/96742 > +// { dg-additional-options "-Wtype-limits" } > + > +template <unsigned N> > +bool f(unsigned x) { > + return unsigned(x < N); > +} > + > +int main() { > + f<0>(1); > +} > diff --git a/gcc/testsuite/g++.old-deja/g++.pt/crash10.C b/gcc/testsuite/g++.old-deja/g++.pt/crash10.C > index 012e3d0c11b..a84b19004ee 100644 > --- a/gcc/testsuite/g++.old-deja/g++.pt/crash10.C > +++ b/gcc/testsuite/g++.old-deja/g++.pt/crash10.C > @@ -6,7 +6,6 @@ public: > enum { val = (N == 0) ? M : GCD<N, M % N>::val }; > // { dg-error "constant expression" "valid" { target *-*-* } .-1 } > // { dg-message "template argument" "valid" { target *-*-* } .-2 } > -// { dg-warning "division by" "" { target *-*-* } .-3 } > }; > > int main() { > > base-commit: 1aeb7d7d67d167297ca2f4a97ef20f68e7546b4c > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] c++: Prevent warnings for value-dependent exprs [PR96742] 2020-10-27 17:36 ` Jason Merrill @ 2020-10-28 18:00 ` Marek Polacek 2020-10-28 18:46 ` Jason Merrill 0 siblings, 1 reply; 10+ messages in thread From: Marek Polacek @ 2020-10-28 18:00 UTC (permalink / raw) To: Jason Merrill; +Cc: GCC Patches On Tue, Oct 27, 2020 at 01:36:30PM -0400, Jason Merrill wrote: > On 10/24/20 6:52 PM, Marek Polacek wrote: > > Here, in r11-155, I changed the call to uses_template_parms to > > type_dependent_expression_p_push to avoid a crash in C++98 in > > value_dependent_expression_p on a non-constant expression. But that > > prompted a host of complaints that we now warn for value-dependent > > expressions in templates. Those warnings are technically valid, but > > people still don't want them because they're awkward to avoid. So let's > > partially revert my earlier fix and make sure that we don't ICE in > > value_dependent_expression_p by checking potential_constant_expression > > first. > > > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/10? > > > > gcc/cp/ChangeLog: > > > > PR c++/96675 > > PR c++/96742 > > * pt.c (tsubst_copy_and_build): Call uses_template_parms instead of > > type_dependent_expression_p_push. Only call uses_template_parms > > for expressions that are potential_constant_expression. > > > > gcc/testsuite/ChangeLog: > > > > PR c++/96675 > > PR c++/96742 > > * g++.dg/warn/Wdiv-by-zero-3.C: Turn dg-warning into dg-bogus. > > * g++.dg/warn/Wtautological-compare3.C: New test. > > * g++.dg/warn/Wtype-limits5.C: New test. > > * g++.old-deja/g++.pt/crash10.C: Remove dg-warning. > > --- > > gcc/cp/pt.c | 6 ++++-- > > gcc/testsuite/g++.dg/warn/Wdiv-by-zero-3.C | 6 ++++-- > > gcc/testsuite/g++.dg/warn/Wtautological-compare3.C | 11 +++++++++++ > > gcc/testsuite/g++.dg/warn/Wtype-limits5.C | 11 +++++++++++ > > gcc/testsuite/g++.old-deja/g++.pt/crash10.C | 1 - > > 5 files changed, 30 insertions(+), 5 deletions(-) > > create mode 100644 gcc/testsuite/g++.dg/warn/Wtautological-compare3.C > > create mode 100644 gcc/testsuite/g++.dg/warn/Wtype-limits5.C > > > > diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c > > index dc664ec3798..8aa0bc2c0d8 100644 > > --- a/gcc/cp/pt.c > > +++ b/gcc/cp/pt.c > > @@ -19618,8 +19618,10 @@ tsubst_copy_and_build (tree t, > > { > > /* If T was type-dependent, suppress warnings that depend on the range > > of the types involved. */ > > - bool was_dep = type_dependent_expression_p_push (t); > > - > > + ++processing_template_decl; > > + const bool was_dep = (!potential_constant_expression (t) > > + || uses_template_parms (t)); > > We don't want to suppress warnings for a non-constant expression that uses > no template parms. So maybe Fair enough. > potential_c_e ? value_d : type_d That works for all the cases I have. > ? Or perhaps instantiation_dependent_expression_p. i_d_e_p would still crash in C++98 :(. Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/10? -- >8 -- Here, in r11-155, I changed the call to uses_template_parms to type_dependent_expression_p_push to avoid a crash in C++98 in value_dependent_expression_p on a non-constant expression. But that prompted a host of complaints that we now warn for value-dependent expressions in templates. Those warnings are technically valid, but people still don't want them because they're awkward to avoid. This patch uses value_dependent_expression_p or type_dependent_expression_p. But make sure that we don't ICE in value_dependent_expression_p by checking potential_constant_expression first. gcc/cp/ChangeLog: PR c++/96675 PR c++/96742 * pt.c (tsubst_copy_and_build): Call value_dependent_expression_p or type_dependent_expression_p instead of type_dependent_expression_p_push. But only call value_dependent_expression_p for expressions that are potential_constant_expression. gcc/testsuite/ChangeLog: PR c++/96675 PR c++/96742 * g++.dg/warn/Wdiv-by-zero-3.C: Turn dg-warning into dg-bogus. * g++.dg/warn/Wtautological-compare3.C: New test. * g++.dg/warn/Wtype-limits5.C: New test. * g++.old-deja/g++.pt/crash10.C: Remove dg-warning. --- gcc/cp/pt.c | 7 +++++-- gcc/testsuite/g++.dg/warn/Wdiv-by-zero-3.C | 6 ++++-- gcc/testsuite/g++.dg/warn/Wtautological-compare3.C | 11 +++++++++++ gcc/testsuite/g++.dg/warn/Wtype-limits5.C | 11 +++++++++++ gcc/testsuite/g++.old-deja/g++.pt/crash10.C | 1 - 5 files changed, 31 insertions(+), 5 deletions(-) create mode 100644 gcc/testsuite/g++.dg/warn/Wtautological-compare3.C create mode 100644 gcc/testsuite/g++.dg/warn/Wtype-limits5.C diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c index 3c0f2546489..57db476645e 100644 --- a/gcc/cp/pt.c +++ b/gcc/cp/pt.c @@ -19618,8 +19618,11 @@ tsubst_copy_and_build (tree t, { /* If T was type-dependent, suppress warnings that depend on the range of the types involved. */ - bool was_dep = type_dependent_expression_p_push (t); - + ++processing_template_decl; + const bool was_dep = (potential_constant_expression (t) + ? value_dependent_expression_p (t) + : type_dependent_expression_p (t)); + --processing_template_decl; tree op0 = RECUR (TREE_OPERAND (t, 0)); tree op1 = RECUR (TREE_OPERAND (t, 1)); diff --git a/gcc/testsuite/g++.dg/warn/Wdiv-by-zero-3.C b/gcc/testsuite/g++.dg/warn/Wdiv-by-zero-3.C index 424eb0c3d49..01f691f2878 100644 --- a/gcc/testsuite/g++.dg/warn/Wdiv-by-zero-3.C +++ b/gcc/testsuite/g++.dg/warn/Wdiv-by-zero-3.C @@ -5,8 +5,10 @@ foo (T t, int i) { int m1 = 10 / t; int m2 = 10 / i; - int m3 = 10 / (sizeof(T) - sizeof(int)); // { dg-warning "division by" } - int m4 = 10 / N; // { dg-warning "division by" } + // People don't want to see warnings for type- or value-dependent + // expressions. + int m3 = 10 / (sizeof(T) - sizeof(int)); // { dg-bogus "division by" } + int m4 = 10 / N; // { dg-bogus "division by" } return m1 + m2 + m3 + m4; } diff --git a/gcc/testsuite/g++.dg/warn/Wtautological-compare3.C b/gcc/testsuite/g++.dg/warn/Wtautological-compare3.C new file mode 100644 index 00000000000..89bf1b619a6 --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Wtautological-compare3.C @@ -0,0 +1,11 @@ +// PR c++/96675 +// { dg-do compile { target c++11 } } +// { dg-additional-options "-Wtautological-compare" } + +template<char c> +constexpr bool f(char d) { + return 'a' <= c && c <= 'z' ? (d | 0x20) == c : + 'A' <= c && c <= 'Z' ? (d & ~0x20) == c : + d == c; +} +static_assert(f<'p'>('P'), ""); diff --git a/gcc/testsuite/g++.dg/warn/Wtype-limits5.C b/gcc/testsuite/g++.dg/warn/Wtype-limits5.C new file mode 100644 index 00000000000..5e79123b622 --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Wtype-limits5.C @@ -0,0 +1,11 @@ +// PR c++/96742 +// { dg-additional-options "-Wtype-limits" } + +template <unsigned N> +bool f(unsigned x) { + return unsigned(x < N); +} + +int main() { + f<0>(1); +} diff --git a/gcc/testsuite/g++.old-deja/g++.pt/crash10.C b/gcc/testsuite/g++.old-deja/g++.pt/crash10.C index 012e3d0c11b..a84b19004ee 100644 --- a/gcc/testsuite/g++.old-deja/g++.pt/crash10.C +++ b/gcc/testsuite/g++.old-deja/g++.pt/crash10.C @@ -6,7 +6,6 @@ public: enum { val = (N == 0) ? M : GCD<N, M % N>::val }; // { dg-error "constant expression" "valid" { target *-*-* } .-1 } // { dg-message "template argument" "valid" { target *-*-* } .-2 } -// { dg-warning "division by" "" { target *-*-* } .-3 } }; int main() { base-commit: 176b8b9679dfec881b7cf379f808cca3950b1e74 -- 2.28.0 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] c++: Prevent warnings for value-dependent exprs [PR96742] 2020-10-28 18:00 ` [PATCH v2] " Marek Polacek @ 2020-10-28 18:46 ` Jason Merrill 2020-10-28 21:29 ` Marek Polacek 0 siblings, 1 reply; 10+ messages in thread From: Jason Merrill @ 2020-10-28 18:46 UTC (permalink / raw) To: Marek Polacek; +Cc: GCC Patches On 10/28/20 2:00 PM, Marek Polacek wrote: > On Tue, Oct 27, 2020 at 01:36:30PM -0400, Jason Merrill wrote: >> On 10/24/20 6:52 PM, Marek Polacek wrote: >>> Here, in r11-155, I changed the call to uses_template_parms to >>> type_dependent_expression_p_push to avoid a crash in C++98 in >>> value_dependent_expression_p on a non-constant expression. But that >>> prompted a host of complaints that we now warn for value-dependent >>> expressions in templates. Those warnings are technically valid, but >>> people still don't want them because they're awkward to avoid. So let's >>> partially revert my earlier fix and make sure that we don't ICE in >>> value_dependent_expression_p by checking potential_constant_expression >>> first. >>> >>> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/10? >>> >>> gcc/cp/ChangeLog: >>> >>> PR c++/96675 >>> PR c++/96742 >>> * pt.c (tsubst_copy_and_build): Call uses_template_parms instead of >>> type_dependent_expression_p_push. Only call uses_template_parms >>> for expressions that are potential_constant_expression. >>> >>> gcc/testsuite/ChangeLog: >>> >>> PR c++/96675 >>> PR c++/96742 >>> * g++.dg/warn/Wdiv-by-zero-3.C: Turn dg-warning into dg-bogus. >>> * g++.dg/warn/Wtautological-compare3.C: New test. >>> * g++.dg/warn/Wtype-limits5.C: New test. >>> * g++.old-deja/g++.pt/crash10.C: Remove dg-warning. >>> --- >>> gcc/cp/pt.c | 6 ++++-- >>> gcc/testsuite/g++.dg/warn/Wdiv-by-zero-3.C | 6 ++++-- >>> gcc/testsuite/g++.dg/warn/Wtautological-compare3.C | 11 +++++++++++ >>> gcc/testsuite/g++.dg/warn/Wtype-limits5.C | 11 +++++++++++ >>> gcc/testsuite/g++.old-deja/g++.pt/crash10.C | 1 - >>> 5 files changed, 30 insertions(+), 5 deletions(-) >>> create mode 100644 gcc/testsuite/g++.dg/warn/Wtautological-compare3.C >>> create mode 100644 gcc/testsuite/g++.dg/warn/Wtype-limits5.C >>> >>> diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c >>> index dc664ec3798..8aa0bc2c0d8 100644 >>> --- a/gcc/cp/pt.c >>> +++ b/gcc/cp/pt.c >>> @@ -19618,8 +19618,10 @@ tsubst_copy_and_build (tree t, >>> { >>> /* If T was type-dependent, suppress warnings that depend on the range >>> of the types involved. */ >>> - bool was_dep = type_dependent_expression_p_push (t); >>> - >>> + ++processing_template_decl; >>> + const bool was_dep = (!potential_constant_expression (t) >>> + || uses_template_parms (t)); >> >> We don't want to suppress warnings for a non-constant expression that uses >> no template parms. So maybe > > Fair enough. > >> potential_c_e ? value_d : type_d > > That works for all the cases I have. > >> ? Or perhaps instantiation_dependent_expression_p. > > i_d_e_p would still crash in C++98 :(. Perhaps we should protect the value_d call in i_d_e_p with potential_c_e? > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/10? OK. > -- >8 -- > Here, in r11-155, I changed the call to uses_template_parms to > type_dependent_expression_p_push to avoid a crash in C++98 in > value_dependent_expression_p on a non-constant expression. But that > prompted a host of complaints that we now warn for value-dependent > expressions in templates. Those warnings are technically valid, but > people still don't want them because they're awkward to avoid. This > patch uses value_dependent_expression_p or type_dependent_expression_p. > But make sure that we don't ICE in value_dependent_expression_p by > checking potential_constant_expression first. > > gcc/cp/ChangeLog: > > PR c++/96675 > PR c++/96742 > * pt.c (tsubst_copy_and_build): Call value_dependent_expression_p or > type_dependent_expression_p instead of type_dependent_expression_p_push. > But only call value_dependent_expression_p for expressions that are > potential_constant_expression. > > gcc/testsuite/ChangeLog: > > PR c++/96675 > PR c++/96742 > * g++.dg/warn/Wdiv-by-zero-3.C: Turn dg-warning into dg-bogus. > * g++.dg/warn/Wtautological-compare3.C: New test. > * g++.dg/warn/Wtype-limits5.C: New test. > * g++.old-deja/g++.pt/crash10.C: Remove dg-warning. > --- > gcc/cp/pt.c | 7 +++++-- > gcc/testsuite/g++.dg/warn/Wdiv-by-zero-3.C | 6 ++++-- > gcc/testsuite/g++.dg/warn/Wtautological-compare3.C | 11 +++++++++++ > gcc/testsuite/g++.dg/warn/Wtype-limits5.C | 11 +++++++++++ > gcc/testsuite/g++.old-deja/g++.pt/crash10.C | 1 - > 5 files changed, 31 insertions(+), 5 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/warn/Wtautological-compare3.C > create mode 100644 gcc/testsuite/g++.dg/warn/Wtype-limits5.C > > diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c > index 3c0f2546489..57db476645e 100644 > --- a/gcc/cp/pt.c > +++ b/gcc/cp/pt.c > @@ -19618,8 +19618,11 @@ tsubst_copy_and_build (tree t, > { > /* If T was type-dependent, suppress warnings that depend on the range > of the types involved. */ > - bool was_dep = type_dependent_expression_p_push (t); > - > + ++processing_template_decl; > + const bool was_dep = (potential_constant_expression (t) > + ? value_dependent_expression_p (t) > + : type_dependent_expression_p (t)); > + --processing_template_decl; > tree op0 = RECUR (TREE_OPERAND (t, 0)); > tree op1 = RECUR (TREE_OPERAND (t, 1)); > > diff --git a/gcc/testsuite/g++.dg/warn/Wdiv-by-zero-3.C b/gcc/testsuite/g++.dg/warn/Wdiv-by-zero-3.C > index 424eb0c3d49..01f691f2878 100644 > --- a/gcc/testsuite/g++.dg/warn/Wdiv-by-zero-3.C > +++ b/gcc/testsuite/g++.dg/warn/Wdiv-by-zero-3.C > @@ -5,8 +5,10 @@ foo (T t, int i) > { > int m1 = 10 / t; > int m2 = 10 / i; > - int m3 = 10 / (sizeof(T) - sizeof(int)); // { dg-warning "division by" } > - int m4 = 10 / N; // { dg-warning "division by" } > + // People don't want to see warnings for type- or value-dependent > + // expressions. > + int m3 = 10 / (sizeof(T) - sizeof(int)); // { dg-bogus "division by" } > + int m4 = 10 / N; // { dg-bogus "division by" } > return m1 + m2 + m3 + m4; > } > > diff --git a/gcc/testsuite/g++.dg/warn/Wtautological-compare3.C b/gcc/testsuite/g++.dg/warn/Wtautological-compare3.C > new file mode 100644 > index 00000000000..89bf1b619a6 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/warn/Wtautological-compare3.C > @@ -0,0 +1,11 @@ > +// PR c++/96675 > +// { dg-do compile { target c++11 } } > +// { dg-additional-options "-Wtautological-compare" } > + > +template<char c> > +constexpr bool f(char d) { > + return 'a' <= c && c <= 'z' ? (d | 0x20) == c : > + 'A' <= c && c <= 'Z' ? (d & ~0x20) == c : > + d == c; > +} > +static_assert(f<'p'>('P'), ""); > diff --git a/gcc/testsuite/g++.dg/warn/Wtype-limits5.C b/gcc/testsuite/g++.dg/warn/Wtype-limits5.C > new file mode 100644 > index 00000000000..5e79123b622 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/warn/Wtype-limits5.C > @@ -0,0 +1,11 @@ > +// PR c++/96742 > +// { dg-additional-options "-Wtype-limits" } > + > +template <unsigned N> > +bool f(unsigned x) { > + return unsigned(x < N); > +} > + > +int main() { > + f<0>(1); > +} > diff --git a/gcc/testsuite/g++.old-deja/g++.pt/crash10.C b/gcc/testsuite/g++.old-deja/g++.pt/crash10.C > index 012e3d0c11b..a84b19004ee 100644 > --- a/gcc/testsuite/g++.old-deja/g++.pt/crash10.C > +++ b/gcc/testsuite/g++.old-deja/g++.pt/crash10.C > @@ -6,7 +6,6 @@ public: > enum { val = (N == 0) ? M : GCD<N, M % N>::val }; > // { dg-error "constant expression" "valid" { target *-*-* } .-1 } > // { dg-message "template argument" "valid" { target *-*-* } .-2 } > -// { dg-warning "division by" "" { target *-*-* } .-3 } > }; > > int main() { > > base-commit: 176b8b9679dfec881b7cf379f808cca3950b1e74 > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] c++: Prevent warnings for value-dependent exprs [PR96742] 2020-10-28 18:46 ` Jason Merrill @ 2020-10-28 21:29 ` Marek Polacek 2020-10-28 21:48 ` Jason Merrill 2020-10-31 10:09 ` Iain Sandoe 0 siblings, 2 replies; 10+ messages in thread From: Marek Polacek @ 2020-10-28 21:29 UTC (permalink / raw) To: Jason Merrill; +Cc: GCC Patches On Wed, Oct 28, 2020 at 02:46:36PM -0400, Jason Merrill wrote: > On 10/28/20 2:00 PM, Marek Polacek wrote: > > On Tue, Oct 27, 2020 at 01:36:30PM -0400, Jason Merrill wrote: > > > On 10/24/20 6:52 PM, Marek Polacek wrote: > > > > Here, in r11-155, I changed the call to uses_template_parms to > > > > type_dependent_expression_p_push to avoid a crash in C++98 in > > > > value_dependent_expression_p on a non-constant expression. But that > > > > prompted a host of complaints that we now warn for value-dependent > > > > expressions in templates. Those warnings are technically valid, but > > > > people still don't want them because they're awkward to avoid. So let's > > > > partially revert my earlier fix and make sure that we don't ICE in > > > > value_dependent_expression_p by checking potential_constant_expression > > > > first. > > > > > > > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/10? > > > > > > > > gcc/cp/ChangeLog: > > > > > > > > PR c++/96675 > > > > PR c++/96742 > > > > * pt.c (tsubst_copy_and_build): Call uses_template_parms instead of > > > > type_dependent_expression_p_push. Only call uses_template_parms > > > > for expressions that are potential_constant_expression. > > > > > > > > gcc/testsuite/ChangeLog: > > > > > > > > PR c++/96675 > > > > PR c++/96742 > > > > * g++.dg/warn/Wdiv-by-zero-3.C: Turn dg-warning into dg-bogus. > > > > * g++.dg/warn/Wtautological-compare3.C: New test. > > > > * g++.dg/warn/Wtype-limits5.C: New test. > > > > * g++.old-deja/g++.pt/crash10.C: Remove dg-warning. > > > > --- > > > > gcc/cp/pt.c | 6 ++++-- > > > > gcc/testsuite/g++.dg/warn/Wdiv-by-zero-3.C | 6 ++++-- > > > > gcc/testsuite/g++.dg/warn/Wtautological-compare3.C | 11 +++++++++++ > > > > gcc/testsuite/g++.dg/warn/Wtype-limits5.C | 11 +++++++++++ > > > > gcc/testsuite/g++.old-deja/g++.pt/crash10.C | 1 - > > > > 5 files changed, 30 insertions(+), 5 deletions(-) > > > > create mode 100644 gcc/testsuite/g++.dg/warn/Wtautological-compare3.C > > > > create mode 100644 gcc/testsuite/g++.dg/warn/Wtype-limits5.C > > > > > > > > diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c > > > > index dc664ec3798..8aa0bc2c0d8 100644 > > > > --- a/gcc/cp/pt.c > > > > +++ b/gcc/cp/pt.c > > > > @@ -19618,8 +19618,10 @@ tsubst_copy_and_build (tree t, > > > > { > > > > /* If T was type-dependent, suppress warnings that depend on the range > > > > of the types involved. */ > > > > - bool was_dep = type_dependent_expression_p_push (t); > > > > - > > > > + ++processing_template_decl; > > > > + const bool was_dep = (!potential_constant_expression (t) > > > > + || uses_template_parms (t)); > > > > > > We don't want to suppress warnings for a non-constant expression that uses > > > no template parms. So maybe > > > > Fair enough. > > > > > potential_c_e ? value_d : type_d > > > > That works for all the cases I have. > > > > > ? Or perhaps instantiation_dependent_expression_p. > > > > i_d_e_p would still crash in C++98 :(. > > Perhaps we should protect the value_d call in i_d_e_p with potential_c_e? Yeah, probably. But then we should also guard the call to value_d in uses_template_parms. I can apply such a patch if it tests fine, if you want. > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/10? > > OK. Thanks, Marek ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] c++: Prevent warnings for value-dependent exprs [PR96742] 2020-10-28 21:29 ` Marek Polacek @ 2020-10-28 21:48 ` Jason Merrill 2020-10-29 2:45 ` Marek Polacek 2020-10-31 10:09 ` Iain Sandoe 1 sibling, 1 reply; 10+ messages in thread From: Jason Merrill @ 2020-10-28 21:48 UTC (permalink / raw) To: Marek Polacek; +Cc: GCC Patches On 10/28/20 5:29 PM, Marek Polacek wrote: > On Wed, Oct 28, 2020 at 02:46:36PM -0400, Jason Merrill wrote: >> On 10/28/20 2:00 PM, Marek Polacek wrote: >>> On Tue, Oct 27, 2020 at 01:36:30PM -0400, Jason Merrill wrote: >>>> On 10/24/20 6:52 PM, Marek Polacek wrote: >>>>> Here, in r11-155, I changed the call to uses_template_parms to >>>>> type_dependent_expression_p_push to avoid a crash in C++98 in >>>>> value_dependent_expression_p on a non-constant expression. But that >>>>> prompted a host of complaints that we now warn for value-dependent >>>>> expressions in templates. Those warnings are technically valid, but >>>>> people still don't want them because they're awkward to avoid. So let's >>>>> partially revert my earlier fix and make sure that we don't ICE in >>>>> value_dependent_expression_p by checking potential_constant_expression >>>>> first. >>>>> >>>>> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/10? >>>>> >>>>> gcc/cp/ChangeLog: >>>>> >>>>> PR c++/96675 >>>>> PR c++/96742 >>>>> * pt.c (tsubst_copy_and_build): Call uses_template_parms instead of >>>>> type_dependent_expression_p_push. Only call uses_template_parms >>>>> for expressions that are potential_constant_expression. >>>>> >>>>> gcc/testsuite/ChangeLog: >>>>> >>>>> PR c++/96675 >>>>> PR c++/96742 >>>>> * g++.dg/warn/Wdiv-by-zero-3.C: Turn dg-warning into dg-bogus. >>>>> * g++.dg/warn/Wtautological-compare3.C: New test. >>>>> * g++.dg/warn/Wtype-limits5.C: New test. >>>>> * g++.old-deja/g++.pt/crash10.C: Remove dg-warning. >>>>> --- >>>>> gcc/cp/pt.c | 6 ++++-- >>>>> gcc/testsuite/g++.dg/warn/Wdiv-by-zero-3.C | 6 ++++-- >>>>> gcc/testsuite/g++.dg/warn/Wtautological-compare3.C | 11 +++++++++++ >>>>> gcc/testsuite/g++.dg/warn/Wtype-limits5.C | 11 +++++++++++ >>>>> gcc/testsuite/g++.old-deja/g++.pt/crash10.C | 1 - >>>>> 5 files changed, 30 insertions(+), 5 deletions(-) >>>>> create mode 100644 gcc/testsuite/g++.dg/warn/Wtautological-compare3.C >>>>> create mode 100644 gcc/testsuite/g++.dg/warn/Wtype-limits5.C >>>>> >>>>> diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c >>>>> index dc664ec3798..8aa0bc2c0d8 100644 >>>>> --- a/gcc/cp/pt.c >>>>> +++ b/gcc/cp/pt.c >>>>> @@ -19618,8 +19618,10 @@ tsubst_copy_and_build (tree t, >>>>> { >>>>> /* If T was type-dependent, suppress warnings that depend on the range >>>>> of the types involved. */ >>>>> - bool was_dep = type_dependent_expression_p_push (t); >>>>> - >>>>> + ++processing_template_decl; >>>>> + const bool was_dep = (!potential_constant_expression (t) >>>>> + || uses_template_parms (t)); >>>> >>>> We don't want to suppress warnings for a non-constant expression that uses >>>> no template parms. So maybe >>> >>> Fair enough. >>> >>>> potential_c_e ? value_d : type_d >>> >>> That works for all the cases I have. >>> >>>> ? Or perhaps instantiation_dependent_expression_p. >>> >>> i_d_e_p would still crash in C++98 :(. >> >> Perhaps we should protect the value_d call in i_d_e_p with potential_c_e? > > Yeah, probably. But then we should also guard the call to value_d in > uses_template_parms. I can apply such a patch if it tests fine, if you > want. Or change uses_template_parms to use i_d. >>> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/10? >> >> OK. > > Thanks, > > Marek > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] c++: Prevent warnings for value-dependent exprs [PR96742] 2020-10-28 21:48 ` Jason Merrill @ 2020-10-29 2:45 ` Marek Polacek 2020-10-29 15:15 ` Jason Merrill 0 siblings, 1 reply; 10+ messages in thread From: Marek Polacek @ 2020-10-29 2:45 UTC (permalink / raw) To: Jason Merrill; +Cc: GCC Patches On Wed, Oct 28, 2020 at 05:48:08PM -0400, Jason Merrill wrote: > On 10/28/20 5:29 PM, Marek Polacek wrote: > > On Wed, Oct 28, 2020 at 02:46:36PM -0400, Jason Merrill wrote: > > > On 10/28/20 2:00 PM, Marek Polacek wrote: > > > > On Tue, Oct 27, 2020 at 01:36:30PM -0400, Jason Merrill wrote: > > > > > On 10/24/20 6:52 PM, Marek Polacek wrote: > > > > > > Here, in r11-155, I changed the call to uses_template_parms to > > > > > > type_dependent_expression_p_push to avoid a crash in C++98 in > > > > > > value_dependent_expression_p on a non-constant expression. But that > > > > > > prompted a host of complaints that we now warn for value-dependent > > > > > > expressions in templates. Those warnings are technically valid, but > > > > > > people still don't want them because they're awkward to avoid. So let's > > > > > > partially revert my earlier fix and make sure that we don't ICE in > > > > > > value_dependent_expression_p by checking potential_constant_expression > > > > > > first. > > > > > > > > > > > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/10? > > > > > > > > > > > > gcc/cp/ChangeLog: > > > > > > > > > > > > PR c++/96675 > > > > > > PR c++/96742 > > > > > > * pt.c (tsubst_copy_and_build): Call uses_template_parms instead of > > > > > > type_dependent_expression_p_push. Only call uses_template_parms > > > > > > for expressions that are potential_constant_expression. > > > > > > > > > > > > gcc/testsuite/ChangeLog: > > > > > > > > > > > > PR c++/96675 > > > > > > PR c++/96742 > > > > > > * g++.dg/warn/Wdiv-by-zero-3.C: Turn dg-warning into dg-bogus. > > > > > > * g++.dg/warn/Wtautological-compare3.C: New test. > > > > > > * g++.dg/warn/Wtype-limits5.C: New test. > > > > > > * g++.old-deja/g++.pt/crash10.C: Remove dg-warning. > > > > > > --- > > > > > > gcc/cp/pt.c | 6 ++++-- > > > > > > gcc/testsuite/g++.dg/warn/Wdiv-by-zero-3.C | 6 ++++-- > > > > > > gcc/testsuite/g++.dg/warn/Wtautological-compare3.C | 11 +++++++++++ > > > > > > gcc/testsuite/g++.dg/warn/Wtype-limits5.C | 11 +++++++++++ > > > > > > gcc/testsuite/g++.old-deja/g++.pt/crash10.C | 1 - > > > > > > 5 files changed, 30 insertions(+), 5 deletions(-) > > > > > > create mode 100644 gcc/testsuite/g++.dg/warn/Wtautological-compare3.C > > > > > > create mode 100644 gcc/testsuite/g++.dg/warn/Wtype-limits5.C > > > > > > > > > > > > diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c > > > > > > index dc664ec3798..8aa0bc2c0d8 100644 > > > > > > --- a/gcc/cp/pt.c > > > > > > +++ b/gcc/cp/pt.c > > > > > > @@ -19618,8 +19618,10 @@ tsubst_copy_and_build (tree t, > > > > > > { > > > > > > /* If T was type-dependent, suppress warnings that depend on the range > > > > > > of the types involved. */ > > > > > > - bool was_dep = type_dependent_expression_p_push (t); > > > > > > - > > > > > > + ++processing_template_decl; > > > > > > + const bool was_dep = (!potential_constant_expression (t) > > > > > > + || uses_template_parms (t)); > > > > > > > > > > We don't want to suppress warnings for a non-constant expression that uses > > > > > no template parms. So maybe > > > > > > > > Fair enough. > > > > > > > > > potential_c_e ? value_d : type_d > > > > > > > > That works for all the cases I have. > > > > > > > > > ? Or perhaps instantiation_dependent_expression_p. > > > > > > > > i_d_e_p would still crash in C++98 :(. > > > > > > Perhaps we should protect the value_d call in i_d_e_p with potential_c_e? > > > > Yeah, probably. But then we should also guard the call to value_d in > > uses_template_parms. I can apply such a patch if it tests fine, if you > > want. > > Or change uses_template_parms to use i_d. Experimenting with this revealed a curious issue: when we have __PRETTY_FUNCTION__ in a template function, we set its DECL_VALUE_EXPR to error_mark_node (cp_make_fname_decl), so potential_c_e returns false when it gets it, but value_dependent_expression_p handles it specially and says true. So this patch --- a/gcc/cp/pt.c +++ b/gcc/cp/pt.c @@ -27277,7 +27277,8 @@ bool instantiation_dependent_expression_p (tree expression) { return (instantiation_dependent_uneval_expression_p (expression) - || value_dependent_expression_p (expression)); + || (potential_constant_expression (expression) + && value_dependent_expression_p (expression))); } /* Like type_dependent_expression_p, but it also works while not processing breaks lambda-generic-pretty1.C. ISTM that potential_c_e should return true for a DECL_PRETTY_FUNCTION_P with error DECL_VALUE_EXPR. Marek ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] c++: Prevent warnings for value-dependent exprs [PR96742] 2020-10-29 2:45 ` Marek Polacek @ 2020-10-29 15:15 ` Jason Merrill 0 siblings, 0 replies; 10+ messages in thread From: Jason Merrill @ 2020-10-29 15:15 UTC (permalink / raw) To: Marek Polacek; +Cc: GCC Patches On 10/28/20 10:45 PM, Marek Polacek wrote: > On Wed, Oct 28, 2020 at 05:48:08PM -0400, Jason Merrill wrote: >> On 10/28/20 5:29 PM, Marek Polacek wrote: >>> On Wed, Oct 28, 2020 at 02:46:36PM -0400, Jason Merrill wrote: >>>> On 10/28/20 2:00 PM, Marek Polacek wrote: >>>>> On Tue, Oct 27, 2020 at 01:36:30PM -0400, Jason Merrill wrote: >>>>>> On 10/24/20 6:52 PM, Marek Polacek wrote: >>>>>>> Here, in r11-155, I changed the call to uses_template_parms to >>>>>>> type_dependent_expression_p_push to avoid a crash in C++98 in >>>>>>> value_dependent_expression_p on a non-constant expression. But that >>>>>>> prompted a host of complaints that we now warn for value-dependent >>>>>>> expressions in templates. Those warnings are technically valid, but >>>>>>> people still don't want them because they're awkward to avoid. So let's >>>>>>> partially revert my earlier fix and make sure that we don't ICE in >>>>>>> value_dependent_expression_p by checking potential_constant_expression >>>>>>> first. >>>>>>> >>>>>>> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/10? >>>>>>> >>>>>>> gcc/cp/ChangeLog: >>>>>>> >>>>>>> PR c++/96675 >>>>>>> PR c++/96742 >>>>>>> * pt.c (tsubst_copy_and_build): Call uses_template_parms instead of >>>>>>> type_dependent_expression_p_push. Only call uses_template_parms >>>>>>> for expressions that are potential_constant_expression. >>>>>>> >>>>>>> gcc/testsuite/ChangeLog: >>>>>>> >>>>>>> PR c++/96675 >>>>>>> PR c++/96742 >>>>>>> * g++.dg/warn/Wdiv-by-zero-3.C: Turn dg-warning into dg-bogus. >>>>>>> * g++.dg/warn/Wtautological-compare3.C: New test. >>>>>>> * g++.dg/warn/Wtype-limits5.C: New test. >>>>>>> * g++.old-deja/g++.pt/crash10.C: Remove dg-warning. >>>>>>> --- >>>>>>> gcc/cp/pt.c | 6 ++++-- >>>>>>> gcc/testsuite/g++.dg/warn/Wdiv-by-zero-3.C | 6 ++++-- >>>>>>> gcc/testsuite/g++.dg/warn/Wtautological-compare3.C | 11 +++++++++++ >>>>>>> gcc/testsuite/g++.dg/warn/Wtype-limits5.C | 11 +++++++++++ >>>>>>> gcc/testsuite/g++.old-deja/g++.pt/crash10.C | 1 - >>>>>>> 5 files changed, 30 insertions(+), 5 deletions(-) >>>>>>> create mode 100644 gcc/testsuite/g++.dg/warn/Wtautological-compare3.C >>>>>>> create mode 100644 gcc/testsuite/g++.dg/warn/Wtype-limits5.C >>>>>>> >>>>>>> diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c >>>>>>> index dc664ec3798..8aa0bc2c0d8 100644 >>>>>>> --- a/gcc/cp/pt.c >>>>>>> +++ b/gcc/cp/pt.c >>>>>>> @@ -19618,8 +19618,10 @@ tsubst_copy_and_build (tree t, >>>>>>> { >>>>>>> /* If T was type-dependent, suppress warnings that depend on the range >>>>>>> of the types involved. */ >>>>>>> - bool was_dep = type_dependent_expression_p_push (t); >>>>>>> - >>>>>>> + ++processing_template_decl; >>>>>>> + const bool was_dep = (!potential_constant_expression (t) >>>>>>> + || uses_template_parms (t)); >>>>>> >>>>>> We don't want to suppress warnings for a non-constant expression that uses >>>>>> no template parms. So maybe >>>>> >>>>> Fair enough. >>>>> >>>>>> potential_c_e ? value_d : type_d >>>>> >>>>> That works for all the cases I have. >>>>> >>>>>> ? Or perhaps instantiation_dependent_expression_p. >>>>> >>>>> i_d_e_p would still crash in C++98 :(. >>>> >>>> Perhaps we should protect the value_d call in i_d_e_p with potential_c_e? >>> >>> Yeah, probably. But then we should also guard the call to value_d in >>> uses_template_parms. I can apply such a patch if it tests fine, if you >>> want. >> >> Or change uses_template_parms to use i_d. > > Experimenting with this revealed a curious issue: when we have > __PRETTY_FUNCTION__ in a template function, we set its DECL_VALUE_EXPR > to error_mark_node (cp_make_fname_decl), so potential_c_e returns false > when it gets it, but value_dependent_expression_p handles it specially > and says true. So this patch > > --- a/gcc/cp/pt.c > +++ b/gcc/cp/pt.c > @@ -27277,7 +27277,8 @@ bool > instantiation_dependent_expression_p (tree expression) > { > return (instantiation_dependent_uneval_expression_p (expression) > - || value_dependent_expression_p (expression)); > + || (potential_constant_expression (expression) > + && value_dependent_expression_p (expression))); > } > > /* Like type_dependent_expression_p, but it also works while not processing > > breaks lambda-generic-pretty1.C. ISTM that potential_c_e should return > true for a DECL_PRETTY_FUNCTION_P with error DECL_VALUE_EXPR. Agreed. Jason ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] c++: Prevent warnings for value-dependent exprs [PR96742] 2020-10-28 21:29 ` Marek Polacek 2020-10-28 21:48 ` Jason Merrill @ 2020-10-31 10:09 ` Iain Sandoe 2020-11-03 18:40 ` Marek Polacek 1 sibling, 1 reply; 10+ messages in thread From: Iain Sandoe @ 2020-10-31 10:09 UTC (permalink / raw) To: Marek Polacek; +Cc: GCC Patches Hi Marek, Marek Polacek via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > On Wed, Oct 28, 2020 at 02:46:36PM -0400, Jason Merrill wrote: >> On 10/28/20 2:00 PM, Marek Polacek wrote: >>> On Tue, Oct 27, 2020 at 01:36:30PM -0400, Jason Merrill wrote: >>>> On 10/24/20 6:52 PM, Marek Polacek wrote: >>>>> Here, in r11-155, I changed the call to uses_template_parms to >>>>> type_dependent_expression_p_push to avoid a crash in C++98 in >>>>> value_dependent_expression_p on a non-constant expression. But that >>>>> prompted a host of complaints that we now warn for value-dependent >>>>> expressions in templates. Those warnings are technically valid, but >>>>> people still don't want them because they're awkward to avoid. So let's >>>>> partially revert my earlier fix and make sure that we don't ICE in >>>>> value_dependent_expression_p by checking potential_constant_expression >>>>> first. >>>>> >>>>> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/10? >>>>> >>>>> gcc/cp/ChangeLog: >>>>> >>>>> PR c++/96675 >>>>> PR c++/96742 >>>>> * pt.c (tsubst_copy_and_build): Call uses_template_parms instead of >>>>> type_dependent_expression_p_push. Only call uses_template_parms >>>>> for expressions that are potential_constant_expression. >>>>> >>>>> gcc/testsuite/ChangeLog: >>>>> >>>>> PR c++/96675 >>>>> PR c++/96742 >>>>> * g++.dg/warn/Wdiv-by-zero-3.C: Turn dg-warning into dg-bogus. >>>>> * g++.dg/warn/Wtautological-compare3.C: New test. >>>>> * g++.dg/warn/Wtype-limits5.C: New test. >>>>> * g++.old-deja/g++.pt/crash10.C: Remove dg-warning. >>>>> — > >>> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/10? >> >> OK. > > Thanks, This regresses Objective-C++ template-4.mm with an ICE (because we don’t have a check for message_send_expr, which can’t be constexpr). I am testing the following fix (which is mostly Objective-C-local), but open to alternate solutions. Personally, I prefer to avoid double-negatives in code but because of the fallthrough to the sorry case, it seems hard to avoid. OK for master if testing is sucessful? (I assume we will also need to do the same on 10.x if you backported already) cheers Iain ======== [PATCH] Objective-C++ : Fix ICE in potential_constant_expression_1. We cannot, as things stand, handle Objective-C tree codes in the switch and deal with this by calling out to a function that has a dummy version when Objective-C is not enabled. Because of the way the logic works (with a fall through to a 'sorry' in case of unhandled expressions), the function reports cases that are known to be unsuitable for constant exprs. The dummy function always reports 'false' and thus will fall through to the 'sorry'. gcc/c-family/ChangeLog: * c-objc.h (objc_non_constant_expr_p): New. * stub-objc.c (objc_non_constant_expr_p): New. gcc/cp/ChangeLog: * constexpr.c (potential_constant_expression_1): Handle expressions known to be non-constant for Objective-C. gcc/objc/ChangeLog: * objc-act.c (objc_non_constant_expr_p): New. --- gcc/c-family/c-objc.h | 1 + gcc/c-family/stub-objc.c | 6 ++++++ gcc/cp/constexpr.c | 2 +- gcc/objc/objc-act.c | 16 +++++++++++++++- 4 files changed, 23 insertions(+), 2 deletions(-) diff --git a/gcc/c-family/c-objc.h b/gcc/c-family/c-objc.h index 6025283e5cf..4577e4f1c7f 100644 --- a/gcc/c-family/c-objc.h +++ b/gcc/c-family/c-objc.h @@ -101,6 +101,7 @@ extern void objc_add_synthesize_declaration (location_t, tree); extern void objc_add_dynamic_declaration (location_t, tree); extern const char * objc_maybe_printable_name (tree, int); extern bool objc_is_property_ref (tree); +extern bool objc_non_constant_expr_p (tree); extern bool objc_string_ref_type_p (tree); extern void objc_check_format_arg (tree, tree); extern void objc_finish_function (void); diff --git a/gcc/c-family/stub-objc.c b/gcc/c-family/stub-objc.c index c30f0b3c67d..9ced68b2b3e 100644 --- a/gcc/c-family/stub-objc.c +++ b/gcc/c-family/stub-objc.c @@ -331,6 +331,12 @@ objc_is_property_ref (tree ARG_UNUSED (node)) return 0; } +bool +objc_non_constant_expr_p (tree ARG_UNUSED (node)) +{ + return 0; +} + tree objc_maybe_build_component_ref (tree ARG_UNUSED (datum), tree ARG_UNUSED (component)) { diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c index b46824f128d..03a826ba250 100644 --- a/gcc/cp/constexpr.c +++ b/gcc/cp/constexpr.c @@ -8410,7 +8410,7 @@ potential_constant_expression_1 (tree t, bool want_rval, bool strict, bool now, return false; default: - if (objc_is_property_ref (t)) + if (objc_non_constant_expr_p (t)) return false; sorry ("unexpected AST of kind %s", get_tree_code_name (TREE_CODE (t))); diff --git a/gcc/objc/objc-act.c b/gcc/objc/objc-act.c index 0393bc44500..c0d07ae9182 100644 --- a/gcc/objc/objc-act.c +++ b/gcc/objc/objc-act.c @@ -1720,7 +1720,6 @@ objc_build_class_component_ref (tree class_name, tree property_ident) } - /* This is used because we don't want to expose PROPERTY_REF to the C/C++ frontends. Maybe we should! */ bool @@ -1732,6 +1731,21 @@ objc_is_property_ref (tree node) return false; } +/* We use this to report tree codes that are known to be invalid in const- + expression contexts. */ +bool +objc_non_constant_expr_p (tree node) +{ + switch (TREE_CODE (node)) + { + default: + return false; + case MESSAGE_SEND_EXPR: + case PROPERTY_REF: + return true; + } +} + /* This function builds a setter call for a PROPERTY_REF (real, for a declared property, or artificial, for a dot-syntax accessor which is not corresponding to a property). 'lhs' must be a PROPERTY_REF -- 2.24.1 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] c++: Prevent warnings for value-dependent exprs [PR96742] 2020-10-31 10:09 ` Iain Sandoe @ 2020-11-03 18:40 ` Marek Polacek 0 siblings, 0 replies; 10+ messages in thread From: Marek Polacek @ 2020-11-03 18:40 UTC (permalink / raw) To: Iain Sandoe; +Cc: GCC Patches On Sat, Oct 31, 2020 at 10:09:32AM +0000, Iain Sandoe wrote: > Hi Marek, > > Marek Polacek via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > > On Wed, Oct 28, 2020 at 02:46:36PM -0400, Jason Merrill wrote: > >> On 10/28/20 2:00 PM, Marek Polacek wrote: > >>> On Tue, Oct 27, 2020 at 01:36:30PM -0400, Jason Merrill wrote: > >>>> On 10/24/20 6:52 PM, Marek Polacek wrote: > >>>>> Here, in r11-155, I changed the call to uses_template_parms to > >>>>> type_dependent_expression_p_push to avoid a crash in C++98 in > >>>>> value_dependent_expression_p on a non-constant expression. But that > >>>>> prompted a host of complaints that we now warn for value-dependent > >>>>> expressions in templates. Those warnings are technically valid, but > >>>>> people still don't want them because they're awkward to avoid. So let's > >>>>> partially revert my earlier fix and make sure that we don't ICE in > >>>>> value_dependent_expression_p by checking potential_constant_expression > >>>>> first. > >>>>> > >>>>> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/10? > >>>>> > >>>>> gcc/cp/ChangeLog: > >>>>> > >>>>> PR c++/96675 > >>>>> PR c++/96742 > >>>>> * pt.c (tsubst_copy_and_build): Call uses_template_parms instead of > >>>>> type_dependent_expression_p_push. Only call uses_template_parms > >>>>> for expressions that are potential_constant_expression. > >>>>> > >>>>> gcc/testsuite/ChangeLog: > >>>>> > >>>>> PR c++/96675 > >>>>> PR c++/96742 > >>>>> * g++.dg/warn/Wdiv-by-zero-3.C: Turn dg-warning into dg-bogus. > >>>>> * g++.dg/warn/Wtautological-compare3.C: New test. > >>>>> * g++.dg/warn/Wtype-limits5.C: New test. > >>>>> * g++.old-deja/g++.pt/crash10.C: Remove dg-warning. > >>>>> — > > > >>> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/10? > >> > >> OK. > > > > Thanks, > > This regresses Objective-C++ template-4.mm with an ICE (because we don’t have a > check for message_send_expr, which can’t be constexpr). Sorry for failing to test Objective-C++ properly. > I am testing the following fix (which is mostly Objective-C-local), but open to alternate > solutions. Personally, I prefer to avoid double-negatives in code but because of the > fallthrough to the sorry case, it seems hard to avoid. > > OK for master if testing is sucessful? > (I assume we will also need to do the same on 10.x if you backported already) > > cheers > Iain > > ======== > > [PATCH] Objective-C++ : Fix ICE in potential_constant_expression_1. > > We cannot, as things stand, handle Objective-C tree codes in > the switch and deal with this by calling out to a function that > has a dummy version when Objective-C is not enabled. > > Because of the way the logic works (with a fall through to a > 'sorry' in case of unhandled expressions), the function reports > cases that are known to be unsuitable for constant exprs. The > dummy function always reports 'false' and thus will fall through > to the 'sorry'. > > gcc/c-family/ChangeLog: > > * c-objc.h (objc_non_constant_expr_p): New. > * stub-objc.c (objc_non_constant_expr_p): New. > > gcc/cp/ChangeLog: > > * constexpr.c (potential_constant_expression_1): Handle > expressions known to be non-constant for Objective-C. > > gcc/objc/ChangeLog: > > * objc-act.c (objc_non_constant_expr_p): New. > --- > gcc/c-family/c-objc.h | 1 + > gcc/c-family/stub-objc.c | 6 ++++++ > gcc/cp/constexpr.c | 2 +- > gcc/objc/objc-act.c | 16 +++++++++++++++- > 4 files changed, 23 insertions(+), 2 deletions(-) > > diff --git a/gcc/c-family/c-objc.h b/gcc/c-family/c-objc.h > index 6025283e5cf..4577e4f1c7f 100644 > --- a/gcc/c-family/c-objc.h > +++ b/gcc/c-family/c-objc.h > @@ -101,6 +101,7 @@ extern void objc_add_synthesize_declaration (location_t, tree); > extern void objc_add_dynamic_declaration (location_t, tree); > extern const char * objc_maybe_printable_name (tree, int); > extern bool objc_is_property_ref (tree); > +extern bool objc_non_constant_expr_p (tree); > extern bool objc_string_ref_type_p (tree); > extern void objc_check_format_arg (tree, tree); > extern void objc_finish_function (void); > diff --git a/gcc/c-family/stub-objc.c b/gcc/c-family/stub-objc.c > index c30f0b3c67d..9ced68b2b3e 100644 > --- a/gcc/c-family/stub-objc.c > +++ b/gcc/c-family/stub-objc.c > @@ -331,6 +331,12 @@ objc_is_property_ref (tree ARG_UNUSED (node)) > return 0; > } > > +bool > +objc_non_constant_expr_p (tree ARG_UNUSED (node)) In new code we can just omit the name of the parameter: (tree) I have no other comments otherwise. I think you can go ahead with this change. Thanks. Marek ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-11-03 18:40 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-10-24 22:52 [PATCH] c++: Prevent warnings for value-dependent exprs [PR96742] Marek Polacek 2020-10-27 17:36 ` Jason Merrill 2020-10-28 18:00 ` [PATCH v2] " Marek Polacek 2020-10-28 18:46 ` Jason Merrill 2020-10-28 21:29 ` Marek Polacek 2020-10-28 21:48 ` Jason Merrill 2020-10-29 2:45 ` Marek Polacek 2020-10-29 15:15 ` Jason Merrill 2020-10-31 10:09 ` Iain Sandoe 2020-11-03 18:40 ` Marek Polacek
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).