diff mbox

[C++] PR 53158

Message ID 4FAA6E14.7060106@oracle.com
State New
Headers show

Commit Message

Paolo Carlini May 9, 2012, 1:16 p.m. UTC 
Hi,

On 05/09/2012 02:59 PM, Jason Merrill wrote:
> On 05/09/2012 08:21 AM, Paolo Carlini wrote:
>> Is unapplied because I was really nervous due to the wrong location
>> (thus caret) of the error call, at the end of the whole condition. Now,
>> I'm wondering, shall we consistently use error_at (location_of (expr),
>> ... for the error messages produced by the *convert* functions?
>
> Sure.  Note that as an alternative to error_at (location_of (expr) you 
> can just use %q+E; the + means "use the location of this argument".
A great, very concise, didn't know that (but maybe we want to make 
Manuel happier ;)
>> The
>> below quick fix makes me *much* more happy, the caret points to the
>> closed round brace of the b() call. Can I trust all the exprs to come
>> with an embedded location *at least* as accurate as input_location,
>> normally better?
> I would think so, yes.  If an expression doesn't have a location, 
> location_of/%q+E will just use input_location.
In the meanwhile I found a "counterexample" ;) Consider:

enum E { e1 };

E e = e1;

void bar(int a)
{
   if (e = a)
     ;
}

right now the location for the "invalid conversion from ‘int’ to ‘E’" 
error message seems pretty good, points to the a in  "if (e = a)". If I do:


then an horrible thing happen: the error message points to the b in bar 
in "void bar(int a)" !!! What the heck has that to do with the actual 
conversion two lines below?!? Isn't even pointing to the a in "void 
bar(int a)"!

So... I guess I could start experimenting with my idea, but we should be 
ready to fix regressions... Or we can already take from the 
"counterexample" a general lesson? Or maybe a subset of the conversion* 
functions, those in cvt.c?, seems first blush safer to tweak?

Paolo.

Comments

Jason Merrill May 9, 2012, 1:20 p.m. UTC | #1 
I think the problem is you really want to use EXPR_LOC_OR_HERE rather 
than location_of; if the argument happens to be a DECL, location_of will 
give you the declaration location rather than the use location.

Jason
Paolo Carlini May 9, 2012, 1:23 p.m. UTC | #2 
On 05/09/2012 03:20 PM, Jason Merrill wrote:
> I think the problem is you really want to use EXPR_LOC_OR_HERE rather 
> than location_of; if the argument happens to be a DECL, location_of 
> will give you the declaration location rather than the use location.
Ah! Thanks a lot, now all those small details I always see in the 
diagnostics are becoming much more clear ;) Let's see what I can do...

Paolo.
diff mbox

Patch

Index: call.c
===================================================================
--- call.c      (revision 187290)
+++ call.c      (working copy)
@@ -5704,7 +5704,7 @@  convert_like_real (conversion *convs, tree expr, t
             break;
         }

-      permerror (input_location, "invalid conversion from %qT to %qT",
+      permerror (location_of (expr), "invalid conversion from %qT to %qT",
                  TREE_TYPE (expr), totype);
        if (fn)
         permerror (DECL_SOURCE_LOCATION (fn),