From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.145.221.124]) by sourceware.org (Postfix) with ESMTPS id 0CB2A3898C70 for ; Tue, 15 Nov 2022 16:17:40 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 0CB2A3898C70 Authentication-Results: sourceware.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=fail smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1668529059; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=8nDD5IOFLfRgiCwIbKXDTO6C2/78mNSGkVKlGHsE/Dg=; b=WfC9x7agOv6Mat0GkXuqv1t4IUbV/QPNVrVPx/YylkfZLwvrYr7gfqhrTgaS3vpugbfCnS +gBh8fFWkb73pNicAVCLUNcNeMGbnMo3c9s+9uudiiDfISb0Wjldr48KexuWGOZpEMhmCS qYOrWBI6vTMIWPm5k/4fpnRgbYuJhAc= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-2-TY-RXXrfN6eUu2v2EFUFEg-1; Tue, 15 Nov 2022 11:17:36 -0500 X-MC-Unique: TY-RXXrfN6eUu2v2EFUFEg-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 3CF1B882824; Tue, 15 Nov 2022 16:17:36 +0000 (UTC) Received: from localhost (unknown [10.33.36.199]) by smtp.corp.redhat.com (Postfix) with ESMTP id F00F31121325; Tue, 15 Nov 2022 16:17:35 +0000 (UTC) Date: Tue, 15 Nov 2022 16:17:33 +0000 From: Jonathan Wakely To: =?iso-8859-1?Q?Fran=E7ois?= Dumont Cc: "libstdc++@gcc.gnu.org" , gcc-patches Subject: Re: [PATCH] Fix gdb FilteringTypePrinter (again) Message-ID: References: <4d1dc3d4-e945-d283-964a-4dab3b3cb33e@gmail.com> MIME-Version: 1.0 In-Reply-To: <4d1dc3d4-e945-d283-964a-4dab3b3cb33e@gmail.com> X-Clacks-Overhead: GNU Terry Pratchett X-Scanned-By: MIMEDefang 3.1 on 10.11.54.3 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=WINDOWS-1252; format=flowed Content-Disposition: inline Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-10.3 required=5.0 tests=BAYES_00,BODY_8BITS,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_NONE,TXREP autolearn=unavailable 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 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. > >- 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? >+ 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.