public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Matthias Kretz <m.kretz@gsi.de>
To: <gcc-patches@gcc.gnu.org>, Jason Merrill <jason@redhat.com>
Subject: Re: [RFC] c++: Print function template parms when relevant (was: [PATCH v4] c++: Add gnu::diagnose_as attribute)
Date: Fri, 19 Nov 2021 10:53:27 +0100	[thread overview]
Message-ID: <2941803.SbB2uZ23bb@excalibur> (raw)
In-Reply-To: <77278857-45f0-f76e-bbab-52598052b9eb@redhat.com>

On Thursday, 18 November 2021 20:24:36 CET Jason Merrill wrote:
> On 11/17/21 17:51, Matthias Kretz wrote:
> > 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)

Like this?

@@ -2890,10 +2890,11 @@ get_mapped_args (tree map)
       tree level = make_tree_vec (list.length ());
       for (unsigned j = 0; j < list.length(); ++j)
         TREE_VEC_ELT (level, j) = list[j];
+      /* None of the args at any level are defaulted.  */
+      SET_NON_DEFAULT_TEMPLATE_ARGS_COUNT (level, list.length());
       SET_TMPL_ARGS_LEVEL (args, i + 1, level);
       list.release ();
     }
-  SET_NON_DEFAULT_TEMPLATE_ARGS_COUNT (args, 0);

   return args;
 }

> >>> __FUNCTION__ was 'fun<int>' 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".

So you think f1<int> in testsuite/g++.old-deja/g++.ext/pretty3.C needs to test 
for

  if (strcmp (function, "f1"))
    bad = true;
  if (strcmp (pretty, "void f1(T) [with T = int]"))
    bad = true;

Or should the latter be "void f1(int)"?

> >>> 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 <class T> void f();
> > template <> void f<int>();
> > 
> > With -fpretty-templates shouldn't it print as 'void f<T>() [with T =
> > float]' and 'void f<int>()'? 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.

Yes, the call signature is the same. But what it calls is different. There's 
no obvious answer for my stated goal "print template parms wherever they
would appear in the source code as well", since it depends on whether the user 
is interested in recognizing the exact function body that was called.

My motivation for printing a function template specialization differently is:

1. It's a different function definition that's being called. The user (caller) 
might be surprised to realize this is the case as he forgot about the 
specialization and was expecting his change to the general template to make a 
difference.

2. There's no T in

template <> void f<int>() {
  // no T here, but of course I can define one:
  using T = int;
}

so presenting the function that was called as 'void f<T>() [with T = int]' is 
not exactly correct. In this case it wasn't even the case that T was deduced 
to be 'int', which we could argue to be useful information that might get 
lost.

For

template <class T> void f(T);
template <> void f(int);

the whole story is "'void f(int)' was called for 'template <class T> void f(T) 
[with T = int]'". What the user wants to see depends on what is more important 
to fix the bug: that T was deduced to be int, or that the specialization of f 
was called instead of the general template. I'd still go with 'void f(int)', 
though I'd be happier if I had some indication that a template was involved.

> >> Ah, you're trying to omit defaulted parms from the <list>?  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<int>::clear() on const
> > object:
> > 
> > error: passing 'const std::vector<int>' 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<int>]'
> > 
> > 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 <class T, class U = int> struct A {
> > 
> >    [[deprecated]] void f(U);
> > 
> > };
> > 
> > A<float> a; a.f(1);
> > 
> > With my patch it prints 'void A<T>::f(U) [with T = float]', with your
> > suggestion 'void A<T, U>::f(U) [with T = float]'. Both are missing
> > important information in the substitution list, IMHO. Would 'void A<T, U
> > = int>::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).

Let me check whether I have the right idea:

I could extend find_typenames (which walks the complete) tree to look for 
TEMPLATE_TYPE_PARM (and the 3 others I don't recall right now). But since that 
walks the *complete* tree, it'll simply find all parms with no indication 
whether they appear in the signature. Ideas:

1. Count occurrences: with 2 occurrences, one of them must be a use in the 
signature.

2. Walk only over TYPE_ARG_TYPES (TREE_TYPE (DECL_TEMPLATE_RESULT (fn))) to 
collect TEMPLATE_TYPE_PARMs.


-- 
──────────────────────────────────────────────────────────────────────────
 Dr. Matthias Kretz                           https://mattkretz.github.io
 GSI Helmholtz Centre for Heavy Ion Research               https://gsi.de
 stdₓ::simd
──────────────────────────────────────────────────────────────────────────




  reply	other threads:[~2021-11-19  9:53 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-15 12:21 [PATCH v3] c++: Add gnu::diagnose_as attribute Matthias Kretz
2021-07-23  8:58 ` [PATCH v4] " Matthias Kretz
2021-08-17 18:31   ` Jason Merrill
2021-11-08 16:40     ` [RFC] c++: Print function template parms when relevant (was: [PATCH v4] c++: Add gnu::diagnose_as attribute) Matthias Kretz
2021-11-08 20:00       ` Matthias Kretz
2021-11-16 20:25         ` Jason Merrill
2021-11-16 20:42           ` Matthias Kretz
2021-11-16 20:49             ` Jason Merrill
2021-11-16 20:51               ` Matthias Kretz
2021-11-17  6:09       ` Jason Merrill
2021-11-17  9:04         ` Matthias Kretz
2021-11-17 18:25           ` Jason Merrill
2021-11-17 22:51             ` Matthias Kretz
2021-11-18 19:24               ` Jason Merrill
2021-11-19  9:53                 ` Matthias Kretz [this message]
2021-11-19 12:02                   ` Matthias Kretz
2021-11-19 22:26                   ` Jason Merrill
2021-11-19 23:11                     ` Matthias Kretz
2021-11-26 15:23                     ` [PATCH 0/2] " Matthias Kretz
2021-11-26 15:24                       ` [PATCH 1/2] c++: Print function template parms when relevant Matthias Kretz
2021-12-02  8:35                         ` [PATCH v2 " Matthias Kretz
2021-11-26 15:24                       ` [PATCH 2/2] c++: Print function template parms when relevant [part 2] Matthias Kretz
2021-09-08  2:21   ` [PATCH v4] c++: Add gnu::diagnose_as attribute Jason Merrill
2021-11-15  0:35     ` [PATCH v5] " Matthias Kretz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2941803.SbB2uZ23bb@excalibur \
    --to=m.kretz@gsi.de \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jason@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).