This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: RFC: Make diagnostics for C++ reference binding more consistent


On 27/07/16 18:06 -0400, Jason Merrill wrote:
On Wed, Jul 27, 2016 at 8:05 AM, Jonathan Wakely <jwakely@redhat.com> wrote:
Consider:

template<typename T> T declval();

int& r1 = declval<int&&>();
int&& r2 = declval<int&>();
int& r3 = declval<const int&>();


This produces three quite different errors:


refs.cc:3:25: error: invalid initialization of non-const reference of type
'int&' from an rvalue of type 'int'
int& r1 = declval<int&&>();
          ~~~~~~~~~~~~~~^~
refs.cc:4:25: error: cannot bind 'int' lvalue to 'int&&'
int&& r2 = declval<int&>();
           ~~~~~~~~~~~~~^~
refs.cc:5:30: error: binding 'const int' to reference of type 'int&'
discards qualifiers
int& r3 = declval<const int&>();
          ~~~~~~~~~~~~~~~~~~~^~


The first one uses the order to/from, but the other two are from/to.

The first and second are saying the same thing (wrong value category)
but very differently.

The first and third mention references but the second doesn't.

The standard talks about binding a reference to something, but the
second and third diagnostics talk about binding something to a
reference (and the first doesn't mention binding at all).

The first diagnostic is specific to lvalue references (it wouldn't be
invalid if it was binding a non-const _rvalue_ reference to an
rvalue), but doesn't use the word "lvalue".

(FWIW Clang and EDG both have their own inconsistencies for the same
example, but that shouldn't stop us trying to improve.)


IMHO the first should use the words "bind" and "lvalue reference":

refs.cc:3:25: error: cannot bind non-const lvalue reference of type 'int&'
to an rvalue of type 'int'
int& r1 = declval<int&&>();
          ~~~~~~~~~~~~~~^~

The second should use the phrasing "bind ref to value" instead of
"bind value to ref", and mention "rvalue reference":

refs.cc:4:25: error: cannot bind rvalue reference of type 'int&&' to an
lvalue of type 'int'
int&& r2 = declval<int&>();
           ~~~~~~~~~~~~~^~

The third should use the same phrasing (but doesn't need to mention
lvalue/rvalue because the problem is related to cv-qualifiers not
value categories):

refs.cc:5:30: error: binding reference of type 'int&' to 'const int'
discards qualifiers
int& r3 = declval<const int&>();
          ~~~~~~~~~~~~~~~~~~~^~


I've only considered trivial examples here, is there some reason to
prefer to current diagnostics for more complex cases?

The attached patch makes that change, but there are probably lots of
tests that would need updating which I haven't done until I know if
the change is worthwhile.

Sounds good to me.

Here's the full patch then. There weren't too many tests to adjust.

Tested x86_64-linux. OK for trunk?


commit c54df5dc2010513aeda6b065da843b4a582248e3
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Fri Aug 5 09:51:54 2016 +0100

    Harmonize diagnostics for invalid reference binding
    
    gcc/cp:
    
    	* call.c (convert_like_real): Harmonize diagnostics for invalid
    	reference binding.
    
    gcc/testsuite:
    
    	* call.c (convert_like_real): Harmonize diagnostics for invalid
    	reference binding.
    	* g++.dg/conversion/pr16333.C: Adjust dg-error regexp.
    	* g++.dg/conversion/pr41426.C: Likewise.
    	* g++.dg/conversion/pr66211.C: Likewise.
    	* g++.dg/cpp1y/lambda-init9.C: Likewise.
    	* g++.dg/init/ref8.C: Likewise.
    	* g++.old-deja/g++.law/cvt20.C: Likewise.
    	* g++.old-deja/g++.mike/p9732c.C: Likewise.

diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index 802c325..918661a 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -6710,15 +6710,15 @@ convert_like_real (conversion *convs, tree expr, tree fn, int argnum,
 	    tree extype = TREE_TYPE (expr);
 	    if (TYPE_REF_IS_RVALUE (ref_type)
 		&& lvalue_p (expr))
-	      error_at (loc, "cannot bind %qT lvalue to %qT",
-			extype, totype);
+	      error_at (loc, "cannot bind rvalue reference of type %qT to "
+                        "lvalue of type %qT", totype, extype);
 	    else if (!TYPE_REF_IS_RVALUE (ref_type) && !lvalue_p (expr)
 		     && !CP_TYPE_CONST_NON_VOLATILE_P (TREE_TYPE (ref_type)))
-	      error_at (loc, "invalid initialization of non-const reference of "
-			"type %qT from an rvalue of type %qT", totype, extype);
+	      error_at (loc, "cannot bind non-const lvalue reference of "
+			"type %qT to an rvalue of type %qT", totype, extype);
 	    else if (!reference_compatible_p (TREE_TYPE (totype), extype))
-	      error_at (loc, "binding %qT to reference of type %qT "
-			"discards qualifiers", extype, totype);
+	      error_at (loc, "binding reference of type %qT to %qT "
+			"discards qualifiers", totype, extype);
 	    else
 	      gcc_unreachable ();
 	    maybe_print_user_conv_context (convs);
diff --git a/gcc/testsuite/g++.dg/conversion/pr16333.C b/gcc/testsuite/g++.dg/conversion/pr16333.C
index 810c12a..a00bc5c 100644
--- a/gcc/testsuite/g++.dg/conversion/pr16333.C
+++ b/gcc/testsuite/g++.dg/conversion/pr16333.C
@@ -7,4 +7,4 @@ struct X {
 int a[3];
 X foo1 () { return a; }
 const X &foo2 () { return a; } // { dg-warning "returning reference to temporary" }
-X &foo3 () { return a; } // { dg-error "invalid initialization" }
+X &foo3 () { return a; } // { dg-error "cannot bind non-const lvalue ref" }
diff --git a/gcc/testsuite/g++.dg/conversion/pr41426.C b/gcc/testsuite/g++.dg/conversion/pr41426.C
index 78ec5fb..5493a91 100644
--- a/gcc/testsuite/g++.dg/conversion/pr41426.C
+++ b/gcc/testsuite/g++.dg/conversion/pr41426.C
@@ -23,7 +23,7 @@ const A<float> &g3()
 A<float> &g4()
 {
    float f[] = {1.1f, 2.3f};
-   return f; // { dg-error "invalid initialization" }
+   return f; // { dg-error "cannot bind non-const lvalue ref" }
 }
 
 struct B
diff --git a/gcc/testsuite/g++.dg/conversion/pr66211.C b/gcc/testsuite/g++.dg/conversion/pr66211.C
index 49d2478..770e8a0 100644
--- a/gcc/testsuite/g++.dg/conversion/pr66211.C
+++ b/gcc/testsuite/g++.dg/conversion/pr66211.C
@@ -7,5 +7,5 @@ int main()
 {
   int x = 0;
   double y = 1;
-  f(1 > 0 ? x : y); // { dg-error "from an rvalue" }
+  f(1 > 0 ? x : y); // { dg-error "cannot bind .* to an rvalue" }
 }
diff --git a/gcc/testsuite/g++.dg/cpp1y/lambda-init9.C b/gcc/testsuite/g++.dg/cpp1y/lambda-init9.C
index a86f03e..795d036 100644
--- a/gcc/testsuite/g++.dg/cpp1y/lambda-init9.C
+++ b/gcc/testsuite/g++.dg/cpp1y/lambda-init9.C
@@ -2,5 +2,5 @@
 
 void f()
 {
-  [&x=1]{};   // { dg-error "cannot capture|invalid initialization" }
+  [&x=1]{};   // { dg-error "cannot capture|cannot bind non-const lvalue ref" }
 }
diff --git a/gcc/testsuite/g++.dg/init/ref8.C b/gcc/testsuite/g++.dg/init/ref8.C
index 406cc10..5b68a2f 100644
--- a/gcc/testsuite/g++.dg/init/ref8.C
+++ b/gcc/testsuite/g++.dg/init/ref8.C
@@ -6,5 +6,5 @@ A operator*(A, A);
 
 A& operator+=(A& a, const A& b)
 {
-   return a = a * b;            // { dg-error "non-const reference" }
+   return a = a * b;            // { dg-error "non-const lvalue reference" }
 }
diff --git a/gcc/testsuite/g++.old-deja/g++.law/cvt20.C b/gcc/testsuite/g++.old-deja/g++.law/cvt20.C
index f5c703b..97b465f 100644
--- a/gcc/testsuite/g++.old-deja/g++.law/cvt20.C
+++ b/gcc/testsuite/g++.old-deja/g++.law/cvt20.C
@@ -16,5 +16,5 @@ void f(const char *& x) // { dg-message "argument" }
 
 int main()
 {
-  f ("foo"); // { dg-error "invalid initialization" }
+  f ("foo"); // { dg-error "cannot bind non-const lvalue reference" }
 }
diff --git a/gcc/testsuite/g++.old-deja/g++.mike/p9732c.C b/gcc/testsuite/g++.old-deja/g++.mike/p9732c.C
index 9239a09..247d372 100644
--- a/gcc/testsuite/g++.old-deja/g++.mike/p9732c.C
+++ b/gcc/testsuite/g++.old-deja/g++.mike/p9732c.C
@@ -2,4 +2,4 @@
 // prms-id: 9732
 
 struct foo {};
-foo& x() { return foo(); }	// { dg-error "invalid init" } 
+foo& x() { return foo(); }	// { dg-error "cannot bind non-const lvalue" } 

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]