On Fri, 10 Nov 2023, Jason Merrill wrote:
> 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.
Since tf_decltype is now also set when substituting op1 i.e. f<T>(),
substitution yields a bare CALL_EXPR with no temporary materialization.
The problematic unary_complex_lvalue call happens when binding
the reference parameter 'const B&' to this bare CALL_EXPR. We
take its address via cp_build_addr_expr, which tries
unary_complex_lvalue. The CALL_EXPR handling in unary_complex_lvalue
in turn materializes a temporary for the call, which fails as expected
due to the destructor but also issues the unexpected error since
unary_complex_lvalue unconditionally uses tf_warning_or_error.
So in short the CALL_EXPR handling in unary_complex_lvalue seems to
assume the requirements of temporary materialization have already
been checked.
>
> > 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.
Ah sorry about that, this hunk wasn't intended to be part of the diff.
Here's a full tested patch for consideration. The additional
build_const_cast_1 call in cp_build_c_cast is needed to preserve the
last four errors in gcc/testsuite/g++.dg/tree-ssa/pr20280.C (which get
indirectly issued from unary_complex_lvalue).
-- >8 --
Subject: [PATCH] c++: make unary_complex_lvalue SFINAE-enabled
gcc/cp/ChangeLog:
* class.cc (build_simple_base_path): Adjust call to
unary_complex_value.
* cp-tree.h (unary_complex_lvalue): Add complain parameter.
* typeck.cc (build_class_member_access_expr): Adjust call to
unary_complex_lvalue.
(cp_build_addr_expr_1): Likewise.
(cp_build_unary_op): Likewise.
(unary_complex_lvalue): Add complain parameter and propagate
it. Propagate error_mark_node result from build_cplus_new.
(cp_build_c_cast): If we're committed to a const_cast and
the result is erroneous, call build_const_cast_1 a second time
to issue errors.
---
gcc/cp/class.cc | 2 +-
gcc/cp/cp-tree.h | 2 +-
gcc/cp/typeck.cc | 27 +++++++++++++++++----------
3 files changed, 19 insertions(+), 12 deletions(-)
diff --git a/gcc/cp/class.cc b/gcc/cp/class.cc
index 9d4d95f85bf..58c45542793 100644
--- a/gcc/cp/class.cc
+++ b/gcc/cp/class.cc
@@ -556,7 +556,7 @@ build_simple_base_path (tree expr, tree binfo)
into `(*(a ? &b : &c)).x', and so on. A COND_EXPR is only
an lvalue in the front end; only _DECLs and _REFs are lvalues
in the back end. */
- temp = unary_complex_lvalue (ADDR_EXPR, expr);
+ temp = unary_complex_lvalue (ADDR_EXPR, expr, tf_warning_or_error);
if (temp)
expr = cp_build_fold_indirect_ref (temp);
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 1fa710d7154..d826afcdb5c 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -8130,7 +8130,7 @@ extern tree cp_build_addr_expr (tree,
tsubst_flags_t);
extern tree cp_build_unary_op (enum tree_code, tree, bool,
tsubst_flags_t);
extern tree genericize_compound_lvalue (tree);
-extern tree unary_complex_lvalue (enum tree_code, tree);
+extern tree unary_complex_lvalue (enum tree_code, tree,
tsubst_flags_t);
extern tree build_x_conditional_expr (location_t, tree, tree, tree,
tsubst_flags_t);
extern tree build_x_compound_expr_from_list (tree, expr_list_kind,
diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
index 49afbd8fb5e..be6667c192e 100644
--- a/gcc/cp/typeck.cc
+++ b/gcc/cp/typeck.cc
@@ -2888,7 +2888,7 @@ build_class_member_access_expr (cp_expr object, tree
member,
/* Transform `(a, b).x' into `(*(a, &b)).x', `(a ? b : c).x' into
`(*(a ? &b : &c)).x', and so on. A COND_EXPR is only an lvalue
in the front end; only _DECLs and _REFs are lvalues in the back end. */
- if (tree temp = unary_complex_lvalue (ADDR_EXPR, object))
+ if (tree temp = unary_complex_lvalue (ADDR_EXPR, object, complain))
{
temp = cp_build_fold_indirect_ref (temp);
if (!lvalue_p (object) && lvalue_p (temp))
@@ -7118,7 +7118,7 @@ cp_build_addr_expr_1 (tree arg, bool strict_lvalue,
tsubst_flags_t complain)
/* Handle complex lvalues (when permitted)
by reduction to simpler cases. */
- val = unary_complex_lvalue (ADDR_EXPR, arg);
+ val = unary_complex_lvalue (ADDR_EXPR, arg, complain);
if (val != 0)
return val;
@@ -7411,7 +7411,7 @@ cp_build_unary_op (enum tree_code code, tree xarg, bool
noconvert,
/* Handle complex lvalues (when permitted)
by reduction to simpler cases. */
- val = unary_complex_lvalue (code, arg);
+ val = unary_complex_lvalue (code, arg, complain);
if (val != 0)
goto return_build_unary_op;
@@ -7657,7 +7657,7 @@ genericize_compound_lvalue (tree lvalue)
NULL_TREE. */
tree
-unary_complex_lvalue (enum tree_code code, tree arg)
+unary_complex_lvalue (enum tree_code code, tree arg, tsubst_flags_t complain)
{
/* Inside a template, making these kinds of adjustments is
pointless; we are only concerned with the type of the
@@ -7669,7 +7669,7 @@ unary_complex_lvalue (enum tree_code code, tree arg)
if (TREE_CODE (arg) == COMPOUND_EXPR)
{
tree real_result = cp_build_unary_op (code, TREE_OPERAND (arg, 1), false,
- tf_warning_or_error);
+ complain);
return build2 (COMPOUND_EXPR, TREE_TYPE (real_result),
TREE_OPERAND (arg, 0), real_result);
}
@@ -7677,13 +7677,14 @@ unary_complex_lvalue (enum tree_code code, tree arg)
/* Handle (a ? b : c) used as an "lvalue". */
if (TREE_CODE (arg) == COND_EXPR
|| TREE_CODE (arg) == MIN_EXPR || TREE_CODE (arg) == MAX_EXPR)
- return rationalize_conditional_expr (code, arg, tf_warning_or_error);
+ return rationalize_conditional_expr (code, arg, complain);
/* Handle (a = b), (++a), and (--a) used as an "lvalue". */
if (TREE_CODE (arg) == MODIFY_EXPR
|| TREE_CODE (arg) == PREINCREMENT_EXPR
|| TREE_CODE (arg) == PREDECREMENT_EXPR)
- return unary_complex_lvalue (code, genericize_compound_lvalue (arg));
+ return unary_complex_lvalue (code, genericize_compound_lvalue (arg),
+ complain);
if (code != ADDR_EXPR)
return NULL_TREE;
@@ -7693,7 +7694,7 @@ unary_complex_lvalue (enum tree_code code, tree arg)
|| TREE_CODE (arg) == INIT_EXPR)
{
tree real_result = cp_build_unary_op (code, TREE_OPERAND (arg, 0), false,
- tf_warning_or_error);
+ complain);
arg = build2 (COMPOUND_EXPR, TREE_TYPE (real_result),
arg, real_result);
suppress_warning (arg /* What warning? */);
@@ -7717,7 +7718,11 @@ unary_complex_lvalue (enum tree_code code, tree arg)
if (TREE_CODE (arg) == SAVE_EXPR)
targ = arg;
else
- targ = build_cplus_new (TREE_TYPE (arg), arg, tf_warning_or_error);
+ {
+ targ = build_cplus_new (TREE_TYPE (arg), arg, complain);
+ if (targ == error_mark_node)
+ return error_mark_node;
+ }
return build1 (ADDR_EXPR, build_pointer_type (TREE_TYPE (arg)), targ);
}
@@ -9174,6 +9179,8 @@ cp_build_c_cast (location_t loc, tree type, tree expr,
maybe_warn_about_useless_cast (loc, type, value, complain);
maybe_warn_about_cast_ignoring_quals (loc, type, complain);
}
+ else if (complain & tf_error)
+ build_const_cast_1 (loc, type, value, tf_error, &valid_p);
return result;
}
@@ -9209,7 +9216,7 @@ cp_build_c_cast (location_t loc, tree type, tree expr,
to succeed. */
if (!same_type_p (non_reference (type), non_reference (result_type)))
{
- result = build_const_cast_1 (loc, type, result, false, &valid_p);
+ result = build_const_cast_1 (loc, type, result, tf_none, &valid_p);
gcc_assert (valid_p);
}
return result;
--
2.43.0.rc1
