From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from esa4.mentor.iphmx.com (esa4.mentor.iphmx.com [68.232.137.252]) by sourceware.org (Postfix) with ESMTPS id 2F24F3860C2D for ; Thu, 2 Sep 2021 19:10:04 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 2F24F3860C2D Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=codesourcery.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=mentor.com IronPort-SDR: XsgPischGzGZ+vedA1ixA6kH24KnTZSkVgjTmfBTSiazoFCKfwcW2LK6mwYO6VgqaTH1sBst6U gBKW3SalTqskN8nECmsFeSaNktYwuTCkpzZtYIa2GojXXW7X1t/LDzFcn+GczztKprKmAY8Q7D NaYcj8/M9enL6/hPAZ7jH1Szc5fuy3udIcBEN3uX3TAFoEIGqwmIPA5fee8fFgjf1Z3V4XtmSa NWIlPgmGlHb/RzFUylXhz8oGG9xccpI9QaAjWdjnMUEZHk/DZMUX/cakVjxUMOuufInV/ZCrMF KMzqMGwTLAi2Bogt2AdugXjM X-IronPort-AV: E=Sophos;i="5.85,263,1624348800"; d="scan'208";a="65630873" Received: from orw-gwy-02-in.mentorg.com ([192.94.38.167]) by esa4.mentor.iphmx.com with ESMTP; 02 Sep 2021 11:10:03 -0800 IronPort-SDR: pcQeCBoOsMaDrVk56SytyJfM3SUxaRT58UteSfebC5mxEgBjUr15QvYys7qoNVVUQ2/5PUg04/ aLG2t2oF6Aim+KrdYI8oYcNizExEta+DooTqLmf+CG7s2KwKNOTux3F7/igehx6EzKJAcjl4Ec uIzdjVwsdBKa6GCTJTG8CRD8Z48DFkIh+AF5hfaci1bxnwFFuz2aHFZUw5Dhk4b7Lm+McKppk6 s5+LOlShNPr6TZhtmPdyosp+zTABnCF6aYTpnb6E5h+qY0BY0E5zem4o5gbYv0VsdML56Zki+J Ivo= From: Thomas Schwinge To: David Malcolm , Subject: Re: [Committed] [PATCH 2/4] (v4) On-demand locations within string-literals In-Reply-To: <871r67w025.fsf@euler.schwinge.homeip.net> References: <1469841382.17384.65.camel@redhat.com> <1470239113-42666-1-git-send-email-dmalcolm@redhat.com> <1470239113-42666-2-git-send-email-dmalcolm@redhat.com> <1470338501.8203.47.camel@redhat.com> <46aafdfa-3cbe-8072-cef1-c827ec144b7e@redhat.com> <1470421018.8203.93.camel@redhat.com> <871r67w025.fsf@euler.schwinge.homeip.net> User-Agent: Notmuch/0.29.1+93~g67ed7df (https://notmuchmail.org) Emacs/26.3 (x86_64-pc-linux-gnu) Date: Thu, 2 Sep 2021 21:09:54 +0200 Message-ID: <87mtouoku5.fsf@dem-tschwing-1.ger.mentorg.com> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable X-Originating-IP: [137.202.0.90] X-ClientProxiedBy: svr-ies-mbx-06.mgc.mentorg.com (139.181.222.6) To svr-ies-mbx-01.mgc.mentorg.com (139.181.222.1) X-Spam-Status: No, score=-12.2 required=5.0 tests=BAYES_00, GIT_PATCH_0, HEADER_FROM_DIFFERENT_DOMAINS, KAM_DMARC_STATUS, SPF_HELO_PASS, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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: Thu, 02 Sep 2021 19:10:06 -0000 Hi! On 2021-09-02T15:59:14+0200, I wrote: > On 2016-08-05T14:16:58-0400, David Malcolm 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 =3D linemap_resolve_location (line_table, loc, LRK_SPELLING_LOCAT= ION, >> + NULL); >> + >> + loc =3D 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 =3D 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 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'. Gr=C3=BC=C3=9Fe Thomas >> --- a/gcc/input.h >> +++ b/gcc/input.h > >> +struct location_hash : int_hash { }; >> + >> +class GTY(()) string_concat_db >> +{ >> +[...] >> + hash_map *m_table; >> +}; > > OK to push the attached > "Generalize 'gcc/input.h:struct location_hash'"? My suggested patch was: > Subject: [PATCH 2/2] Generalize 'gcc/input.h:struct location_hash' > > This is currently only used here ('gcc/input.h:class string_concat_db'), = but is > actually generally useful, so advertize it as such. > > Per the rationale given, we may use 'BUILTINS_LOCATION' as spare value fo= r > 'Deleted', in addition to the existing use of 'UNKNOWN_LOCATION' as spare= value > for 'Empty'. > > gcc/ > * input.h (location_hash): Use 'BUILTINS_LOCATION' as spare value > for 'Deleted'. Turn into a '#define'. > --- > gcc/input.h | 21 +++++++++++++++++++-- > 1 file changed, 19 insertions(+), 2 deletions(-) > > diff --git a/gcc/input.h b/gcc/input.h > index e6881072c5f..46971a2684c 100644 > --- a/gcc/input.h > +++ b/gcc/input.h > @@ -36,6 +36,25 @@ extern GTY(()) class line_maps *saved_line_table; > both UNKNOWN_LOCATION and BUILTINS_LOCATION fit into that. */ > STATIC_ASSERT (BUILTINS_LOCATION < RESERVED_LOCATION_COUNT); > > +/* Hasher for 'location_t' values satisfying '!RESERVED_LOCATION_P', thu= s able > + to use 'UNKNOWN_LOCATION'/'BUILTINS_LOCATION' as spare values for > + 'Empty'/'Deleted'. */ > +/* If the following is used more than once, 'gengtype' generates duplica= te > + functions (thus: "error: redefinition of 'void gt_ggc_mx(location_has= h&)'" > + etc.): > + > + struct location_hash > + : int_hash {}; > + > + Likewise for this: > + > + typedef int_hash > + location_hash; > + > + Thus, use a plain ol' '#define': > +*/ > +#define location_hash int_hash > + > extern bool is_location_from_builtin_token (location_t); > extern expanded_location expand_location (location_t); > > @@ -230,8 +249,6 @@ public: > location_t * GTY ((atomic)) m_locs; > }; > > -struct location_hash : int_hash { }; > - > class GTY(()) string_concat_db > { > public: ----------------- Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstra=C3=9Fe 201= , 80634 M=C3=BCnchen; Gesellschaft mit beschr=C3=A4nkter Haftung; Gesch=C3= =A4ftsf=C3=BChrer: Thomas Heurung, Frank Th=C3=BCrauf; Sitz der Gesellschaf= t: M=C3=BCnchen; Registergericht M=C3=BCnchen, HRB 106955