From patchwork Fri Sep 10 07:48:56 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Thomas Schwinge X-Patchwork-Id: 1526422 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=2620:52:3:1:0:246e:9693:128c; helo=sourceware.org; envelope-from=gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Received: from sourceware.org (server2.sourceware.org [IPv6:2620:52:3:1:0:246e:9693:128c]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4H5Sh33jmqz9sW8 for ; Fri, 10 Sep 2021 17:49:38 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 3B712385743D for ; Fri, 10 Sep 2021 07:49:30 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from esa3.mentor.iphmx.com (esa3.mentor.iphmx.com [68.232.137.180]) by sourceware.org (Postfix) with ESMTPS id EE1EF3858C39 for ; Fri, 10 Sep 2021 07:49:06 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org EE1EF3858C39 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: FBBiL1XeRmnE4xrGcU9dtkk20nEThUOy1d+VIUmooX9LryyJE2bD+1wNDR5eIfUjP2EevZhvv/ P0XbdkfoLjfCbkFnRQpBSEIqwPKbFruBIrO/6vhp/ALiQI4Iv5HRCCswP5wAvs4UbBTkPYBIiR elBB/qrSzC1wTcrhQ4AspI8QFTEF9tJ1P2ONCxAsjARCZmvm2DGW/GCcfT2d9WXHIkkUjxEsaN +uhaXLGFgvSuNi17jLWydijVa9o4Ffghx6SfJIjUCn7Xk9XGoqL9JRnLrsWtPd3KiS0JLD2AFt JerLM+wl3H88b1u1ioMVV8lD X-IronPort-AV: E=Sophos;i="5.85,282,1624348800"; d="scan'208,223";a="65698135" Received: from orw-gwy-01-in.mentorg.com ([192.94.38.165]) by esa3.mentor.iphmx.com with ESMTP; 09 Sep 2021 23:49:06 -0800 IronPort-SDR: YDorM1fHynGlUk+BYvp1KIJ98b2SDY09wTzYFt+ucXExKXcnWz65s86CkHbnoLo81PplAAK7CX gAAgG8EcbdiJVGv+uPaiAzI2YfZ/SP0i5JiS1Ic4G3zQIFta539N13aEbMGsEF85bHsmbY6Yr2 lo1O5EDCF1CYl4NdxzP12iGzk7iwoJa9qbIkQTy3dFDXKvuvNqk3ASfPDq5fSDP8u1jrrhrhOA cG6FQw6x7JuU+r0dWygJiTR4o2y26T4KQQDFxOPBOg8zP8dKl4hLl5/Gm//tv9h/ZXwMUpg/pZ RMc= From: Thomas Schwinge To: David Malcolm , Subject: [PING] Re: [Committed] [PATCH 2/4] (v4) On-demand locations within string-literals In-Reply-To: <87k0jxobz2.fsf@dem-tschwing-1.ger.mentorg.com> 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> <87mtouoku5.fsf@dem-tschwing-1.ger.mentorg.com> <87k0jxobz2.fsf@dem-tschwing-1.ger.mentorg.com> User-Agent: Notmuch/0.29.3+94~g74c3f1b (https://notmuchmail.org) Emacs/27.1 (x86_64-pc-linux-gnu) Date: Fri, 10 Sep 2021 09:48:56 +0200 Message-ID: <87r1dwuazb.fsf@euler.schwinge.homeip.net> MIME-Version: 1.0 X-Originating-IP: [137.202.0.90] X-ClientProxiedBy: SVR-IES-MBX-07.mgc.mentorg.com (139.181.222.7) 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: , Errors-To: gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org Sender: "Gcc-patches" Hi! Ping. My patches again attached, for easy reference. Grüße Thomas On 2021-09-03T18:33:37+0200, I wrote: > Hi! > > On 2021-09-02T21:09:54+0200, I wrote: >> 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 = linemap_resolve_location (line_table, loc, LRK_SPELLING_LOCATION, >>>> + NULL); >>>> + >>>> + loc = 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 = 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 think I convinced myself that the current code doesn't have stable > behavior, so... > >> 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'. > > ... I didn't do this, but instead would like to push the attached > "Don't record string concatenation data for 'RESERVED_LOCATION_P'" > (replacing "Harden 'gcc/input.c:string_concat_db::get_key_loc'" as > originally proposed). OK? > > > ... and then re: > >>>> --- 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'"? > > Attached again. > > > Grüße > Thomas ----------------- Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955 From 349a3172f64db93ee98ea39b36489b702b6596ab Mon Sep 17 00:00:00 2001 From: Thomas Schwinge Date: Tue, 31 Aug 2021 23:30:25 +0200 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 for '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', thus able + to use 'UNKNOWN_LOCATION'/'BUILTINS_LOCATION' as spare values for + 'Empty'/'Deleted'. */ +/* If the following is used more than once, 'gengtype' generates duplicate + functions (thus: "error: redefinition of 'void gt_ggc_mx(location_hash&)'" + 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: -- 2.33.0