From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ed1-x52d.google.com (mail-ed1-x52d.google.com [IPv6:2a00:1450:4864:20::52d]) by sourceware.org (Postfix) with ESMTPS id 6B5B239730DB; Thu, 17 Nov 2022 05:28:46 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 6B5B239730DB 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-ed1-x52d.google.com with SMTP id x2so1015610edd.2; Wed, 16 Nov 2022 21:28:46 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=k2X3VUVgvznB/XiOEdb+upcmPbhH3nXhr9wRKla9PLg=; b=Z/BgVHqkrt/dlrO0XUErfhQPfAcpfBCNzV6R9KGYasRiv/MOJ5k5i+x3g57W2ktOCf CpEOWTPa3W1OXx/mtW24stKVVpPlyyO4x/p31TzZ/ML3w5V64etwasZuoT7dee/vPxA0 gGWjPaCGQOTx5AhyvBQ30yZ67z90IT2OydC1D8Jb2fADRcbM75NBuvFjumoVkR3AGhUo jO9fbRbcOOmmafoDlgKCc0vq+FjVbabvPvHIPZRctTWUg6XBTLgcvxWvttw8CQ9KS4f/ xMU28klFbDulJD13lLYsoQBi8ewNdpBof0G/WCDEqWjOAXKQxXM/ehQ8grJhPT06fKR5 O3RQ== 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:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=k2X3VUVgvznB/XiOEdb+upcmPbhH3nXhr9wRKla9PLg=; b=tZCy6qdeEATZN3XCQJzm1Goge0ZXByfpaJcfv8VKa+BvOOv/BPQX7rM7EGInugJods eEzkZOQmWgLkQy5UmcPxo04RRRpXcCOpziJCMgCBTcuQj72gwhgX5y4WefVxX/Pc7LTH HOMtn/csRonK2pPD/TjrQf4NMpeZH+LQoJ/BbNVNH096lsyPTM+1/oqWPA712JJHuigX Z0qvDFUBr1+8Js6kSKvmp0A4WmP7aj9bFTr0uJQ2DhBbtms18jmMROhc/47h2ILwaUEY SIDaadCmyMy7fry3Go6phMVx9EOjbthxlwI9H8xQ+xPc8mceKoTow94+Hp1ITR3zvTNi TOZQ== X-Gm-Message-State: ANoB5pn+YsxZP8E4jaOI0mYeSHWeI+2lfHhSGezHtW5fEu47FNsef27K nMeppxdZDmT6bzzZxx2SgCk= X-Google-Smtp-Source: AA0mqf5LtfZ1rjJkCZblYRpx7w9Wd/hkZEt1UnwddWgUJ0ndw+FPu6AL2pnbdR8LzjIzlBro77FPEg== X-Received: by 2002:a05:6402:2992:b0:460:12ef:cc45 with SMTP id eq18-20020a056402299200b0046012efcc45mr771612edb.249.1668662924560; Wed, 16 Nov 2022 21:28:44 -0800 (PST) Received: from [10.2.0.87] ([109.190.253.14]) by smtp.googlemail.com with ESMTPSA id o19-20020a170906769300b00722e50dab2csm7652187ejm.109.2022.11.16.21.28.42 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 16 Nov 2022 21:28:43 -0800 (PST) Message-ID: <854a1216-130c-a091-65c2-fb041f21629e@gmail.com> Date: Thu, 17 Nov 2022 06:28:41 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.4.2 Subject: Re: [PATCH] Fix gdb FilteringTypePrinter (again) To: Jonathan Wakely Cc: "libstdc++@gcc.gnu.org" , gcc-patches References: <4d1dc3d4-e945-d283-964a-4dab3b3cb33e@gmail.com> <6056a668-3b5a-04b0-a0cc-98ad0c6064b2@gmail.com> Content-Language: fr 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=-9.9 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,GIT_PATCH_0,NICE_REPLY_A,RCVD_IN_BARRACUDACENTRAL,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham 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 16/11/22 12:54, Jonathan Wakely wrote: > On Wed, 16 Nov 2022 at 11:35, Jonathan Wakely wrote: >> On Wed, 16 Nov 2022 at 06:04, François Dumont wrote: >>> On 15/11/22 17:17, Jonathan Wakely wrote: >>>> On 06/10/22 19:38 +0200, François Dumont wrote: >>>>> Hi >>>>> >>>>> Looks like the previous patch was not enough. When using it in the >>>>> context of a build without dual abi and versioned namespace I started >>>>> having failures again. I guess I hadn't rebuild everything properly. >>>>> >>>>> This time I think the problem was in those lines: >>>>> >>>>> if self.type_obj == type_obj: >>>>> return strip_inline_namespaces(self.name) >>>>> >>>>> I've added a call to gdb.types.get_basic_type so that we do not compare >>>>> a type with its typedef. >>>>> >>>>> Thanks for the pointer to the doc ! >>>>> >>>>> Doing so I eventually use your code Jonathan to make FilteringTypeFilter >>>>> more specific to a given instantiation. >>>>> >>>>> libstdc++: Fix gdb FilteringTypePrinter >>>>> >>>>> Once we found a matching FilteringTypePrinter instance we look for >>>>> the associated >>>>> typedef and check that the returned Python Type is equal to the >>>>> Type to recognize. >>>>> But gdb Python Type includes properties to distinguish a typedef >>>>> from the actual >>>>> type. So use gdb.types.get_basic_type to check if we are indeed on >>>>> the same type. >>>>> >>>>> Additionnaly enhance FilteringTypePrinter matching mecanism by >>>>> introducing targ1 that, >>>>> if not None, will be used as the 1st template parameter. >>>>> >>>>> libstdc++-v3/ChangeLog: >>>>> >>>>> * python/libstdcxx/v6/printers.py (FilteringTypePrinter): >>>>> Rename 'match' field >>>>> 'template'. Add self.targ1 to specify the first template >>>>> parameter of the instantiation >>>>> to match. >>>>> (add_one_type_printer): Add targ1 optional parameter, >>>>> default to None. >>>>> Use gdb.types.get_basic_type to compare the type to >>>>> recognize and the type >>>>> returned from the typedef lookup. >>>>> (register_type_printers): Adapt calls to >>>>> add_one_type_printers. >>>>> >>>>> Tested under Linux x86_64 normal, version namespace with or without dual >>>>> abi. >>>>> >>>>> François >>>>> >>>>> diff --git a/libstdc++-v3/python/libstdcxx/v6/printers.py >>>>> b/libstdc++-v3/python/libstdcxx/v6/printers.py >>>>> index 0fa7805183e..52339b247d8 100644 >>>>> --- a/libstdc++-v3/python/libstdcxx/v6/printers.py >>>>> +++ b/libstdc++-v3/python/libstdcxx/v6/printers.py >>>>> @@ -2040,62 +2040,72 @@ def add_one_template_type_printer(obj, name, >>>>> defargs): >>>>> >>>>> class FilteringTypePrinter(object): >>>>> r""" >>>>> - A type printer that uses typedef names for common template >>>>> specializations. >>>>> + A type printer that uses typedef names for common template >>>>> instantiations. >>>>> >>>>> Args: >>>>> - match (str): The class template to recognize. >>>>> + template (str): The class template to recognize. >>>>> name (str): The typedef-name that will be used instead. >>>>> + targ1 (str): The first template argument. >>>>> + If arg1 is provided (not None), only template >>>>> instantiations with this type >>>>> + as the first template argument, e.g. if >>>>> template='basic_string>>>> >>>>> - Checks if a specialization of the class template 'match' is the >>>>> same type >>>>> + Checks if an instantiation of the class template 'template' is >>>>> the same type >>>>> as the typedef 'name', and prints it as 'name' instead. >>>>> >>>>> - e.g. if an instantiation of std::basic_istream is the same >>>>> type as >>>>> + e.g. for template='basic_istream', name='istream', if any >>>>> instantiation of >>>>> + std::basic_istream is the same type as std::istream then >>>>> print it as >>>>> + std::istream. >>>>> + >>>>> + e.g. for template='basic_istream', name='istream', targ1='char', >>>>> if any >>>>> + instantiation of std::basic_istream is the same type as >>>>> std::istream then print it as std::istream. >>>>> """ >>>> These are template specializations, not instantiations. Please undo >>>> the changes to the comments, because the comments are 100% correct >>>> now, and would become wrong with this patch. >>>> >>>> template struct foo { }; >>>> using F = foo; // #1 >>>> template struct foo { }; // #2 >>>> template<> struct foo { }; // #3 >>>> >>>> #1 is a *specialization* of the class template foo. It is >>>> *instantiated* when you construct one or depend on its size, or its >>>> members. >>>> #2 is a *partial specialization* and #3 is an explicit specialization. >>>> But #1 is a speclialization, not an instantiation. >>>> >>>> Instantiation is a process that happens during compilation. A >>>> specialization is a type (or function, or variable) generated from a >>>> template by substituting arguments for the template parameters. The >>>> python type printer matches specializations. >>> Lesson learned, thanks. >>> >>> Maybe comment on line 169 is wrong then. I think there is a clue in the >>> function name 'is_specialization_of' :-) >> Good point! Thanks, I'll fix it. >> >>>>> - def __init__(self, match, name): >>>>> - self.match = match >>>>> + def __init__(self, template, name, targ1): >>>> Is there a reason to require targ1 here, instead of making it >>>> optional, by using =None as the default? >>> In your original, and I know untested, proposal it was not working. >>> >>> The function add_one_type_printer was missing to pass its targ1 >>> parameter to the FilteringTypePrinter ctor but thanks to the default >>> value it was un-noticed by the interpreter. >> My untested patch had this, which adds it, doesn't it? >> >> -def add_one_type_printer(obj, match, name): >> - printer = FilteringTypePrinter('std::' + match, 'std::' + name) >> +def add_one_type_printer(obj, match, name, targ1 = None): >> + printer = FilteringTypePrinter('std::' + match, 'std::' + name, targ1) >> gdb.types.register_type_printer(obj, printer) >> if _versioned_namespace: >> ns = 'std::' + _versioned_namespace >> - printer = FilteringTypePrinter(ns + match, ns + name) >> + printer = FilteringTypePrinter(ns + match, ns + name, targ1) >> gdb.types.register_type_printer(obj, printer) Indeed, I guess I mess it up myself when doing all my tests. >> >> >> I think FilteringTypePrinter should be usable without specifying None >> explicitly as the argument. Even if we don't actually use it that way >> today, it seems like a better API. If the argument is optional, then >> the idiomatic way to express that is to give it a default, not require >> None to be passed. >> >> I'll add that default argument, but first I need to figure out why I'm >> seeing new failures for libfundts.cc with -D_GLIBCXX_USE_CXX11_ABI=0. >> Your patch has introduced this new error: >> >> $12 = Python Exception : No type named >> std::experimental::fundamentals_v1::any::_Manager_internal> std::char_traits, std::allocator >>. >> got: $12 = Python Exception : No type named >> std::experimental::fundamentals_v1::any::_Manager_internal> std::char_traits, std::allocator >>. >> FAIL: libstdc++-prettyprinters/libfundts.cc print as > The problem happens here in StdExpAnyPrinter: > > mgrname = m.group(1) > # FIXME need to expand 'std::string' so that gdb.lookup_type works > if 'std::string' in mgrname: > mgrname = re.sub("std::string(?!\w)", > str(gdb.lookup_type('std::string').strip_typedefs()), m.group(1)) > > mgrtype = gdb.lookup_type(mgrname) > > After your patch, gdb.lookup_type('std::string').strip_typedefs() is > returning std::__cxx11::basic_string which is not the > correct type for this specialization of the any manager function. It > contains a std::basic_string. > I had noticed this usage of std::string but surprisingly had never managed to have a test failure because of that so I left it untouch. Note that I had not re-tested the patch after your approval, maybe something changed in the month gap. Thanks for having sorted that out.