[/,RFC] Improving more locations for binary expressions

Hi,

I'm looking into making more progress on those locations, for another 
class of cases. Consider:

struct T { };

T foo();

void bar(int a, int b)
{
   if (foo() && a < b)
     ;
}

thus, in this case, we have a class type T, instead of void. The error 
message is:

a.cc: In function ‘void bar(int, int)’:
a.cc:7:20: error: no match for ‘operator&&’ (operand types are ‘T’ and 
‘bool’)
    if (foo() && a < b)
                     ^
a.cc:7:20: note: candidate is:
a.cc:7:20: note: operator&&(bool, bool) <built-in>
    if (foo() && a < b)
                     ^
a.cc:7:20: note:   no known conversion for argument 1 from ‘T’ to ‘bool’

in case my message ends up garbled, the carets do not point to && 
(column 13), two times point to b (column 20), which is obviously wrong. 
In other terms, all the columns are 20, all wrong.

Now, a first step toward fixing this is improving on the work I did some 
days ago on cp_parser_binary_expression: make sure that the location we 
are eventually passing to build_x_binary_op is always, provably, the 
location of the operator. It seems to me that In order to achieve that, 
the right thing to do is always handling tree_type and loc (the location 
of the operator) together, per the attached patch_parser (the 
initialization of loc to input_location is just to prevent uninitialized 
warnings)

Then, with patch_parser applied (it passes testing), we end up with a 
correct first caret for the testcase above, that for "no match", but the 
second one remains wrong. What happens is that the location is correct 
up to the print_z_candidates call. It gets a correct location as 
argument, and prints the "candidate is" message with the right column, 
13. Then it calls print_z_candidate without passing a location, which, 
in turn:

     - Prints the <built-in> message with input_location as location, 
which is 20, wrong.
     - Then calls print_conversion_rejection passing location_of 
(candidate->fn) which is again 20, again wrong.

Now, if I change print_z_candidate to get the location_t from 
print_z_candidates and use it for both the last two outputs, I finally get:

a.cc: In function ‘void bar(int, int)’:
a.cc:7:13: error: no match for ‘operator&&’ (operand types are ‘T’ and 
‘bool’)
    if (foo() && a < b)
              ^
a.cc:7:13: note: candidate is:
a.cc:7:13: note: operator&&(bool, bool) <built-in>
a.cc:7:13: note:   no known conversion for argument 1 from ‘T’ to ‘bool’

again, in case the text is garbled, all the locations seems finally 
right to me, and, interestingly there is no caret at the end, which 
seems Ok.

Now, what I suspect we are doing wrong here, is that we are not handling 
separately, in terms of locations, the *builtin* overloads, which should 
simply use always the location passed from upstream (as I did in my 
quick experiment) from all the other overloads, for which the current 
code seems fine. Seems right? The patch for this second half of the 
issue seems now also rather simple to me.

Thanks,
Paolo.

///////////////////////

Paolo Carlini <paolo.carlini@oracle.com> writes:
> in case my message ends up garbled, the carets do not point to &&
> (column 13), two times point to b (column 20), which is obviously
> wrong. In other terms, all the columns are 20, all wrong.

The new caret support does seem to have revealed a bunch of places
where the column info in error messages was pretty screwy... I guess
nobody paid much attention to it before... :]

Should these get reported as bugzilla bugs...?

-miles

On 10 May 2012 07:55, Miles Bader <miles@gnu.org> wrote:
> Paolo Carlini <paolo.carlini@oracle.com> writes:
>> in case my message ends up garbled, the carets do not point to &&
>> (column 13), two times point to b (column 20), which is obviously
>> wrong. In other terms, all the columns are 20, all wrong.
>
> The new caret support does seem to have revealed a bunch of places
> where the column info in error messages was pretty screwy... I guess
> nobody paid much attention to it before... :]
>
> Should these get reported as bugzilla bugs...?

In principle, yes. In practice, there are already so many known issues
that adding more would just waste contributors time doing bugzilla
administration.

So help would very much appreciated. In particular, the C FE and the
preprocessor are in much worse shape in terms of locations than the
C++ FE.

Some issues may be hard but many of them are a matter of setting a
breakpoint at the error, going up the frame, and figuring out where
the correct location could be got from. Then passing it down to the
error so it can use the correct location. If you can figure out that
but can't/won't write a patch, then please open a PR.

Cheers,

Manuel.

Looks good.

Jason