From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ej1-x62d.google.com (mail-ej1-x62d.google.com [IPv6:2a00:1450:4864:20::62d]) by sourceware.org (Postfix) with ESMTPS id 839AF3954475; Wed, 16 Nov 2022 06:04:53 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 839AF3954475 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-x62d.google.com with SMTP id kt23so41586635ejc.7; Tue, 15 Nov 2022 22:04:53 -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=cVW7RH3J2nQKkrnKw/L1UZVJE6fxJmXTbyz1mJc0nwc=; b=LrM6QnYYtF7z6y8Kk7QHZNsGldJK5AZpVh5B1oYLHX71DWMpDdvy/4+cWRCCLVJRA2 wExxoEmCBQu95jHISE6wDy1rcUU8YwQDn/K+yVjRprKRs4OCqFlqVBoewarxmBC6aJjM DwZ8Q9JHHCCRvpTQjJIWlo9pyNXVOaRs4AHl8D6xGckYIJkcKeBo8wV/9qXEaJrqvpG2 wUJYKQ/3D5zJw49hi6lgYM3lbGzMbgL7btvx2AXh66cKT3tG/MZU6yzY3FYm4i5fdE72 uLKrSyVe/iJXgT9IbmTJAMu94CaTm9Z+Yoc4KqfbvmSSdRGfd0QV1nUdNJsiTALe/Pjt Y8RQ== 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=cVW7RH3J2nQKkrnKw/L1UZVJE6fxJmXTbyz1mJc0nwc=; b=y6REUSHDIqdoeIMP8gjN/69B0KHKFyMgdZwDIaWoZSuTKD4UJg5XKz186qGnvj4mA6 4NNiKE/JnTljps9Q7N/KgcPmE+d3KZRWF8LX0rb6uDx1h/mdg+C9894Rcs04q6KGKNVl 9N+IsI8ylfqVuMoQJVvHEKjoWla+zv9fRQV67OtGk+rjX81zG/PRfkmq4Fo9xDIA8Ght r1rMebAe8meHvz5E3lM0rjA8iKujBsoCVw8hGAKa4d47iMbN7i5iNXP/1vm6Ug84ZCM3 T0+kuLssvCoAj1PDzkHLQWWZslIg7wsHSzEsrOEtIqP6oVxka8dZAnIdRREdd0FMo36+ wzmg== X-Gm-Message-State: ANoB5pm9ueU9djZcs+HoaG3V4dcSIlsWmRaV7qta8ALqJFqXBBsx+lQS b/KBCcjHA+q82LQTTlSfJrc= X-Google-Smtp-Source: AA0mqf75VceLe/tWwthAn+dXnlR3ADr3EzqKgY9XnGxXsFYHu0C99VL012NqHa6UAA3KeOoXAJaLiQ== X-Received: by 2002:a17:906:b2ca:b0:7ad:92c5:637a with SMTP id cf10-20020a170906b2ca00b007ad92c5637amr16772454ejb.87.1668578690299; Tue, 15 Nov 2022 22:04:50 -0800 (PST) Received: from [10.13.0.180] ([109.190.253.11]) by smtp.googlemail.com with ESMTPSA id bc22-20020a056402205600b0045bccd8ab83sm7114439edb.1.2022.11.15.22.04.47 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 15 Nov 2022 22:04:49 -0800 (PST) Message-ID: <6056a668-3b5a-04b0-a0cc-98ad0c6064b2@gmail.com> Date: Wed, 16 Nov 2022 07:04:45 +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> Content-Language: fr, en-US 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=-6.5 required=5.0 tests=BAYES_00,BODY_8BITS,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,RCVD_IN_SBL_CSS,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 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' :-) > >> >> -    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. So I removed it as useless and redundant with this function default value. > > > >> +        self.template = template >>         self.name = name >> +        self.targ1 = targ1 >>         self.enabled = True >> >>     class _recognizer(object): >>         "The recognizer class for FilteringTypePrinter." >> >> -        def __init__(self, match, name): >> -            self.match = match >> +        def __init__(self, template, name, targ1): >> +            self.template = template >>             self.name = name >> +            self.targ1 = targ1 >>             self.type_obj = None >> >>         def recognize(self, type_obj): >>             """ >> -            If type_obj starts with self.match and is the same type as >> +            If type_obj starts with self.template and is the same >> type as >>             self.name then return self.name, otherwise None. >>             """ >>             if type_obj.tag is None: >>                 return None >> >>             if self.type_obj is None: >> -                if not type_obj.tag.startswith(self.match): >> +                if self.targ1 is not None: >> +                    if not >> type_obj.tag.startswith('{}<{}'.format(self.template, self.targ1)): > > I wonder if we should make targ1 a gdb.Type object, not just a string. > That seems like it would be better. The add_on_type_printer function > could still accept a string, and then call gdb.lookup_type(targ1) to > get a gdb.Type. We can change that later though. > > OK for trunk with the comments fixed to say "specialization" again. > Now committed.