Hi, This is the second version of the patch based on the previous discussion. In this new version, the major changes are:
1. The name of the option is changed to -flarge-source-files;
2. Add a hint to use this new option “-flarge-source-files” in the routine
“get_visual_column”;
3. Documentation for this new option;
4. Update the testing case location-overflow-test-1.c to include the new hint.
Please take a look at this new patch and let me know any new comment.
thanks.
Qing.
gcc/ChangeLog:
2020-04-22 qing zhao <qing.z...@oracle.com>
PR c/94230
* common.opt: Add -flarge-source-files.
* doc/invoke.texi: Document it.
* toplev.c (process_options): set line_table->default_range_bits
to 0 when flag_large_source_files is true.
gcc/c-family/ChangeLog:
2020-04-22 qing zhao <qing.z...@oracle.com>
PR c/94230
* c-indentation.c (get_visual_column): Add a hint to use the new
-flarge-source-files option.
gcc/testsuite/ChangeLog:
2020-04-22 qing zhao <qing.z...@oracle.com>
PR c/94230
* gcc.dg/plugin/location-overflow-test-1.c (fn_1): New message to
provide hint to use the new -flarge-source-files option.
PR94230.patch
Description: Binary data
> On Apr 22, 2020, at 9:22 AM, Qing Zhao via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
>
> 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
>
