Hi, Richard And Dave:
Thanks a lot for the review and comments.
> On Apr 21, 2020, at 1:46 PM, Richard Sandiford <richard.sandif...@arm.com>
> wrote:
>
> David Malcolm <dmalc...@redhat.com> writes:
>> On Tue, 2020-04-21 at 15:04 +0100, Richard Sandiford wrote
>>>
>>> Please add:
>>>
>>> PR c/94230
Will do.
>>>
>>>> * common.opt: Add -flocation-ranges.
>>>> * doc/invoke.texi: Document it.
>>>> * toplev.c (process_options): set line_table-
>>>>> default_range_bits
>>>> to 0 when flag_location_ranges is false.
>>>
>>> I think it would be worth adding a hint to use the new option to
>>> get_visual_column, when warning about column tracking being disabled.
>>> This should probably be a second inform(), immediately after the
>>> current one.
Sounds reasonable to me, I will add that.
>>>
>>>> @@ -14151,6 +14151,13 @@ This option may be useful in conjunction
>>>> with the @option{-B} or
>>>> perform additional processing of the program source between
>>>> normal preprocessing and compilation.
>>>>
>>>> +@item -flocation-ranges
>>>> +@opindex flocation-ranges
>>>
>>> Normally the documented option should be the non-default one,
>>> so -fno-... in this case.
Okay.
>>>
>>>> +Enable range tracking when recording source locations.
>>>> +By default, GCC enables range tracking when recording source
>>>> locations.
>>>> +If disable range tracking by -fno-location-ranges, more location
>>>> space
>>>> +will be saved for column tracking.
>>>
>>> My understanding is that the patch doesn't actually disable location-
>>> range
>>> tracking, but simply uses a less efficient form for *all* ranges,
>>> rather
>>> than only using the less efficient form for ranges that aren't "caret
>>> at
>>> start, length < 64 chars".
>>
>> Indeed.
Okay, I see.
Providing a good documentation at the user level for this option might be a
challenge to me, I will try. -:)
>>
>>> I know you're simply following the suggestion in the PR, sorry,
>>
>> Sorry. I did put a caveat on the suggestion FWIW.
>>
>>> but I wonder if the option should instead be:
>>>
>>> -flarge-source-files
>>>
>>> since that seems like a more user-facing concept. The option would
>>> tell GCC that the source files are likely to be very large and that
>>> GCC should adapt accordingly. In particular, the option makes GCC
>>> cope with more source lines at the expense of slowing down
>>> compilation
>>> and using more memory.
>>
>> Another approach would be to go lower-level and introduce a param for
>> this; something like "--param location-range-bits" defaulting to 5; the
>> user can set it to 0 to disable the range-bit optimization to deal with
>> bigger files, and it allows for experimentation without rebuilding the
>> compiler.
>>
>> Again, I don't know if this is a good idea; I'm thinking aloud; I'm not
>> sure what the best direction here is.
>
> The reason I like the -flarge-source-files (suggestion for better
> names welcome) is that the user is giving user-level information and
> letting the compiler decide how to deal with that. What the option
> actually does can change with the implementation as necessary.
>
> Potentially any user could hit the -Wmisleading-indent note, or could
> hit the limit at which columns get dropped from diagnostics. So while
> this option isn't going to be used all that often, it also doesn't feel
> like an option designed specifically for “power users” who like to
> experiment with compiler internals.
Agreed, I prefer to use -flarge-source-files for this functionality.
Let me know if you have any other suggestions for this patch.
Thanks.
Qing
>
> Thanks,
> Richard
