[PATCH v2] c++: ICE in value_dependent_expression_p in C++98 mode [PR94938]
Jason Merrill
jason@redhat.com
Wed May 6 21:27:58 GMT 2020
On 5/5/20 5:35 PM, Marek Polacek wrote:
> On Mon, May 04, 2020 at 09:18:57PM -0400, Jason Merrill wrote:
>> On 5/4/20 7:32 PM, Marek Polacek wrote:
>>> Here we ICE with -std=c++98 since the newly added call to uses_template_parms
>>> (r10-6357): we hit
>>> 26530 gcc_assert (cxx_dialect >= cxx11
>>> 26531 || INTEGRAL_OR_ENUMERATION_TYPE_P (type));
>>> and TYPE is a record type. The problem is that the argument to
>>> value_dependent_expression_p does not satisfy potential_constant_expression
>>> which it must, as the comment explains. I thought about fixing this in
>>> uses_template_parms -- only call v_d_e_p if p_c_e is true, but in this
>>> case we want to also suppress the warnings if we don't have a constant
>>> expression. I couldn't simply check TREE_CONSTANT as in
>>> compute_array_index_type_loc, because then we'd stop warning in the new
>>> Wtype-limits3.C test.
>>>
>>> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/10?
>>>
>>> PR c++/94938
>>> * pt.c (tsubst_copy_and_build): Don't call value_dependent_expression_p
>>> if the expression is not potential_constant_expression.
>>>
>>> * g++.dg/warn/template-2.C: New test.
>>> ---
>>> gcc/cp/pt.c | 5 ++++-
>>> gcc/testsuite/g++.dg/warn/template-2.C | 22 ++++++++++++++++++++++
>>> 2 files changed, 26 insertions(+), 1 deletion(-)
>>> create mode 100644 gcc/testsuite/g++.dg/warn/template-2.C
>>>
>>> diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
>>> index 61cb75bf801..691f6e4df36 100644
>>> --- a/gcc/cp/pt.c
>>> +++ b/gcc/cp/pt.c
>>> @@ -19423,7 +19423,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 = uses_template_parms (t);
>>> + ++processing_template_decl;
>>> + bool was_dep = (!potential_constant_expression (t)
>>> + || value_dependent_expression_p (t));
>>> + --processing_template_decl;
>>
>> Hmm, it seems weird to condition the warning on whether or not the
>> expression is constant.
>>
>> Do we want type_dependent_expression_p or
>> instantiation_dependent_expression_p here?
>
> So the question is essentially do we want to suppress the warnings for
> value-dependent but not type-dependent expressions, like sizeof (T)? I think
> we don't need to, the second paragraph in my commit message explains why.
> clang++ also warns for the new Wdiv-by-zero-3.C test. Further, we already have
> a _push variant of type_dependent_expression_p which is convenient.
>
>> I also wonder if we want to move the warning_sentinels after the RECURs.
>
> ...because we only mean to use them for build_x_binary_op purposes, not various
> tsubst_* in the RECURs? Makes sense, done.
>
> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/10.2?
OK.
> -- >8 --
> Here we ICE with -std=c++98 since the newly added call to uses_template_parms
> (r10-6357): we hit
> 26530 gcc_assert (cxx_dialect >= cxx11
> 26531 || INTEGRAL_OR_ENUMERATION_TYPE_P (type));
> and TYPE is a record type. The problem is that the argument to
> value_dependent_expression_p does not satisfy potential_constant_expression
> which it must, as the comment explains. I thought about fixing this in
> uses_template_parms -- only call v_d_e_p if p_c_e is true, but in this
> case we want to also suppress the warnings if we don't have a constant
> expression. I couldn't simply check TREE_CONSTANT as in
> compute_array_index_type_loc, because then we'd stop warning in the new
> Wtype-limits3.C test.
>
> Fixed by using type_dependent_expression_p_push instead. This means
> that we won't suppress the warnings for value-dependent expressions that
> aren't type-dependent, e.g. sizeof (T). This only seems to make a
> difference for -Wdiv-by-zero, now tested in Wdiv-by-zero-3.C, where I
> think it's reasonable to warn. It could make -Wtautological-compare
> warn more, but that warning doesn't trigger when it gets constant arguments.
> Wtype-limits4.C is a test reduced from poly-int.h and it tests a scenario
> that was missing in our testsuite.
>
> This patch also moves the warning_sentinels after the RECURs -- we mean
> to use them for build_x_binary_op purposes only.
>
> PR c++/94938
> * pt.c (tsubst_copy_and_build): Call type_dependent_expression_p_push
> instead of uses_template_parms. Move the warning_sentinels after the
> RECURs.
>
> * g++.dg/warn/Wdiv-by-zero-3.C: New test.
> * g++.dg/warn/Wtype-limits4.C: New test.
> * g++.dg/warn/template-2.C: New test.
> * g++.old-deja/g++.pt/crash10.C: Add dg-warning.
> ---
> gcc/cp/pt.c | 8 ++++---
> gcc/testsuite/g++.dg/warn/Wdiv-by-zero-3.C | 17 +++++++++++++++
> gcc/testsuite/g++.dg/warn/Wtype-limits4.C | 23 +++++++++++++++++++++
> gcc/testsuite/g++.dg/warn/template-2.C | 22 ++++++++++++++++++++
> gcc/testsuite/g++.old-deja/g++.pt/crash10.C | 1 +
> 5 files changed, 68 insertions(+), 3 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/warn/Wdiv-by-zero-3.C
> create mode 100644 gcc/testsuite/g++.dg/warn/Wtype-limits4.C
> create mode 100644 gcc/testsuite/g++.dg/warn/template-2.C
>
> diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
> index ff8391c2093..b1a3317b62e 100644
> --- a/gcc/cp/pt.c
> +++ b/gcc/cp/pt.c
> @@ -19424,14 +19424,16 @@ 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 = uses_template_parms (t);
> + bool was_dep = type_dependent_expression_p_push (t);
> +
> + tree op0 = RECUR (TREE_OPERAND (t, 0));
> + tree op1 = RECUR (TREE_OPERAND (t, 1));
> +
> warning_sentinel s1(warn_type_limits, was_dep);
> warning_sentinel s2(warn_div_by_zero, was_dep);
> warning_sentinel s3(warn_logical_op, was_dep);
> warning_sentinel s4(warn_tautological_compare, was_dep);
>
> - tree op0 = RECUR (TREE_OPERAND (t, 0));
> - tree op1 = RECUR (TREE_OPERAND (t, 1));
> tree r = build_x_binary_op
> (input_location, TREE_CODE (t),
> op0,
> diff --git a/gcc/testsuite/g++.dg/warn/Wdiv-by-zero-3.C b/gcc/testsuite/g++.dg/warn/Wdiv-by-zero-3.C
> new file mode 100644
> index 00000000000..424eb0c3d49
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/warn/Wdiv-by-zero-3.C
> @@ -0,0 +1,17 @@
> +// PR c++/94938
> +
> +template <typename T, int N> int
> +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" }
> + return m1 + m2 + m3 + m4;
> +}
> +
> +void
> +f ()
> +{
> + foo<int, 0>(0, 0);
> +}
> diff --git a/gcc/testsuite/g++.dg/warn/Wtype-limits4.C b/gcc/testsuite/g++.dg/warn/Wtype-limits4.C
> new file mode 100644
> index 00000000000..3ae44b4f25a
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/warn/Wtype-limits4.C
> @@ -0,0 +1,23 @@
> +// PR c++/94938
> +// { dg-additional-options "-Wtype-limits" }
> +
> +template<unsigned N> struct B { unsigned arr[N]; };
> +template<> struct B<1u> { int arr[10]; };
> +
> +template <unsigned N> bool
> +foo(B<N> l)
> +{
> + int i = 0;
> + return l.arr[i] < 0;
> +}
> +
> +void
> +j()
> +{
> + B<1u> b;
> + foo (b);
> + B<2u> b2;
> + // I think that in this instantiation we could warn, but it breaks
> + // gcc bootstrap (marek 5/2020).
> + foo (b2);
> +}
> diff --git a/gcc/testsuite/g++.dg/warn/template-2.C b/gcc/testsuite/g++.dg/warn/template-2.C
> new file mode 100644
> index 00000000000..1d29528b2ac
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/warn/template-2.C
> @@ -0,0 +1,22 @@
> +// PR c++/94938 - ICE in value_dependent_expression_p in C++98 mode.
> +// { dg-do compile }
> +
> +template <typename> struct S { S(); S(bool); };
> +
> +struct C {
> + bool operator()(S<float>);
> +};
> +
> +S<float> fn (bool);
> +
> +template<typename T> void
> +foo (T)
> +{
> + S<float> s;
> + S<float> x = fn(false || C()(s));
> +}
> +
> +int main ()
> +{
> + foo(int());
> +}
> diff --git a/gcc/testsuite/g++.old-deja/g++.pt/crash10.C b/gcc/testsuite/g++.old-deja/g++.pt/crash10.C
> index a84b19004ee..012e3d0c11b 100644
> --- a/gcc/testsuite/g++.old-deja/g++.pt/crash10.C
> +++ b/gcc/testsuite/g++.old-deja/g++.pt/crash10.C
> @@ -6,6 +6,7 @@ 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: e5185cc6be3da99435129cdc0c769d4081e82989
>
More information about the Gcc-patches
mailing list