Hi!

On 2021-09-02T21:09:54+0200, I wrote:
> On 2021-09-02T15:59:14+0200, I wrote:
>> On 2016-08-05T14:16:58-0400, David Malcolm <dmalcolm@redhat.com> wrote:
>>> Committed to trunk as r239175; I'm attaching the final version of the
>>> patch for reference.
>>
>> David, you've added here 'gcc/input.h:struct location_hash' (see quoted
>> below), which will be useful elsewhere, so:
>>
>>> --- a/gcc/input.c
>>> +++ b/gcc/input.c
>>
>>> +/* Internal function.  Canonicalize LOC into a form suitable for
>>> +   use as a key within the database, stripping away macro expansion,
>>> +   ad-hoc information, and range information, using the location of
>>> +   the start of LOC within an ordinary linemap.  */
>>> +
>>> +location_t
>>> +string_concat_db::get_key_loc (location_t loc)
>>> +{
>>> +  loc = linemap_resolve_location (line_table, loc, LRK_SPELLING_LOCATION,
>>> +                             NULL);
>>> +
>>> +  loc = get_range_from_loc (line_table, loc).m_start;
>>> +
>>> +  return loc;
>>> +}
>>
>> OK to push the attached
>> "Harden 'gcc/input.c:string_concat_db::get_key_loc'"?  (This fell out of
>> my analysis for development work elsewhere.)
>
> My suggested patch was:
>
>     --- a/gcc/input.c
>     +++ b/gcc/input.c
>     @@ -1483,6 +1483,9 @@ string_concat_db::get_key_loc (location_t loc)
>
>        loc = get_range_from_loc (line_table, loc).m_start;
>
>     +  /* Ascertain that 'loc' is valid as a key in 'm_table'.  */
>     +  gcc_checking_assert (!RESERVED_LOCATION_P (loc));
>     +
>        return loc;
>      }
>
> Uh, I should've looked at the correct test logs...  This change actually
> does regress 'c-c++-common/substring-location-PR-87721.c' and
> 'gcc.dg/plugin/diagnostic-test-string-literals-1.c': for these, we do see
> 'BUILTINS_LOCATION' (via 'string_concat_db::record_string_concatenation').
> Unless someone tell me that's unexpected (I'm completely lost in this
> code...)

I think I convinced myself that the current code doesn't have stable
behavior, so...

> I shall change/generalize my changes to provide both a
> 'location_hash' only using 'UNKNOWN_LOCATION' as a spare value for
> 'Empty' (as currently used here) and another variant additionally using
> 'BUILTINS_LOCATION' as spare value for 'Deleted'.

... I didn't do this, but instead would like to push the attached
"Don't record string concatenation data for 'RESERVED_LOCATION_P'"
(replacing "Harden 'gcc/input.c:string_concat_db::get_key_loc'" as
originally proposed).  OK?


... and then re:

>>> --- a/gcc/input.h
>>> +++ b/gcc/input.h
>>
>>> +struct location_hash : int_hash <location_t, UNKNOWN_LOCATION> { };
>>> +
>>> +class GTY(()) string_concat_db
>>> +{
>>> +[...]
>>> +  hash_map <location_hash, string_concat *> *m_table;
>>> +};
>>
>> OK to push the attached
>> "Generalize 'gcc/input.h:struct location_hash'"?

Attached again.


Grüße
 Thomas


-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955