Bug 84231 - [8 Regression] cannot bind non-const lvalue reference of type ‘const char*&’ to an rvalue of type ‘const char*’
Summary: [8 Regression] cannot bind non-const lvalue reference of type ‘const char*&’ ...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 8.0
: P3 normal
Target Milestone: 8.0
Assignee: Alexandre Oliva
URL:
Keywords: patch, rejects-valid
Depends on:
Blocks:
 
Reported: 2018-02-06 12:52 UTC by Jonathan Wakely
Modified: 2018-03-06 06:27 UTC (History)
4 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2018-02-06 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Wakely 2018-02-06 12:52:57 UTC
From https://bugzilla.redhat.com/show_bug.cgi?id=1542187

GCC 8 rejects this valid code, but only inside a template (either in a function template, or a member function of a class template):

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");

  // 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);
}
Comment 1 Jonathan Wakely 2018-02-06 13:00:56 UTC
Bisection suggests r255605 although it doesn't look very relevant.
Comment 2 Alexandre Oliva 2018-02-07 09:13:03 UTC
Mine
Comment 3 Alexandre Oliva 2018-02-07 10:27:19 UTC
The difference arises because, when resolving the % overload in normal_function, the result operands of the ternary operator have gained rvalue-forcing NOP_EXPRs, which makes their lvalue_kind clk_none, so the ref-binding is a bad conversion and the non-const template is not viable, but within function_template, the rvalue-forcing casts aren't there, so lvalue_kind for the ternary operator results is clk_ordinary, and reference_binding is regarded as acceptable, and so both operator%s are viable, and the one without const is later selected as a better match.
I suppose the parsing of the ternary operator should behave the same way, given that nothing is dependent in the expression.  But when processing_template_decl, build_x_conditional_expr creates a COND_EXPR node with the original operands.  Oops.
Comment 4 Alexandre Oliva 2018-02-09 02:24:03 UTC
Patch posted:
https://gcc.gnu.org/ml/gcc-patches/2018-02/msg00478.html
Comment 5 Alexandre Oliva 2018-03-06 06:25:44 UTC
Author: aoliva
Date: Tue Mar  6 06:25:12 2018
New Revision: 258271

URL: https://gcc.gnu.org/viewcvs?rev=258271&root=gcc&view=rev
Log:
[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.  Rename
	is_lvalue to is_glvalue.
	* 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.

Added:
    trunk/gcc/testsuite/g++.dg/pr84231.C
Modified:
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/call.c
    trunk/gcc/cp/parser.c
    trunk/gcc/cp/tree.c
    trunk/gcc/cp/typeck.c
    trunk/gcc/testsuite/ChangeLog
Comment 6 Alexandre Oliva 2018-03-06 06:27:06 UTC
Fixed