[PATCH v2] c++: Fix ICE with { } as template argument [PR94592]

[Jason Merrill]

On 4/20/20 4:44 PM, Marek Polacek wrote:
> On Mon, Apr 20, 2020 at 11:51:59AM -0400, Jason Merrill wrote:
>> On 4/17/20 4:28 PM, Marek Polacek wrote:
>>> As an extension (there should be a CWG about this though), we support
>>> braced-init-list as a template argument, but convert_nontype_argument
>>> had trouble digesting them.  We ICEd because of the double coercion we
>>> perform for template arguments: convert_nontype_argument called from
>>> finish_template_type got a { }, and since a class type was involved and
>>> we were in a template, convert_like created an IMPLICIT_CONV_EXPR.  Then
>>> the second conversion of the same argument crashed in constexpr.c
>>> because the IMPLICIT_CONV_EXPR had gotten wrapped in a TARGET_EXPR.
>>> Another issue was that an IMPLICIT_CONV_EXPR leaked to constexpr.c when
>>> building an aggregate init.
>>>
>>> We should have instantiated the IMPLICIT_CONV_EXPR in the first call to
>>> convert_nontype_argument, but we didn't, because the call to
>>> is_nondependent_constant_expression returned false because it checks
>>> !BRACE_ENCLOSED_INITIALIZER_P.  Then non_dep was false even though the
>>> expression didn't contain anything dependent and we didn't instantiate
>>> it in convert_nontype_argument.  I'm not sure what the point
>>> of that BRACE_ENCLOSED_INITIALIZER_P. check was, but removing it doesn't
>>> break anything and fixes these crashes.
>>
>> The point was to avoid trying to get a constant value for an untyped
>> braced-init-list.  I'm happy to remove the check from these functions, but
>> let's add it back to maybe_constant_{value,init}, where it was until
>> r6-7825-geb07f187a471f9a203626aecced17d6947c3cc46 .
> 
> Thanks for the pointer.
> 
>> Or better, put it in cxx_eval_outermost_constant_expr.
> 
> Done here.  Since we didn't set non_constant_p and emit an error, I'm not
> doing it here, either.

Hmm, maybe checking_assert allow_non_constant to make sure we don't need 
an error.  OK with that change.

> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> 
> -- >8 --
> As an extension (there should be a CWG about this though), we support
> braced-init-list as a template argument, but convert_nontype_argument
> had trouble digesting them.  We ICEd because of the double coercion we
> perform for template arguments: convert_nontype_argument called from
> finish_template_type got a { }, and since a class type was involved and
> we were in a template, convert_like created an IMPLICIT_CONV_EXPR.  Then
> the second conversion of the same argument crashed in constexpr.c
> because the IMPLICIT_CONV_EXPR had gotten wrapped in a TARGET_EXPR.
> Another issue was that an IMPLICIT_CONV_EXPR leaked to constexpr.c when
> building an aggregate init.
> 
> We should have instantiated the IMPLICIT_CONV_EXPR in the first call to
> convert_nontype_argument, but we didn't, because the call to
> is_nondependent_constant_expression returned false because it checks
> !BRACE_ENCLOSED_INITIALIZER_P.  Then non_dep was false even though the
> expression didn't contain anything dependent and we didn't instantiate
> it in convert_nontype_argument.  To fix this, check
> BRACE_ENCLOSED_INITIALIZER_P in cxx_eval_outermost_constant_expr rather
> than in is_nondependent_*.
> 
> 	PR c++/94592
> 	* constexpr.c (cxx_eval_outermost_constant_expr): Return when T is
> 	a BRACE_ENCLOSED_INITIALIZER_P.
> 	(is_nondependent_constant_expression): Don't check
> 	BRACE_ENCLOSED_INITIALIZER_P.
> 	(is_nondependent_static_init_expression): Likewise.
> 
> 	* g++.dg/cpp2a/nontype-class34.C: New test.
> 	* g++.dg/cpp2a/nontype-class35.C: New test.
> ---
>   gcc/cp/constexpr.c                           |  5 +++--
>   gcc/testsuite/g++.dg/cpp2a/nontype-class34.C | 16 ++++++++++++++++
>   gcc/testsuite/g++.dg/cpp2a/nontype-class35.C | 17 +++++++++++++++++
>   3 files changed, 36 insertions(+), 2 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/cpp2a/nontype-class34.C
>   create mode 100644 gcc/testsuite/g++.dg/cpp2a/nontype-class35.C
> 
> diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
> index c8e7d083f40..fa592834f0a 100644
> --- a/gcc/cp/constexpr.c
> +++ b/gcc/cp/constexpr.c
> @@ -6534,6 +6534,9 @@ cxx_eval_outermost_constant_expr (tree t, bool allow_non_constant,
>     bool non_constant_p = false;
>     bool overflow_p = false;
>   
> +  if (BRACE_ENCLOSED_INITIALIZER_P (t))
> +    return t;
> +
>     constexpr_global_ctx global_ctx;
>     constexpr_ctx ctx = { &global_ctx, NULL, NULL, NULL, NULL, NULL, NULL,
>   			allow_non_constant, strict,
> @@ -8295,7 +8298,6 @@ bool
>   is_nondependent_constant_expression (tree t)
>   {
>     return (!type_unknown_p (t)
> -	  && !BRACE_ENCLOSED_INITIALIZER_P (t)
>   	  && is_constant_expression (t)
>   	  && !instantiation_dependent_expression_p (t));
>   }
> @@ -8307,7 +8309,6 @@ bool
>   is_nondependent_static_init_expression (tree t)
>   {
>     return (!type_unknown_p (t)
> -	  && !BRACE_ENCLOSED_INITIALIZER_P (t)
>   	  && is_static_init_expression (t)
>   	  && !instantiation_dependent_expression_p (t));
>   }
> diff --git a/gcc/testsuite/g++.dg/cpp2a/nontype-class34.C b/gcc/testsuite/g++.dg/cpp2a/nontype-class34.C
> new file mode 100644
> index 00000000000..2d3ba018618
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp2a/nontype-class34.C
> @@ -0,0 +1,16 @@
> +// PR c++/94592 - ICE with { } as template argument.
> +// { dg-do compile { target c++2a } }
> +
> +struct A {
> +  constexpr A() {}
> +};
> +
> +template <A> struct B { };
> +
> +template<typename> void bar () {
> +    B<{}> var;
> +}
> +
> +void fu() {
> +    bar<int>();
> +}
> diff --git a/gcc/testsuite/g++.dg/cpp2a/nontype-class35.C b/gcc/testsuite/g++.dg/cpp2a/nontype-class35.C
> new file mode 100644
> index 00000000000..78cf0a39c81
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp2a/nontype-class35.C
> @@ -0,0 +1,17 @@
> +// PR c++/94592 - ICE with { } as template argument.
> +// { dg-do compile { target c++2a } }
> +
> +struct A {
> +    int i;
> +    constexpr A(int n) : i(n) {}
> +};
> +
> +template <A a> struct B { int i; constexpr B() : i(a.i) { } };
> +
> +template<typename> void bar () {
> +    B<{1}> var;
> +}
> +
> +void fu() {
> +    bar<int>();
> +}
> 
> base-commit: 5bdd4c5d3fc9c143e8edea3b10828e4b75d7a385
>