On Mon, Mar 28, 2011 at 1:08 PM, Eric Botcazou <ebotca...@adacore.com> wrote:
>> This overloads UNKNOWN_LOCATION for both insn_locator and
>> source_location, I don't think this is the best idea. It'll eventually
>> break when compiling with C++ anyway.
>
> Could you elaborate? UNKNOWN_LOCATION isn't used for INSN_LOCATOR at all
> thanks for the curr_insn_locator hunk.
int
curr_insn_locator (void)
{
- if (curr_rtl_loc == -1)
+ if (curr_rtl_loc == -1 || curr_location == UNKNOWN_LOCATION)
return 0;
Oh, I didn't realize curr_location is of type location_t. I'm not very familiar
with these bits. Btw, insn_locators_alloc initializes it with -1 while
it should probably initialize it with UNKNOWN_LCOATION as well.
>> The expand_gimple_stmt change will cause late diagnostic to
>> use an unknown location instead of one from a previously expanded
>> stmt. Probably not ideal but matches what GIMPLE diagnostics
>> would have done earlier.
>
> Huh? What is the line setting input_location for?
Oh, it's contionally set ... not enough coffee.
>> In general I think the patch is a good thing, but the UNKNOWN_LOCATION
>> overloading needs to be resolved. Maybe just add UINKNOWN_LOCATOR?
>> The special value also should be documented somewhere (no idea where
>> core insn-locator functionality resides).
>
> Well, RTL locators are tested against zero all over the place so it's pretty
> clear what zero means. I don't see any overloading here.
So it looks like the patch is perfect after all. And ok.
Thanks,
Richard.
> --
> Eric Botcazou
>
