[C++,PR84231] overload on cond_expr in template

Message ID orvaf6gbhl.fsf@lxoliva.fsfla.org
State New
Headers show
Series
  • [C++,PR84231] overload on cond_expr in template
Related show

Commit Message

Alexandre Oliva Feb. 9, 2018, 2:09 a.m.
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 verifies that, if a non-type-dependent COND_EXPRs is
recognized as an rvalues, so is the to-be-template-substituted one
created in its stead, so that overload resolution selects the correct
alternative.

Regstrapped on x86_64- and i686-linux-gnu.  Ok to install?

for  gcc/cp/ChangeLog

	PR c++/84231
	* typeck.c (build_x_conditional_expr): Make sure the
	to-be-tsubsted expr is an rvalue when it should.

for  gcc/testsuite/g++.dg/ChangeLog

	PR c++/84231
	* pr84231.C: New.
---
 gcc/cp/typeck.c                |   16 +++++++++++++++-
 gcc/testsuite/g++.dg/pr84231.C |   29 +++++++++++++++++++++++++++++
 2 files changed, 44 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/g++.dg/pr84231.C

Comments

Jason Merrill Feb. 15, 2018, 7:20 p.m. | #1
On Thu, Feb 8, 2018 at 9:09 PM, Alexandre Oliva <aoliva@redhat.com> wrote:
> +         /* If it was supposed to be an rvalue but it's not, adjust
> +            one of the operands so that any overload resolution
> +            taking this COND_EXPR as an operand makes the correct
> +            decisions.  See c++/84231.  */
> +         TREE_OPERAND (min, 2) = build1_loc (loc, NON_LVALUE_EXPR,
> +                                             TREE_TYPE (min),
> +                                             TREE_OPERAND (min, 2));
> +         EXPR_LOCATION_WRAPPER_P (TREE_OPERAND (min, 2)) = 1;

But that's not true, this isn't a location wrapper, it has semantic
effect.  And would be the first such use of NON_LVALUE_EXPR in a
template.

Since we're already using the type of the COND_EXPR to indicate a
glvalue, maybe lvalue_kind should say that within a template, a
COND_EXPR which got past the early check for reference type is a
prvalue.

Jason

Patch

diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index 83e767829986..25ac44e57772 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -6560,12 +6560,26 @@  build_x_conditional_expr (location_t loc, tree ifexp, tree op1, tree op2,
   expr = build_conditional_expr (loc, ifexp, op1, op2, complain);
   if (processing_template_decl && expr != error_mark_node)
     {
+      bool rval = !glvalue_p (expr);
       tree min = build_min_non_dep (COND_EXPR, expr,
 				    orig_ifexp, orig_op1, orig_op2);
+      bool mrval = !glvalue_p (min);
       /* Remember that the result is an lvalue or xvalue.  */
-      if (glvalue_p (expr) && !glvalue_p (min))
+      if (!rval && mrval)
 	TREE_TYPE (min) = cp_build_reference_type (TREE_TYPE (min),
 						   !lvalue_p (expr));
+      else if (rval && !mrval)
+	{
+	  /* If it was supposed to be an rvalue but it's not, adjust
+	     one of the operands so that any overload resolution
+	     taking this COND_EXPR as an operand makes the correct
+	     decisions.  See c++/84231.  */
+	  TREE_OPERAND (min, 2) = build1_loc (loc, NON_LVALUE_EXPR,
+					      TREE_TYPE (min),
+					      TREE_OPERAND (min, 2));
+	  EXPR_LOCATION_WRAPPER_P (TREE_OPERAND (min, 2)) = 1;
+	  gcc_checking_assert (!glvalue_p (min));
+	}
       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);
+}