Re: [PATCH] c++: fix tf_decltype manipulation for COMPOUND_EXPR

[Jason Merrill <jason@redhat.com>]

On 11/10/23 12:25, Patrick Palka wrote:
> On Thu, 9 Nov 2023, Jason Merrill wrote:
> 
>> On 11/7/23 10:08, Patrick Palka wrote:
>>> bootstrapped and regtested on x86_64-pc-linxu-gnu, does this look OK for
>>> trunk?
>>>
>>> -- >8 --
>>>
>>> In the COMPOUND_EXPR case of tsubst_expr, we were redundantly clearing
>>> the tf_decltype flag when substituting the LHS and also neglecting to
>>> propagate it when substituting the RHS.  This patch corrects this flag
>>> manipulation, which allows us to accept the below testcase.
>>>
>>> gcc/cp/ChangeLog:
>>>
>>> 	* pt.cc (tsubst_expr) <case COMPOUND_EXPR>: Don't redundantly
>>> 	clear tf_decltype when substituting the LHS.  Propagate
>>> 	tf_decltype when substituting the RHS.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> 	* g++.dg/cpp0x/decltype-call7.C: New test.
>>> ---
>>>    gcc/cp/pt.cc                                | 9 ++++-----
>>>    gcc/testsuite/g++.dg/cpp0x/decltype-call7.C | 9 +++++++++
>>>    2 files changed, 13 insertions(+), 5 deletions(-)
>>>    create mode 100644 gcc/testsuite/g++.dg/cpp0x/decltype-call7.C
>>>
>>> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
>>> index 521749df525..5f879287a58 100644
>>> --- a/gcc/cp/pt.cc
>>> +++ b/gcc/cp/pt.cc
>>> @@ -20382,11 +20382,10 @@ tsubst_expr (tree t, tree args, tsubst_flags_t
>>> complain, tree in_decl)
>>>          case COMPOUND_EXPR:
>>>          {
>>> -	tree op0 = tsubst_expr (TREE_OPERAND (t, 0), args,
>>> -				complain & ~tf_decltype, in_decl);
>>> -	RETURN (build_x_compound_expr (EXPR_LOCATION (t),
>>> -				       op0,
>>> -				       RECUR (TREE_OPERAND (t, 1)),
>>> +	tree op0 = RECUR (TREE_OPERAND (t, 0));
>>> +	tree op1 = tsubst_expr (TREE_OPERAND (t, 1), args,
>>> +				complain|decltype_flag, in_decl);
>>> +	RETURN (build_x_compound_expr (EXPR_LOCATION (t), op0, op1,
>>>    				       templated_operator_saved_lookups (t),
>>>    				       complain|decltype_flag));
>>
>> Hmm, passing decltype_flag to both op1 and the , is concerning.  Can you add a
>> test with overloaded operator, where the RHS is a class with a destructor?
> 
> I'm not sure if this is what you had in mind, but indeed with this patch
> we reject the following with an error outside the immediate context:
> 
>      struct B { ~B() = delete; };
>      template<class T> B f();
> 
>      void operator,(int, const B&);
> 
>      template<class T> decltype(42, f<T>()) g(int) = delete; // #1
>      template<class T> void g(...); // #2
> 
>      int main() {
>        g<B>(0); // should select #2
>      }
> 
> gcc/testsuite/g++.dg/cpp0x/decltype-call8.C: In substitution of ‘template<class T> decltype ((42, f<T>())) g(int) [with T = B]’:
> gcc/testsuite/g++.dg/cpp0x/decltype-call8.C:12:7:   required from here
>     12 |   g<B>(0);
>        |   ~~~~^~~
> gcc/testsuite/g++.dg/cpp0x/decltype-call8.C:8:30: error: use of deleted function ‘B::~B()’
>      8 | template<class T> decltype(42, f<T>()) g(int) = delete; // #1
>        |                            ~~^~~~~~~~
> gcc/testsuite/g++.dg/cpp0x/decltype-call8.C:3:12: note: declared here
>      3 | struct B { ~B() = delete; };
>        |            ^
> 
> Ultimately because unary_complex_lvalue isn't SFINAE-enabled.

Please elaborate; my understanding is that unary_complex_lvalue is 
supposed to be a semantically neutral transformation.

> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> index 4f2cb2cd402..277c81412b9 100644
> --- a/gcc/cp/pt.cc
> +++ b/gcc/cp/pt.cc
> @@ -20386,7 +20386,7 @@ tsubst_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl)
>   				complain|decltype_flag, in_decl);
>   	RETURN (build_x_compound_expr (EXPR_LOCATION (t), op0, op1,
>   				       templated_operator_saved_lookups (t),
> -				       complain|decltype_flag));
> +				       complain));

This looks like it will break if the operator, returns a class with a 
deleted destructor.

Jason