From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ej1-x62b.google.com (mail-ej1-x62b.google.com [IPv6:2a00:1450:4864:20::62b]) by sourceware.org (Postfix) with ESMTPS id B40EA3858D38; Mon, 3 Oct 2022 16:51:29 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org B40EA3858D38 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-ej1-x62b.google.com with SMTP id f1so5968996ejw.7; Mon, 03 Oct 2022 09:51:29 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date; bh=VWI57UHKsAF2Uo1nA+Wj/qvm86+HGM4dKgghsZEcBVo=; b=kWcy8qTbyKJ+EmMzxcc59dRt6m5sTFslFesQTcAvQQj1XY6bRSRhg5k8TXWXUusypZ vpeblUnlE6x5IUZBnzhrGwi/sbEyS2srzln/bD1SYP/e6bhYcb4g0nXxccSC0PKwOGEv sEAiJWDfMwLj3UvByJkCtNK3ilEeFAOIq5Y+iHjdYmjNRAh5yaZZc22J7m3WTl1gCx0p /I4vkkGJ3WxB3TK2vWnlqXmrfJehxFkIlYHEACMl+w86m37ThZmrsr9rQ+IsnykSWfQW AHgK0UxRy1e4cYiaVl69zmiotXXwKn3Tif3mTgAHuAwufs9lghfCwzE2CCNxxycNHVqn 1MCw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date; bh=VWI57UHKsAF2Uo1nA+Wj/qvm86+HGM4dKgghsZEcBVo=; b=YUqUvpYmVhGXk2pdEpH/tf1Jo6ZoMUOyx0v5WNtRlegL02GDCksUR/y+1fdWNZqpvz 7487A/zCrw1kMzMYfDy2A0MMhOSU9XrN5GF+zFaMpglWIHpPkHggJRYUic9Of/P05ZNt vE5vpRA5YVUfEhBNz17z6Xd9/5V0udIT2kFytCHE1B4bA/QM1pjhOKUz47ewihyyyqVR lyQJTOtBVJfz09vuq+AQSmM0mhVAq5Uqngm/9MqfgShzKqvCb6a4LImh6XvKHc31MTPN bRb/TD6d7/7CGc99vza0wqa86N8qwaj2H5J1QDehFGeWKiLed10Ju8JM4svxWtd4PWnQ eAPg== X-Gm-Message-State: ACrzQf1vDbgN4r3TuW4yDGz00vaviKG+pvC21rKZ5lAexmgUfV183/ZC YHCsUpw+fdj55CLH/nrHiSo= X-Google-Smtp-Source: AMsMyM67TXLdDhw+A1VJ9TZ2nFT682t8zBeF2JCxeqvlaRtCh4OyXUlt3sXnHwaBasWfSwWzpzuqyA== X-Received: by 2002:a17:907:a05a:b0:78c:fdd6:bf5b with SMTP id gz26-20020a170907a05a00b0078cfdd6bf5bmr881169ejc.20.1664815888341; Mon, 03 Oct 2022 09:51:28 -0700 (PDT) Received: from [10.40.2.8] ([109.190.253.15]) by smtp.googlemail.com with ESMTPSA id d27-20020a170906305b00b0078ae0fb3d11sm2584658ejd.54.2022.10.03.09.51.27 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 03 Oct 2022 09:51:27 -0700 (PDT) Message-ID: <36ff419f-ff22-f954-b62b-8d7e62920fb0@gmail.com> Date: Mon, 3 Oct 2022 18:51:25 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.11.0 Subject: Re: [PATCH] Fix gdb printers for std::string Content-Language: en-US To: Jonathan Wakely Cc: "libstdc++@gcc.gnu.org" , gcc-patches References: <1d7b853e-0180-718c-1947-2517272f44e8@gmail.com> <2eb944c4-131e-001d-a3a6-cf1b7aea5614@gmail.com> From: =?UTF-8?Q?Fran=c3=a7ois_Dumont?= In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-0.4 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,NICE_REPLY_A,RCVD_IN_BARRACUDACENTRAL,RCVD_IN_DNSWL_NONE,RCVD_IN_SBL_CSS,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On 01/10/22 17:30, Jonathan Wakely wrote: > On Sat, 1 Oct 2022 at 11:43, François Dumont wrote: >> On 01/10/22 12:06, Jonathan Wakely wrote: >>> On Sat, 1 Oct 2022 at 08:20, François Dumont via Libstdc++ >>> wrote: >>>> I had forgotten to re-run tests after I removed the #define >>>> _GLIBCXX_USE_CXX11_ABI 0. >>>> >>>> The comment was misleading, it could also impact output of std::list. >>>> >>>> I am also restoring the correct std::string alias for >>>> std::__cxx11::basic_string, even if with my workaround it doesn't really >>>> matter as the one for std::basic_string will be used. >>>> >>>> I also restored the printer for std::__cxx11::string typedef. Is it >>>> intentional to keep this ? >>> Yes, I kept that intentionally. There can be programs where some >>> objects still use that typedef, if those objects were compiled with >>> GCC 8.x or older. >>> >>>> libstdc++: Fix gdb pretty printers when dealing with std::string >>>> >>>> Since revision 33b43b0d8cd2de722d177ef823930500948a7487 std::string >>>> and other >>>> similar typedef are ambiguous from a gdb point of view because it >>>> matches both >>>> std::basic_string and std::__cxx11::basic_string >>>> symbols. For those >>>> typedef add a workaround to accept the substitution as long as the >>>> same regardless >>>> of __cxx11 namespace. >>> Thanks for figuring out what was going wrong here, and how to fix it. >>> >>> >>>> Also avoid to register printers for types in std::__cxx11::__8:: >>>> namespace, there is >>>> no such symbols. >>>> >>>> libstdc++-v3/ChangeLog: >>>> >>>> * libstdc++-v3/python/libstdcxx/v6/printers.py >>>> (Printer.add_version): Do not add >>>> version namespace for __cxx11 symbols. >>>> (add_one_template_type_printer): Likewise. >>>> (add_one_type_printer): Likewise. >>>> (FilteringTypePrinter._recognizer.recognize): Add a >>>> workaround for std::string & al >>>> ambiguous typedef matching both std:: and std::__cxx11:: >>>> symbols. >>>> (register_type_printers): Refine type registration to limit >>>> false positive in >>>> FilteringTypePrinter._recognize.recognize requiring to look >>>> for the type in gdb. >>> I don't really like this part of the change though: >> I'll check what you are proposing but I don't think it is necessary to >> fix the problem. > Most of my patch is an alternative way to make the filter match on > "basic_string the ambiguous string typedefs, by using match.split('::')[-1] to get > the class template name without namespaces and then compare that to > "basic_string". > >> I did this on my path to find out what was going wrong. Once I >> understood it I consider that it was just a good change to keep. If you >> think otherwise I can revert this part. > Yeah it looks like it's just an optimization to fail faster without > having to do gdb.lookup_type. > > Please revert the changes to register_type_printers then, and we can > consider that part later if we want to revisit it. I'm not opposed to > making that fail-fast optimization, as long as we keep the property > that FilteringTypePrinter.match is the class template name. Maybe it > should be renamed to something other than "match" to make that clear. Or change the doc ? For my info, why is it so important to comply to the current doc ? Is it extracted from some gdb doc ? Now that the problem is fixed it is less important but I never managed to find any doc about this gdb feature that we are relying on. > > The rest of the patch is OK for trunk, thanks. > >> I also noted that gdb consider the filters as a filo list, not fifo. And >> I think that the 1st filters registered are the most extensively used. I >> can propose to invert all the registration if you think it worth it. > I've not noticed any performance problems with the printers, but I > have wondered how many printers is too many. That's an interesting > observation about the order they're checked. I'll talk to some of the > GDB devs to find out if they think it's something we should worry > about. Let's not try make premature optimizations until we know if it > matters. Yes, but with the 1st registration and so the last evaluation being 'std::string' it sounds more like a premature lowering ;-)