[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