On 16 May 2012 19:09, Paolo Carlini <pcarl...@gmail.com> wrote:
> Hi
>
>> On 16 May 2012 17:41, Jason Merrill <ja...@redhat.com> wrote:
>>> On 05/16/2012 06:54 AM, Paolo Carlini wrote:
>>>>
>>>> isn't the diagnostic machinery able to cope with UNKNOWN_LOCATION? By
>>>> default should be interpreted as input_location, no?
>>>
>>>
>>> That would make sense to me; I don't know if it works that way now, though.
>>
>> No, I don't think it works that way. In fact, if something printed in
>> diagnostics has an unknown location, that seems a bug, because either
>> it is some artificial construct that we should not be giving
>> diagnostics about, or the location passed down is wrong. Of course,
>> for release compilers, we could add a check in
>> diagnostic_report_diagnostic() and use input_location instead. For
>> development compilers we could have a gcc_checking_assert(location !=
>> UNKNOWN_LOCATION). But I am not sure what would happen for such a
>> check.
>>
>> In any case, the general rule should be that input_location (or
>> variants using that) should be only used in the parser (who actually
>> knows what input_location is pointing at). Other functions should use
>> a location coming from somewhere else (an argument or a tree).
>> UNKNOWN_LOCATION should be used for anything that is
>> artificial/compiler-generated. Of course, in some cases, we don't have
>> a good location at the point that we want to set one (because we get
>> unknown_location or the tree doesn't have a location), and getting the
>> right location there may be more work that you want to do at the
>> moment. So I would suggest that you simply look at what the underlying
>> function does by default (most use input_location) and use that with a
>> FIXME (or use the non-loc variant if you don't need to touch that
>> function call).
>
> Thanks Manuel. So... what shoould I do here? Is the LOC_OR_HERE variant (+
> the improvement just mentioned by Jason) right? Should I test and submit
> that? Jason?
I cannot answer because I don't know what are the defaults. What does
build_min_nt use by default?
- return build_min_nt (COMPONENT_REF, object, name, NULL_TREE);
+ return build_min_nt_loc (UNKNOWN_LOCATION, COMPONENT_REF,
+ object, name, NULL_TREE);
does it uses always UNKNOWN_LOCATION like above or you are changing
the behaviour of this call?
- expr = build_new_op (loc, code, LOOKUP_NORMAL, arg1, arg2, NULL_TREE,
+ expr = build_new_op (LOC_OR_HERE (loc), code, LOOKUP_NORMAL,
+ arg1, arg2, NULL_TREE,
overload, complain);
This doesn't seem correct to me because of the reasons Jason gave. If
loc is unknown, you are changing the behaviour.
- expr = build_new_op (input_location, ARRAY_REF, LOOKUP_NORMAL, arg1,
+ expr = build_new_op (LOC_OR_HERE (loc), ARRAY_REF, LOOKUP_NORMAL, arg1,
arg2, NULL_TREE, /*overload=*/NULL, complain);
This (and similar ones that change input_location to LOC_OR_HERE) seem
correct to me because either you have a valid location or things stay
as before. Of course, whether the valid location is more correct than
input_location depends on the context. Which is the really tricky
part, specially because during the transition neither option may be
optimal.
This is hard stuff, so I am very happy that there is someone brave
enough to tackle it! ;-)
Thanks!
Cheers,
Manuel.
