From: Patrick Palka <ppalka@redhat.com>
To: Jason Merrill <jason@redhat.com>
Cc: Patrick Palka <ppalka@redhat.com>, gcc-patches@gcc.gnu.org
Subject: Re: [PATCH] c++: Return only in-scope tparms in keep_template_parm [PR95310]
Date: Tue, 22 Sep 2020 14:28:36 -0400 (EDT) [thread overview]
Message-ID: <8f5f5514-e8b0-b910-411e-334eb81797df@idea> (raw)
In-Reply-To: <45118c59-948d-c373-4622-6ae2bede8707@redhat.com>
On Mon, 21 Sep 2020, Jason Merrill wrote:
> On 9/19/20 3:49 PM, Patrick Palka wrote:
> > In the testcase below, the dependent specializations iter_reference_t<F>
> > and iter_reference_t<Out> share the same tree due to specialization
> > caching. So when find_template_parameters walks through the
> > requires-expression (as part of normalization), it sees and includes the
> > out-of-scope template parameter F in the list of template parameters
> > it found within the requires-expression (along with Out and N).
> >
> > From a correctness perspective this is harmless since the parameter mapping
> > routines only care about the level and index of each parameter, so F is
> > no different from Out in this sense. (And it's also harmless that two
> > parameters in the parameter mapping have the same level and index.)
> >
> > But having both Out and F in the parameter mapping is extra work for
> > hash_atomic_constrant, tsubst_parameter_mapping and get_mapped_args; and
> > it also means we print this irrelevant template parameter in the
> > testcase's diagnostics (via pp_cxx_parameter_mapping):
> >
> > in requirements with ‘Out o’ [with N = (const int&)&a; F = const int*;
> > Out = const int*]
> >
> > This patch makes keep_template_parm return only in-scope template
> > parameters by looking into ctx_parms for the corresponding in-scope one.
> >
> > (That we sometimes print irrelevant template parameters in diagnostics is
> > also the subject of PR99 and PR66968, so the above diagnostic issue
> > could likely be fixed in a more general way, but this targeted fix to
> > keep_template_parm is perhaps worthwhile on its own.)
> >
> > Bootstrapped and regtested on x86_64-pc-linux-gnu, and also tested on
> > cmcstl2 and range-v3. Does this look OK for trunk?
> >
> > gcc/cp/ChangeLog:
> >
> > PR c++/95310
> > * pt.c (keep_template_parm): Adjust the given template parameter
> > to the corresponding in-scope one from ctx_parms.
> >
> > gcc/testsuite/ChangeLog:
> >
> > PR c++/95310
> > * g++.dg/concepts/diagnostic15.C: New test.
> > * g++.dg/cpp2a/concepts-ttp2.C: New test.
> > ---
> > gcc/cp/pt.c | 19 +++++++++++++++++++
> > gcc/testsuite/g++.dg/concepts/diagnostic15.C | 16 ++++++++++++++++
> > 2 files changed, 35 insertions(+)
> > create mode 100644 gcc/testsuite/g++.dg/concepts/diagnostic15.C
> >
> > diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
> > index fe45de8d796..c2c70ff02b9 100644
> > --- a/gcc/cp/pt.c
> > +++ b/gcc/cp/pt.c
> > @@ -10550,6 +10550,25 @@ keep_template_parm (tree t, void* data)
> > BOUND_TEMPLATE_TEMPLATE_PARM itself. */
> > t = TREE_TYPE (TEMPLATE_TEMPLATE_PARM_TEMPLATE_DECL (t));
> > + /* This template parameter might be an argument to a cached dependent
> > + specalization that was formed earlier inside some other template, in
> > which
> > + case the parameter is not among the ones that are in-scope. Look in
> > + CTX_PARMS to find the corresponding in-scope template parameter and
> > + always return that instead. */
> > + tree cparms = ftpi->ctx_parms;
> > + while (TMPL_PARMS_DEPTH (cparms) > level)
> > + cparms = TREE_CHAIN (cparms);
> > + gcc_assert (TMPL_PARMS_DEPTH (cparms) == level);
> > + if (TREE_VEC_LENGTH (TREE_VALUE (cparms)))
> > + {
> > + t = TREE_VALUE (TREE_VEC_ELT (TREE_VALUE (cparms), index));
> > + /* As in template_parm_to_arg. */
> > + if (TREE_CODE (t) == TYPE_DECL || TREE_CODE (t) == TEMPLATE_DECL)
> > + t = TREE_TYPE (t);
> > + else
> > + t = DECL_INITIAL (t);
> > + }
>
> This seems like a useful separate function: given a parmlist and a single
> template parm (or index+level), return the corresponding parm from the
> parmlist. Basically the reverse of canonical_type_parameter.
Sounds good. Like this?
-- >8 --
gcc/cp/ChangeLog:
PR c++/95310
* pt.c (corresponding_template_parameter): Define.
(keep_template_parm): Use it to adjust the given template
parameter to the corresponding in-scope one from ctx_parms.
gcc/testsuite/ChangeLog:
PR c++/95310
* g++.dg/concepts/diagnostic15.C: New test.
* g++.dg/cpp2a/concepts-ttp2.C: New test.
---
gcc/cp/pt.c | 44 ++++++++++++++++++++
gcc/testsuite/g++.dg/concepts/diagnostic15.C | 16 +++++++
2 files changed, 60 insertions(+)
create mode 100644 gcc/testsuite/g++.dg/concepts/diagnostic15.C
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 44ca14afc4e..bec8396f9f4 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -10244,6 +10244,42 @@ lookup_and_finish_template_variable (tree templ, tree targs,
return convert_from_reference (templ);
}
+/* If the set of template parameters PARMS contains a template with
+ the given LEVEL and INDEX, then return this parameter. Otherwise
+ return NULL_TREE. */
+
+static tree
+corresponding_template_parameter (tree parms, int level, int index)
+{
+ while (TMPL_PARMS_DEPTH (parms) > level)
+ parms = TREE_CHAIN (parms);
+
+ if (TMPL_PARMS_DEPTH (parms) != level
+ || TREE_VEC_LENGTH (TREE_VALUE (parms)) <= index)
+ return NULL_TREE;
+
+ tree t = TREE_VALUE (TREE_VEC_ELT (TREE_VALUE (parms), index));
+ /* As in template_parm_to_arg. */
+ if (TREE_CODE (t) == TYPE_DECL || TREE_CODE (t) == TEMPLATE_DECL)
+ t = TREE_TYPE (t);
+ else
+ t = DECL_INITIAL (t);
+
+ gcc_assert (TEMPLATE_PARM_P (t));
+ return t;
+}
+
+/* Return the template parameter from PARMS that positionally corresponds
+ to the template parameter PARM, or else return NULL_TREE. */
+
+static tree
+corresponding_template_parameter (tree parms, tree parm)
+{
+ int level, index;
+ template_parm_level_and_index (parm, &level, &index);
+ return corresponding_template_parameter (parms, level, index);
+}
+
\f
struct pair_fn_data
{
@@ -10550,6 +10586,14 @@ keep_template_parm (tree t, void* data)
BOUND_TEMPLATE_TEMPLATE_PARM itself. */
t = TREE_TYPE (TEMPLATE_TEMPLATE_PARM_TEMPLATE_DECL (t));
+ /* This template parameter might be an argument to a cached dependent
+ specalization that was formed earlier inside some other template, in
+ which case the parameter is not among the ones that are in-scope.
+ Look in CTX_PARMS to find the corresponding in-scope template
+ parameter and use it instead. */
+ if (tree in_scope = corresponding_template_parameter (ftpi->ctx_parms, t))
+ t = in_scope;
+
/* Arguments like const T yield parameters like const T. This means that
a template-id like X<T, const T> would yield two distinct parameters:
T and const T. Adjust types to their unqualified versions. */
diff --git a/gcc/testsuite/g++.dg/concepts/diagnostic15.C b/gcc/testsuite/g++.dg/concepts/diagnostic15.C
new file mode 100644
index 00000000000..3acd9f67968
--- /dev/null
+++ b/gcc/testsuite/g++.dg/concepts/diagnostic15.C
@@ -0,0 +1,16 @@
+// PR c++/95310
+// { dg-do compile { target concepts } }
+
+template <class T>
+using iter_reference_t = decltype(*T{});
+
+template <typename F>
+struct result { using type = iter_reference_t<F>; };
+
+template <class Out, const int& N>
+concept indirectly_writable = requires(Out o) { // { dg-bogus "F =" }
+ iter_reference_t<Out>(*o) = N;
+};
+
+const int a = 0;
+static_assert(indirectly_writable<const int*, a>); // { dg-error "assert" }
--
2.28.0.497.g54e85e7af1
next prev parent reply other threads:[~2020-09-22 18:28 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-19 19:49 Patrick Palka
2020-09-21 21:42 ` Jason Merrill
2020-09-22 18:28 ` Patrick Palka [this message]
2020-09-22 18:41 ` Patrick Palka
2020-09-22 20:06 ` Jason Merrill
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=8f5f5514-e8b0-b910-411e-334eb81797df@idea \
--to=ppalka@redhat.com \
--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).