PING [PATCH][gcc][PR94230]provide an option to change the size limitation for -Wmisleading-indent

[Richard Sandiford]

David Malcolm <dmalcolm@redhat.com> writes:
> On Tue, 2020-04-21 at 15:04 +0100, Richard Sandiford wrote:
>> Qing Zhao via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
>> > Hi,
>> > 
>> > Please take a look at the attached patch and let me know your
>> > comments.
>> > 
>> > Thanks.
>> > 
>> > Qing
>> 
>> Acting as a reviewer of last resort since this isn't really my area,
>
> Sorry; life has been crazy lately.

NP, just thought I'd better justify treading in this area. :-)

>> but FWIW, I agree losing column tracking at the current limit is a
>> genuine regression and should be fixed for GCC 10.
>
> Is this a regression in GCC 10 though?  I think this has been the case
> since GCC 6 onwards.

Agree it's not a regression in GCC 10, but it still seems fair game
as a regression in general.

>> > gcc/ChangeLog:
>> > 
>> > 2020-04-03  qing zhao  <qing.zhao@oracle.com>
>> > 
>> 
>> Please add:
>> 
>> 	PR c/94230
>> 
>> > 	* 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.
>> 
>> > @@ -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.
>> 
>> > +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.
>
>> 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.

Thanks,
Richard