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 [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id B3EC23858014 for ; Thu, 18 Nov 2021 19:24:41 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org B3EC23858014 Received: from mail-qv1-f70.google.com (mail-qv1-f70.google.com [209.85.219.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-99-nME7Zr8sMDmp2t4Gyfu-Nw-1; Thu, 18 Nov 2021 14:24:39 -0500 X-MC-Unique: nME7Zr8sMDmp2t4Gyfu-Nw-1 Received: by mail-qv1-f70.google.com with SMTP id j9-20020a05621419c900b003b815c01a54so6712132qvc.10 for ; Thu, 18 Nov 2021 11:24:39 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:subject :content-language:to:references:from:in-reply-to :content-transfer-encoding; bh=WmUAspAEuWs+p4HH9DSToxUTO8JS6Z29K230wA0LEE4=; b=pQTAqnxbSBQF5nZ33MGLL7+KmmWqG+18TEhqMy2CBmDvWHLMQ4RmzKjQMwoLV1ovl9 SfQ/VpoaS9Ixl3joM4iQywP6SS4KtX0v7GLTfyazrz2jTMjerWNxJUP4g8BFgPXg5tBk Kns5AoDrdmJ7J9uXlWNKYr8puPmIzzyEyMDpZc+d6BnGG8C5g6P2qvggEnETvLrMLE/r m7I5cf71NQ/W+H00mgxvSXNx7b+CFZDb78mhr7ZcHTHiUuUHs/Cu1J1iO3pqGdEOxbKq /RaAW9/m8Rb/B2+zfvcoFE+MqlEZOmo8UVxkVR4bEfmagA/W8w66ZnDXLyqZdgn5su5B yPSw== X-Gm-Message-State: AOAM53399id0xTeyd32NT4JzOS9/koxNTkZ+FTEBpl+RWdHZ31OMDhbS WeCPIwubHMgro4TdBAq/X1eIQl683spj6Y0vV9WgATXc68MQqWRShJL4isAJ1ntJB1ENmln9TLO zzvHuYQTMo3WO995cXQ== X-Received: by 2002:a05:6214:2623:: with SMTP id gv3mr66242586qvb.63.1637263478539; Thu, 18 Nov 2021 11:24:38 -0800 (PST) X-Google-Smtp-Source: ABdhPJyhEWiUcY7akkAwC/0V/AYyRvj48JyvTVJQboEStSEjemcUqyozqqyYJ01PVBik5wipMa9l1w== X-Received: by 2002:a05:6214:2623:: with SMTP id gv3mr66242546qvb.63.1637263478103; Thu, 18 Nov 2021 11:24:38 -0800 (PST) Received: from [192.168.1.149] (130-44-159-43.s15913.c3-0.arl-cbr1.sbo-arl.ma.cable.rcncustomer.com. [130.44.159.43]) by smtp.gmail.com with ESMTPSA id u18sm420906qki.69.2021.11.18.11.24.37 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 18 Nov 2021 11:24:37 -0800 (PST) Message-ID: <77278857-45f0-f76e-bbab-52598052b9eb@redhat.com> Date: Thu, 18 Nov 2021 14:24:36 -0500 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.3.1 Subject: Re: [RFC] c++: Print function template parms when relevant (was: [PATCH v4] c++: Add gnu::diagnose_as attribute) To: Matthias Kretz , gcc-patches@gcc.gnu.org References: <4361366.VLH7GnMWUR@minbar> <1716806.eMUbO1HJkO@excalibur> <17241ec0-9e13-e804-ac34-2ca6cdd0dc4b@redhat.com> <1845985.Z2AExFMA8N@excalibur> From: Jason Merrill In-Reply-To: <1845985.Z2AExFMA8N@excalibur> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-7.7 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, NICE_REPLY_A, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, TXREP, WEIRD_PORT 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: , X-List-Received-Date: Thu, 18 Nov 2021 19:24:43 -0000 On 11/17/21 17:51, Matthias Kretz wrote: > On Wednesday, 17 November 2021 19:25:46 CET Jason Merrill wrote: >> On 11/17/21 04:04, Matthias Kretz wrote: >>> On Wednesday, 17 November 2021 07:09:18 CET Jason Merrill wrote: >>>>> - if (CHECKING_P) >>>>> - SET_NON_DEFAULT_TEMPLATE_ARGS_COUNT (a, TREE_VEC_LENGTH (a)); >>>>> + SET_NON_DEFAULT_TEMPLATE_ARGS_COUNT (a, nondefault); >>>> >>>> should have been >>>> >>>> if (CHECKING_P || nondefault != TREE_VEC_LENGTH (a)) >>>> >>>> SET_NON_DEFAULT_TEMPLATE_ARGS_COUNT (a, nondefault); >>> >>> TBH, I don't understand the purpose of CHECKING_P here, or rather it makes >>> me nervous because AFAIU I'm only testing with CHECKING_P enabled. Why >>> make behavior dependent on CHECKING_P? I expected CHECKING_P to basically >>> only add more assertions. >> >> The idea when NON_DEFAULT_TEMPLATE_ARGS_COUNT was added years back was >> to leave the TREE_CHAIN null when !CHECKING_P and treat that as >> equivalent to TREE_VEC_LENGTH (args). But perhaps you're right that >> it's not a savings worth the complexity. > > Thanks, now I understand. > >>>>> (copy_template_args): Jason? >>>> >>>> Only copy the non-default template args count on TREE_VECs that should >>>> have it. >>> >>> Why not simply set the count on all args? Is it a performance concern? The >>> INTEGER_CST the TREE_CHAIN has to point to exists anyway, so it's not >>> wasting any memory, right? >> >> In this case the TREE_VEC we're excluding is the one wrapping multiple >> levels of template args; it doesn't contain args directly, so setting >> NON_DEFAULT_ARGS_COUNT on it doesn't make sense. > > Right, I had already added a `gcc_assert (!TMPL_ARGS_HAVE_MULTIPLE_LEVELS > (args))` to my new set_non_default_template_args_count function and found cp/ > constraint.cc:2896 (get_mapped_args), which calls > SET_NON_DEFAULT_TEMPLATE_ARGS_COUNT on the outer TREE_VEC. Was this supposed > to apply to all inner TREE_VECs? Or is deleting the line the correct fix? That should apply to the inner TREE_VECs (and probably use list.length) >>>>> + /* Pretty print only template instantiations. Don't pretty print >>>>> explicit >>>>> + specializations like 'template <> void fun (int)'. >>>> >>>> This seems like a significant change of behavior unrelated to printing >>>> default template arguments. What's the rationale for handling >>>> specializations differently from instantiations? >>> >>> Right, this is about "The general idea of this change is to print template >>> parms wherever they would appear in the source code as well". >>> >>> Initially, the change to print function template arguments/parameters only >>> if the args were explicitly specified lead to printing 'void fun (T) >>> [with T = ...]' or 'template <> void fun (int)'. Both are not telling the >>> full story, even if the former is how the function would be called. >> >> and the latter is how I expect the specialization to be declared, not >> with the deducible template argument made explicit. > > You're right. From my tests: > > template > [[deprecated]] void f4(a); > > template <> > [[deprecated]] void f4(int); > > template <> > [[deprecated]] void f4(float); > > f4(1.); // { dg-warning "'void f4\\(a\\) .with a = double.'" } > f4(1); // { dg-warning "'void f4\\(int\\)'" } > f4(1.f); // { dg-warning "'void f4\\(float\\)'" } > > So how it's printed depends on how the specialization is declared. It just > falls out that way and it didn't seem awfully wrong... ;) > >>> But if the reader >>> should quickly recognize what code is getting called, it is helpful to see >>> right away that a function template specialization is called. (It might >>> also reveal an implementation detail of a library, so it's not 100% >>> obvious how to choose here.) Also, saying 'T = int' is kind of wrong. >>> Yes, 'int' was deduced. But there's no T in fun: >>> >>> template void fun (T); >>> template <> void fun (int); >> >> There's a T in the template, and as you said above, that's how it's >> called (and mangled). >> >>> __FUNCTION__ was 'fun' all the time, but __PRETTY_FUNCTION__ was >>> 'void >>> fun(T) [with T = int]'. >> >> Isn't that true for instantiations, as well? > > No, instantiations don't have template args/parms in __FUNCTION__. Hmm, that inconsistency seems like a bug, though I'm not sure whether it should have the template args or not; I lean toward not. The standard says that the value of __func__ is implementation-defined, the GCC manual says it's "the unadorned name of the function". >>> It's more consistent that __PRETTY_FUNCTION__ contains __FUNCTION__, IMHO >> >> I suppose, but I don't see that as a strong enough motivation to mix >> this up. > > What about > > template void f(); > template <> void f(); > > With -fpretty-templates shouldn't it print as 'void f() [with T = float]' > and 'void f()'? Yes, it's probably too subtle for most users to notice > the difference. But I find it's more consistent this way. I disagree; the function signature is the same whether a particular function is an explicit specialization or an instantiation. >>> so it would have to be at least 'void fun(T) [with T >>> = int]'. But that's strange: How it uses T and int for the same type. So I >>> settled on 'void fun(int)'. >>> >>>> I also don't understand the purpose of TFF_AS_PRIMARY. >>> >>> dump_function_decl generalizes the TEMPLATE_DECL (if flag_pretty_templates >>> is true) and, before this change, passes the generalized TEMPLATE_DECL to >>> dump_type (... DECL_CONTEXT (t) ...) and dump_function_name (... t ...). >>> That's why the whole template is printed as primary template (i.e. with >>> template parms instead of template args, as is needed for >>> flag_pretty_templates). But this drops the count of non-default template >>> args. >> Ah, you're trying to omit defaulted parms from the ? I'm not sure >> that's necessary, leaving them out of the [with ...] list should be >> sufficient. > > I was thinking about all the std::allocator defaults in the standard library. > I don't want to see them. E.g. vector::clear() on const object: > > error: passing 'const std::vector' as 'this' argument discards qualifiers > [...]/stl_vector.h:1498:7: note: in call to 'void std::vector<_Tp, > _Alloc>::clear() [with _Tp = int; _Alloc = std::allocator]' > > With my patch the last line becomes > [...]/stl_vector.h:1498:7: note: in call to 'void std::vector<_Tp>::clear() > [with _Tp = int]' > > > Another case I didn't consider before: > > template struct A { > [[deprecated]] void f(U); > }; > > A a; a.f(1); > > With my patch it prints 'void A::f(U) [with T = float]', with your > suggestion 'void A::f(U) [with T = float]'. Both are missing important > information in the substitution list, IMHO. Would 'void A::f(U) > [with T = float]' be an improvement? Or should find_typenames (in cp/error.c) > find defaulted template parms and add them to its list? IIUC find_typenames > would find all template parms and couldn't know whether they're defaulted. That sounds good: omit defaulted parms only if they don't appear in the signature (other than as another default template argument). >>>>> + 4. either >>>>> + - flags requests to show no function arguments, in which case >>>>> deduced + types could be hidden, or >>>>> + - at least one function template argument was given explicitly, >>>>> or >>>>> + - we're printing a DWARF name, >>>> >>>> Though, why do we want >>>> different behavior here when printing a DWARF name? >>> >>> Sorry, I should have asked ... I also added the same issue as an open >>> question on the diagnose_as patch. When I ran the whole testsuite I had >>> failures in the DWARF tests. This change resolved them. I don't know >>> enough about how those strings are used and whether they may change >>> between GCC versions. Anyway, the DWARF strings in that test had only the >>> function name and template argument list (i.e. no function arguments). >> >> If they only have arguments, isn't TFF_NO_FUNCTION_ARGUMENTS set? > > No, it isn't set. I could try to set it for DWARF names. It sounds like the > right solution here. I agree. Jason