[PATCH] c++: premature analysis of requires-expression [PR96410]

Jason Merrill jason@redhat.com
Thu Aug 13 16:54:01 GMT 2020


On 8/13/20 11:21 AM, Patrick Palka wrote:
> On Mon, 10 Aug 2020, Jason Merrill wrote:
> 
>> On 8/10/20 2:18 PM, Patrick Palka wrote:
>>> On Mon, 10 Aug 2020, Patrick Palka wrote:
>>>
>>>> In the below testcase, semantic analysis of the requires-expressions in
>>>> the generic lambda must be delayed until instantiation of the lambda
>>>> because the requirements depend on the lambda's template arguments.  But
>>>> tsubst_requires_expr does semantic analysis even during regeneration of
>>>> the lambda, which leads to various bogus errors and ICEs since some
>>>> subroutines aren't prepared to handle dependent/template trees.
>>>>
>>>> This patch adjusts subroutines of tsubst_requires_expr to avoid doing
>>>> some problematic semantic analyses when processing_template_decl.
>>>> In particular, expr_noexcept_p generally can't be checked on a dependent
>>>> expression.  Next, tsubst_nested_requirement should avoid checking
>>>> satisfaction when processing_template_decl.  And similarly for
>>>> convert_to_void (called from tsubst_valid_expression_requirement).
>>
>> I wonder if, instead of trying to do a partial substitution into a
>> requires-expression at all, we want to use the
>> PACK_EXPANSION_EXTRA_ARGS/IF_STMT_EXTRA_ARGS mechanism to remember the
>> arguments for later satisfaction?
> 
> IIUC, avoiding partial substitution into a requires-expression would
> mean we'd go from currently accepting the following testcase to
> rejecting it, because we'd now instantiate B<int>::type as part of the
> first requirement before first noticing the SFINAE error in the second
> requirement (which depends only on the outer template argument, and
> which would determine the value of the requires-expression):
> 
>    template<typename T>
>    struct B { using type = T::fatal; };
> 
>    template<typename T>
>    constexpr auto foo() {
>      return [] <typename U> (U) {
>        return requires { typename B<U>::type; typename T::type; };
>      };
>    };
> 
>    int i = foo<int>()(0);
> 
> I guess this is exactly the kind of testcase that motivates using the
> PACK_EXPANSION_EXTRA_ARGS/IF_STMT_EXTRA_ARGS mechanism for
> requires-expressions?

I think so, yes.

>>
>>>> Bootstrapped and regtested on x86_64-pc-linux-gnu, and also tested
>>>> against the cmcstl2 project.  Does this look OK to commit?
>>>>
>>>> gcc/cp/ChangeLog:
>>>>
>>>> 	PR c++/96409
>>>> 	PR c++/96410
>>>> 	* constraint.cc (tsubst_compound_requirement): When
>>>> 	processing_template_decl, don't check noexcept of the
>>>> 	substituted expression.
>>>> 	(tsubst_nested_requirement): Just substitute into the constraint
>>>> 	when processing_template_decl.
>>>> 	* cvt.c (convert_to_void): Don't resolve concept checks when
>>>> 	processing_template_decl.
>>>>
>>>> gcc/testsuite/ChangeLog:
>>>>
>>>> 	PR c++/96409
>>>> 	PR c++/96410
>>>> 	* g++.dg/cpp2a/concepts-lambda13.C: New test.
>>>> ---
>>>>    gcc/cp/constraint.cc                          |  9 ++++++-
>>>>    gcc/cp/cvt.c                                  |  2 +-
>>>>    .../g++.dg/cpp2a/concepts-lambda13.C          | 25 +++++++++++++++++++
>>>>    3 files changed, 34 insertions(+), 2 deletions(-)
>>>>    create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-lambda13.C
>>>>
>>>> diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
>>>> index e4aace596e7..db2036502a7 100644
>>>> --- a/gcc/cp/constraint.cc
>>>> +++ b/gcc/cp/constraint.cc
>>>> @@ -1993,7 +1993,8 @@ tsubst_compound_requirement (tree t, tree args,
>>>> subst_info info)
>>>>        /* Check the noexcept condition.  */
>>>>      bool noexcept_p = COMPOUND_REQ_NOEXCEPT_P (t);
>>>> -  if (noexcept_p && !expr_noexcept_p (expr, tf_none))
>>>> +  if (!processing_template_decl
>>>> +      && noexcept_p && !expr_noexcept_p (expr, tf_none))
>>>>        return error_mark_node;
>>>>        /* Substitute through the type expression, if any.  */
>>>> @@ -2023,6 +2024,12 @@ static tree
>>>>    tsubst_nested_requirement (tree t, tree args, subst_info info)
>>>>    {
>>>>      /* Ensure that we're in an evaluation context prior to satisfaction.
>>>> */
>>>> +  if (processing_template_decl)
>>>> +    {
>>>> +      tree r = tsubst_constraint (TREE_OPERAND (t, 0), args,
>>>> +				  info.complain, info.in_decl);
>>>
>>> Oops, the patch is missing a check for error_mark_node here, so that
>>> upon substitution failure we immediately resolve the requires-expression
>>> to false.  Here's an updated patch with the check and a regression test
>>> added:
>>>
>>> -- >8 --
>>>
>>> gcc/cp/ChangeLog:
>>>
>>> 	PR c++/96409
>>> 	PR c++/96410
>>> 	* constraint.cc (tsubst_compound_requirement): When
>>> 	processing_template_decl, don't check noexcept of the
>>> 	substituted expression.
>>> 	(tsubst_nested_requirement): Just substitute into the constraint
>>> 	when processing_template_decl.
>>> 	* cvt.c (convert_to_void): Don't resolve concept checks when
>>> 	processing_template_decl.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> 	PR c++/96409
>>> 	PR c++/96410
>>> 	* g++.dg/cpp2a/concepts-lambda13.C: New test.
>>> 	* g++.dg/cpp2a/concepts-lambda14.C: New test.
>>> ---
>>>    gcc/cp/constraint.cc                          | 11 +++++++-
>>>    gcc/cp/cvt.c                                  |  2 +-
>>>    .../g++.dg/cpp2a/concepts-lambda13.C          | 25 +++++++++++++++++++
>>>    .../g++.dg/cpp2a/concepts-lambda14.C          | 10 ++++++++
>>>    4 files changed, 46 insertions(+), 2 deletions(-)
>>>    create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-lambda13.C
>>>    create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-lambda14.C
>>>
>>> diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
>>> index e4aace596e7..5b4964dd36e 100644
>>> --- a/gcc/cp/constraint.cc
>>> +++ b/gcc/cp/constraint.cc
>>> @@ -1993,7 +1993,8 @@ tsubst_compound_requirement (tree t, tree args,
>>> subst_info info)
>>>        /* Check the noexcept condition.  */
>>>      bool noexcept_p = COMPOUND_REQ_NOEXCEPT_P (t);
>>> -  if (noexcept_p && !expr_noexcept_p (expr, tf_none))
>>> +  if (!processing_template_decl
>>> +      && noexcept_p && !expr_noexcept_p (expr, tf_none))
>>>        return error_mark_node;
>>>        /* Substitute through the type expression, if any.  */
>>> @@ -2023,6 +2024,14 @@ static tree
>>>    tsubst_nested_requirement (tree t, tree args, subst_info info)
>>>    {
>>>      /* Ensure that we're in an evaluation context prior to satisfaction.  */
>>> +  if (processing_template_decl)
>>> +    {
>>> +      tree r = tsubst_constraint (TREE_OPERAND (t, 0), args,
>>> +				  info.complain, info.in_decl);
>>> +      if (r == error_mark_node)
>>> +	return error_mark_node;
>>> +      return finish_nested_requirement (EXPR_LOCATION (t), r);
>>> +    }
>>>      tree norm = TREE_TYPE (t);
>>>      tree result = satisfy_constraint (norm, args, info);
>>>      if (result == error_mark_node && info.quiet ())
>>> diff --git a/gcc/cp/cvt.c b/gcc/cp/cvt.c
>>> index c9e7b1ff044..1d2c2d3351c 100644
>>> --- a/gcc/cp/cvt.c
>>> +++ b/gcc/cp/cvt.c
>>> @@ -1159,7 +1159,7 @@ convert_to_void (tree expr, impl_conv_void implicit,
>>> tsubst_flags_t complain)
>>>        /* Explicitly evaluate void-converted concept checks since their
>>>         satisfaction may produce ill-formed programs.  */
>>> -   if (concept_check_p (expr))
>>> +   if (!processing_template_decl && concept_check_p (expr))
>>>         expr = evaluate_concept_check (expr, tf_warning_or_error);
>>>        if (VOID_TYPE_P (TREE_TYPE (expr)))
>>> diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-lambda13.C
>>> b/gcc/testsuite/g++.dg/cpp2a/concepts-lambda13.C
>>> new file mode 100644
>>> index 00000000000..9757ce49d67
>>> --- /dev/null
>>> +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-lambda13.C
>>> @@ -0,0 +1,25 @@
>>> +// PR c++/96410
>>> +// { dg-do compile { target c++20 } }
>>> +
>>> +struct S { using blah = void; };
>>> +
>>> +template <typename T> constexpr bool trait = !__is_same(T, S);
>>> +template <typename T> concept C = trait<T>;
>>> +
>>> +template<typename T>
>>> +void foo() noexcept(!__is_same(T, void)) { }
>>> +
>>> +template<typename U>
>>> +auto f() {
>>> +  return []<typename T>(T){
>>> +    static_assert(requires { requires C<U> && (C<T> || C<T>); }); // {
>>> dg-error "assert" }
>>> +    static_assert(requires { C<T>; });
>>> +    static_assert(requires { { foo<T>() } noexcept -> C; });
>>> +    static_assert(!requires { typename T::blah; }); // { dg-error "assert"
>>> }
>>> +    return 0;
>>> +  };
>>> +}
>>> +
>>> +auto g = f<int>(); // { dg-bogus "" }
>>> +int n = g(0); // { dg-bogus "" }
>>> +int m = g(S{}); // { dg-message "required from here" }
>>> diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-lambda14.C
>>> b/gcc/testsuite/g++.dg/cpp2a/concepts-lambda14.C
>>> new file mode 100644
>>> index 00000000000..e6ae73c4872
>>> --- /dev/null
>>> +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-lambda14.C
>>> @@ -0,0 +1,10 @@
>>> +// { dg-do compile { target c++20 } }
>>> +
>>> +template<typename T>
>>> +auto f() {
>>> +  return [](auto){
>>> +    static_assert(requires { requires T::fail; }); // { dg-error "assert" }
>>> +  };
>>> +}
>>> +
>>> +auto g = f<int>(); // { dg-message "required from here" }
>>>
>>
>>
> 



More information about the Gcc-patches mailing list