On Feb 27, 2018, Jason Merrill <ja...@redhat.com> wrote:
> Perhaps it would be easier to add the REFERENCE_TYPE in
> build_conditional_expr_1, adjusting result_type based on
> processing_template_decl and is_lvalue.
It is, indeed!
Here's the patch, regstrapped on i686- and x86_64-linux-gnu. The only
unexpected glitch was the need for adjusting the fold expr parser to
deal with an indirect_ref, lest g++.dg/cpp1x/fold6.C would fail to
error at the line with the ternary operator.
Ok to install?
[C++] [PR84231] overload on cond_expr in template
A non-type-dependent COND_EXPR within a template is reconstructed with
the original operands, after one with non-dependent proxies is built to
determine its result type. This is problematic because the operands of
a COND_EXPR determined to be an rvalue may have been converted to denote
their rvalue nature. The reconstructed one, however, won't have such
conversions, so lvalue_kind may not recognize it as an rvalue, which may
lead to e.g. incorrect overload resolution decisions.
If we mistake such a COND_EXPR for an lvalue, overload resolution might
regard a conversion sequence that binds it to a non-const reference as
viable, and then select that over one that binds it to a const
reference. Only after template substitution would we rebuild the
COND_EXPR, realize it is an rvalue, and conclude the reference binding
is ill-formed, but at that point we'd have long discarded any alternate
candidates we could have used.
This patch modifies the logic that determines whether a
(non-type-dependent) COND_EXPR in a template is an lvalue, to rely on
its type, more specifically, on the presence of a REFERENCE_TYPE
wrapper. In order to avoid a type bootstrapping problem, the
REFERENCE_TYPE that wraps the type of some such COND_EXPRs is
introduced earlier, so that we don't have to test for lvalueness of
the expression using the very code that we wish to change.
for gcc/cp/ChangeLog
PR c++/84231
* tree.c (lvalue_kind): Use presence/absence of REFERENCE_TYPE
only while processing template decls.
* typeck.c (build_x_conditional_expr): Move wrapping of
reference type around type...
* call.c (build_conditional_expr_1): ... here.
* parser.c (cp_parser_fold_expression): Catch REFERENCE_REF_P
INDIRECT_REF of COND_EXPR too.
for gcc/testsuite/ChangeLog
PR c++/84231
* g++.dg/pr84231.C: New.
---
gcc/cp/call.c | 3 +++
gcc/cp/parser.c | 4 +++-
gcc/cp/tree.c | 8 ++++++++
gcc/cp/typeck.c | 4 ----
gcc/testsuite/g++.dg/pr84231.C | 29 +++++++++++++++++++++++++++++
5 files changed, 43 insertions(+), 5 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/pr84231.C
diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index 11fe28292fb1..9d98a3d90d25 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -5348,6 +5348,9 @@ build_conditional_expr_1 (location_t loc, tree arg1, tree
arg2, tree arg3,
return error_mark_node;
valid_operands:
+ if (processing_template_decl)
+ result_type = cp_build_reference_type (result_type, !is_lvalue);
+
result = build3_loc (loc, COND_EXPR, result_type, arg1, arg2, arg3);
/* If the ARG2 and ARG3 are the same and don't have side-effects,
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index bcee1214c2f3..c483b6ce25ea 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -4961,7 +4961,9 @@ cp_parser_fold_expression (cp_parser *parser, tree expr1)
else if (is_binary_op (TREE_CODE (expr1)))
error_at (location_of (expr1),
"binary expression in operand of fold-expression");
- else if (TREE_CODE (expr1) == COND_EXPR)
+ else if (TREE_CODE (expr1) == COND_EXPR
+ || (REFERENCE_REF_P (expr1)
+ && TREE_CODE (TREE_OPERAND (expr1, 0)) == COND_EXPR))
error_at (location_of (expr1),
"conditional expression in operand of fold-expression");
diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c
index 9b9e36a1173f..76148c876b71 100644
--- a/gcc/cp/tree.c
+++ b/gcc/cp/tree.c
@@ -194,6 +194,14 @@ lvalue_kind (const_tree ref)
break;
case COND_EXPR:
+ /* Except for type-dependent exprs, a REFERENCE_TYPE will
+ indicate whether its result is an lvalue or so.
+ REFERENCE_TYPEs are handled above, so if we reach this point,
+ we know we got an rvalue, unless we have a type-dependent
+ expr. */
+ if (processing_template_decl
+ && !type_dependent_expression_p (CONST_CAST_TREE (ref)))
+ return clk_none;
op1_lvalue_kind = lvalue_kind (TREE_OPERAND (ref, 1)
? TREE_OPERAND (ref, 1)
: TREE_OPERAND (ref, 0));
diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index 0e7c63dd1973..fba04c49ec2d 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -6565,10 +6565,6 @@ build_x_conditional_expr (location_t loc, tree ifexp,
tree op1, tree op2,
{
tree min = build_min_non_dep (COND_EXPR, expr,
orig_ifexp, orig_op1, orig_op2);
- /* Remember that the result is an lvalue or xvalue. */
- if (glvalue_p (expr) && !glvalue_p (min))
- TREE_TYPE (min) = cp_build_reference_type (TREE_TYPE (min),
- !lvalue_p (expr));
expr = convert_from_reference (min);
}
return expr;
diff --git a/gcc/testsuite/g++.dg/pr84231.C b/gcc/testsuite/g++.dg/pr84231.C
new file mode 100644
index 000000000000..de7c89a2ab69
--- /dev/null
+++ b/gcc/testsuite/g++.dg/pr84231.C
@@ -0,0 +1,29 @@
+// PR c++/84231 - overload resolution with cond_expr in a template
+
+// { dg-do compile }
+
+struct format {
+ template<typename T> format& operator%(const T&) { return *this; }
+ template<typename T> format& operator%(T&) { return *this; }
+};
+
+format f;
+
+template <typename>
+void function_template(bool b)
+{
+ // Compiles OK with array lvalue:
+ f % (b ? "x" : "x");
+
+ // Used to fails with pointer rvalue:
+ f % (b ? "" : "x");
+}
+
+void normal_function(bool b)
+{
+ // Both cases compile OK in non-template function:
+ f % (b ? "x" : "x");
+ f % (b ? "" : "x");
+
+ function_template<void>(b);
+}
--
Alexandre Oliva, freedom fighter http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/ FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer
