From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by sourceware.org (Postfix) with ESMTP id B48DE384B0C1 for ; Tue, 21 Apr 2020 18:46:28 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org B48DE384B0C1 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=arm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=richard.sandiford@arm.com Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 2AF281FB; Tue, 21 Apr 2020 11:46:28 -0700 (PDT) Received: from localhost (e121540-lin.manchester.arm.com [10.32.98.126]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 67FCF3F68F; Tue, 21 Apr 2020 11:46:27 -0700 (PDT) From: Richard Sandiford To: David Malcolm Mail-Followup-To: David Malcolm , Qing Zhao via Gcc-patches , jakub@redhat.com, Qing Zhao , richard.sandiford@arm.com Cc: Qing Zhao via Gcc-patches , jakub@redhat.com, Qing Zhao Subject: Re: PING [PATCH][gcc][PR94230]provide an option to change the size limitation for -Wmisleading-indent References: <616B2BE0-0C66-4736-90E5-ECAD1FA4F8AD@ORACLE.COM> <47B8948D-C9BA-41EC-B55B-AF8117E79C50@ORACLE.COM> Date: Tue, 21 Apr 2020 19:46:26 +0100 In-Reply-To: (David Malcolm's message of "Tue, 21 Apr 2020 11:21:30 -0400") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-16.5 required=5.0 tests=BAYES_00, GIT_PATCH_1, KAM_DMARC_STATUS, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 21 Apr 2020 18:46:30 -0000 David Malcolm writes: > On Tue, 2020-04-21 at 15:04 +0100, Richard Sandiford wrote: >> Qing Zhao via Gcc-patches writes: >> > Hi, >> >=20 >> > Please take a look at the attached patch and let me know your >> > comments. >> >=20 >> > Thanks. >> >=20 >> > Qing >>=20 >> 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: >> >=20 >> > 2020-04-03 qing zhao >> >=20 >>=20 >> Please add: >>=20 >> PR c/94230 >>=20 >> > * 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.=20 >>=20 >> 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. >>=20 >> > @@ -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. >> >=20 >> > +@item -flocation-ranges >> > +@opindex flocation-ranges >>=20 >> Normally the documented option should be the non-default one, >> so -fno-... in this case. >>=20 >> > +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. >>=20 >> 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: >>=20 >> -flarge-source-files >>=20 >> 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 =E2=80=9Cpower users=E2=80=9D who = like to experiment with compiler internals. Thanks, Richard