From: Jason Merrill <jason@redhat.com>
To: Patrick Palka <ppalka@redhat.com>
Cc: gcc-patches@gcc.gnu.org
Subject: Re: [PATCH] c++: return-type-req in constraint using only outer tparms [PR104527]
Date: Fri, 11 Mar 2022 17:19:44 -0500 [thread overview]
Message-ID: <d84b3155-1bbd-489f-113f-5977f7a06083@redhat.com> (raw)
In-Reply-To: <211b752f-ac4a-36c1-4212-49077259459b@idea>
On 3/10/22 16:57, Patrick Palka wrote:
>
> On Thu, 10 Mar 2022, Jason Merrill wrote:
>
>> On 2/16/22 15:56, Patrick Palka wrote:
>>> On Tue, 15 Feb 2022, Jason Merrill wrote:
>>>
>>>> On 2/14/22 11:32, Patrick Palka wrote:
>>>>> Here the template context for the atomic constraint has two levels of
>>>>> template arguments, but since it depends only on the innermost argument
>>>>> T we use a single-level argument vector during substitution into the
>>>>> constraint (built by get_mapped_args). We eventually pass this vector
>>>>> to do_auto_deduction as part of checking the return-type-requirement
>>>>> inside the atom, but do_auto_deduction expects outer_targs to be a full
>>>>> set of arguments for sake of satisfaction.
>>>>
>>>> Could we note the current number of levels in the map and use that in
>>>> get_mapped_args instead of the highest level parameter we happened to use?
>>>
>>> Ah yeah, that seems to work nicely. IIUC it should suffice to remember
>>> whether the atomic constraint expression came from a concept definition.
>>> If it did, then the depth of the argument vector returned by
>>> get_mapped_args must be one, otherwise (as in the testcase) it must be
>>> the same as the template depth of the constrained entity, which is the
>>> depth of ARGS.
>>>
>>> How does the following look? Bootstrapped and regtested on
>>> x86_64-pc-linux-gnu and also on cmcstl2 and range-v3.
>>>
>>> -- >8 --
>>>
>>> Subject: [PATCH] c++: return-type-req in constraint using only outer tparms
>>> [PR104527]
>>>
>>> Here the template context for the atomic constraint has two levels of
>>> template parameters, but since it depends only on the innermost parameter
>>> T we use a single-level argument vector (built by get_mapped_args) during
>>> substitution into the atom. We eventually pass this vector to
>>> do_auto_deduction as part of checking the return-type-requirement within
>>> the atom, but do_auto_deduction expects outer_targs to be a full set of
>>> arguments for sake of satisfaction.
>>>
>>> This patch fixes this by making get_mapped_args always return an
>>> argument vector whose depth corresponds to the template depth of the
>>> context in which the atomic constraint expression was written, instead
>>> of the highest parameter level that the expression happens to use.
>>>
>>> PR c++/104527
>>>
>>> gcc/cp/ChangeLog:
>>>
>>> * constraint.cc (normalize_atom): Set
>>> ATOMIC_CONSTR_EXPR_FROM_CONCEPT_P appropriately.
>>> (get_mapped_args): Make static, adjust parameters. Always
>>> return a vector whose depth corresponds to the template depth of
>>> the context of the atomic constraint expression. Micro-optimize
>>> by passing false as exact to safe_grow_cleared and by collapsing
>>> a multi-level depth-one argument vector.
>>> (satisfy_atom): Adjust call to get_mapped_args and
>>> diagnose_atomic_constraint.
>>> (diagnose_atomic_constraint): Replace map parameter with an args
>>> parameter.
>>> * cp-tree.h (ATOMIC_CONSTR_EXPR_FROM_CONCEPT_P): Define.
>>> (get_mapped_args): Remove declaration.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> * g++.dg/cpp2a/concepts-return-req4.C: New test.
>>> ---
>>> gcc/cp/constraint.cc | 64 +++++++++++--------
>>> gcc/cp/cp-tree.h | 7 +-
>>> .../g++.dg/cpp2a/concepts-return-req4.C | 24 +++++++
>>> 3 files changed, 69 insertions(+), 26 deletions(-)
>>> create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-return-req4.C
>>>
>>> diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
>>> index 12db7e5cf14..306e28955c6 100644
>>> --- a/gcc/cp/constraint.cc
>>> +++ b/gcc/cp/constraint.cc
>>> @@ -764,6 +764,8 @@ normalize_atom (tree t, tree args, norm_info info)
>>> tree ci = build_tree_list (t, info.context);
>>> tree atom = build1 (ATOMIC_CONSTR, ci, map);
>>> + if (info.in_decl && concept_definition_p (info.in_decl))
>>> + ATOMIC_CONSTR_EXPR_FROM_CONCEPT_P (atom) = true;
>>
>> I'm a bit nervous about relying on in_decl, given that we support normalizing
>> when it isn't set; I don't remember the circumstances for that. Maybe make
>> the flag indicate that ctx_parms had depth 1?
>
> in_decl gets reliably updated by norm_info::update_context whenever we
> recurse inside a concept-id during normalization. And I think the only
> other situation we have to worry about is when starting out with a
> concept-id, which is handled by normalize_concept_definition where we
> also set in_decl appropriately.
>
> AFAICT, in_decl is not set (at the start) only when normalizing a
> placeholder type constraint or nested-requirement, and from some
> subsumption entrypoints. And we shouldn't see an atom that belongs to a
> concept in these cases unless we recurse into a concept-id, in which
> case norm_info::update_context will update in_decl appropriately.
>
> So IMHO it should be safe to rely on in_decl here to detect if the atom
> belongs to a concept, at least given the current entrypoints to
> subsumption/satisfaction..
Sounds good; please put a bit of that explanation in a comment where you
set the flag. OK with that change.
>>
>>> if (!info.generate_diagnostics ())
>>> {
>>> /* Cache the ATOMIC_CONSTRs that we return, so that
>>> sat_hasher::equal
>>> @@ -2826,33 +2828,37 @@ satisfaction_value (tree t)
>>> return boolean_true_node;
>>> }
>>> -/* Build a new template argument list with template arguments
>>> corresponding
>>> - to the parameters used in an atomic constraint. */
>>> +/* Build a new template argument vector according to the parameter
>>> + mapping of the atomic constraint T, using arguments from ARGS. */
>>> -tree
>>> -get_mapped_args (tree map)
>>> +static tree
>>> +get_mapped_args (tree t, tree args)
>>> {
>>> + tree map = ATOMIC_CONSTR_MAP (t);
>>> +
>>> /* No map, no arguments. */
>>> if (!map)
>>> return NULL_TREE;
>>> - /* Find the mapped parameter with the highest level. */
>>> - int count = 0;
>>> - for (tree p = map; p; p = TREE_CHAIN (p))
>>> - {
>>> - int level;
>>> - int index;
>>> - template_parm_level_and_index (TREE_VALUE (p), &level, &index);
>>> - if (level > count)
>>> - count = level;
>>> - }
>>> + /* Determine the depth of the resulting argument vector. */
>>> + int depth;
>>> + if (ATOMIC_CONSTR_EXPR_FROM_CONCEPT_P (t))
>>> + /* The expression of this atomic constraint comes from a concept
>>> definition,
>>> + whose template depth is always one, so the resulting argument vector
>>> + will also have depth one. */
>>> + depth = 1;
>>> + else
>>> + /* Otherwise, the expression of this atomic constraint was written in
>>> + the context of the constrained entity, whose template depth is that
>>> + of ARGS. */
>>> + depth = TMPL_ARGS_DEPTH (args);
>>> /* Place each argument at its corresponding position in the argument
>>> list. Note that the list will be sparse (not all arguments supplied),
>>> but instantiation is guaranteed to only use the parameters in the
>>> mapping, so null arguments would never be used. */
>>> - auto_vec< vec<tree> > lists (count);
>>> - lists.quick_grow_cleared (count);
>>> + auto_vec< vec<tree> > lists (depth);
>>> + lists.quick_grow_cleared (depth);
>>> for (tree p = map; p; p = TREE_CHAIN (p))
>>> {
>>> int level;
>>> @@ -2862,12 +2868,12 @@ get_mapped_args (tree map)
>>> /* Insert the argument into its corresponding position. */
>>> vec<tree> &list = lists[level - 1];
>>> if (index >= (int)list.length ())
>>> - list.safe_grow_cleared (index + 1, true);
>>> + list.safe_grow_cleared (index + 1, /*exact=*/false);
>>> list[index] = TREE_PURPOSE (p);
>>> }
>>> /* Build the new argument list. */
>>> - tree args = make_tree_vec (lists.length ());
>>> + args = make_tree_vec (lists.length ());
>>> for (unsigned i = 0; i != lists.length (); ++i)
>>> {
>>> vec<tree> &list = lists[i];
>>> @@ -2879,6 +2885,16 @@ get_mapped_args (tree map)
>>> }
>>> SET_NON_DEFAULT_TEMPLATE_ARGS_COUNT (args, 0);
>>> + if (TMPL_ARGS_HAVE_MULTIPLE_LEVELS (args)
>>> + && TMPL_ARGS_DEPTH (args) == 1)
>>> + {
>>> + /* Micro-optimization: represent a depth-one argument vector
>>> + using a single level. */
>>> + tree level = TMPL_ARGS_LEVEL (args, 1);
>>> + ggc_free (args);
>>> + args = level;
>>> + }
>>> +
>>> return args;
>>> }
>>> @@ -2933,7 +2949,7 @@ satisfy_atom (tree t, tree args, sat_info info)
>>> }
>>> /* Rebuild the argument vector from the parameter mapping. */
>>> - args = get_mapped_args (map);
>>> + args = get_mapped_args (t, args);
>>> /* Apply the parameter mapping (i.e., just substitute). */
>>> tree expr = ATOMIC_CONSTR_EXPR (t);
>>> @@ -2955,7 +2971,7 @@ satisfy_atom (tree t, tree args, sat_info info)
>>> if (!same_type_p (TREE_TYPE (result), boolean_type_node))
>>> {
>>> if (info.noisy ())
>>> - diagnose_atomic_constraint (t, map, result, info);
>>> + diagnose_atomic_constraint (t, args, result, info);
>>> return cache.save (inst_cache.save (error_mark_node));
>>> }
>>> @@ -2974,7 +2990,7 @@ satisfy_atom (tree t, tree args, sat_info info)
>>> }
>>> result = satisfaction_value (result);
>>> if (result == boolean_false_node && info.diagnose_unsatisfaction_p ())
>>> - diagnose_atomic_constraint (t, map, result, info);
>>> + diagnose_atomic_constraint (t, args, result, info);
>>> return cache.save (inst_cache.save (result));
>>> }
>>> @@ -3642,11 +3658,10 @@ diagnose_trait_expr (tree expr, tree args)
>>> }
>>> }
>>> -/* Diagnose a substitution failure in the atomic constraint T when
>>> applied
>>> - with the instantiated parameter mapping MAP. */
>>> +/* Diagnose a substitution failure in the atomic constraint T using ARGS.
>>> */
>>> static void
>>> -diagnose_atomic_constraint (tree t, tree map, tree result, sat_info info)
>>> +diagnose_atomic_constraint (tree t, tree args, tree result, sat_info info)
>>> {
>>> /* If the constraint is already ill-formed, we've previously diagnosed
>>> the reason. We should still say why the constraints aren't satisfied.
>>> */
>>> @@ -3667,7 +3682,6 @@ diagnose_atomic_constraint (tree t, tree map, tree
>>> result, sat_info info)
>>> /* Generate better diagnostics for certain kinds of expressions. */
>>> tree expr = ATOMIC_CONSTR_EXPR (t);
>>> STRIP_ANY_LOCATION_WRAPPER (expr);
>>> - tree args = get_mapped_args (map);
>>> switch (TREE_CODE (expr))
>>> {
>>> case TRAIT_EXPR:
>>> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
>>> index f253b32c3f2..dc2429a8406 100644
>>> --- a/gcc/cp/cp-tree.h
>>> +++ b/gcc/cp/cp-tree.h
>>> @@ -466,6 +466,7 @@ extern GTY(()) tree cp_global_trees[CPTI_MAX];
>>> IMPLICIT_CONV_EXPR_NONTYPE_ARG (in IMPLICIT_CONV_EXPR)
>>> BASELINK_FUNCTIONS_MAYBE_INCOMPLETE_P (in BASELINK)
>>> BIND_EXPR_VEC_DTOR (in BIND_EXPR)
>>> + ATOMIC_CONSTR_EXPR_FROM_CONCEPT_P (in ATOMIC_CONSTR)
>>> 2: IDENTIFIER_KIND_BIT_2 (in IDENTIFIER_NODE)
>>> ICS_THIS_FLAG (in _CONV)
>>> DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P (in VAR_DECL)
>>> @@ -1679,6 +1680,11 @@ check_constraint_info (tree t)
>>> #define ATOMIC_CONSTR_MAP_INSTANTIATED_P(NODE) \
>>> TREE_LANG_FLAG_0 (ATOMIC_CONSTR_CHECK (NODE))
>>> +/* Whether the expression for this atomic constraint belongs to a
>>> + concept definition. */
>>> +#define ATOMIC_CONSTR_EXPR_FROM_CONCEPT_P(NODE) \
>>> + TREE_LANG_FLAG_1 (ATOMIC_CONSTR_CHECK (NODE))
>>> +
>>> /* The expression of an atomic constraint. */
>>> #define ATOMIC_CONSTR_EXPR(NODE) \
>>> CONSTR_EXPR (ATOMIC_CONSTR_CHECK (NODE))
>>> @@ -8306,7 +8312,6 @@ extern tree evaluate_requires_expr
>>> (tree);
>>> extern tree tsubst_constraint (tree, tree,
>>> tsubst_flags_t, tree);
>>> extern tree tsubst_constraint_info (tree, tree,
>>> tsubst_flags_t, tree);
>>> extern tree tsubst_parameter_mapping (tree, tree,
>>> tsubst_flags_t, tree);
>>> -extern tree get_mapped_args (tree);
>>> struct processing_constraint_expression_sentinel
>>> {
>>> diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-return-req4.C
>>> b/gcc/testsuite/g++.dg/cpp2a/concepts-return-req4.C
>>> new file mode 100644
>>> index 00000000000..471946bc8eb
>>> --- /dev/null
>>> +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-return-req4.C
>>> @@ -0,0 +1,24 @@
>>> +// PR c++/104527
>>> +// { dg-do compile { target c++20 } }
>>> +
>>> +template<class T, class U>
>>> +concept is_same = __is_same(T, U);
>>> +
>>> +template<class T>
>>> +struct A {
>>> + template<class...>
>>> + requires requires { { 0 } -> is_same<T>; }
>>> + struct B {};
>>> +
>>> + template<class...>
>>> + requires requires { { 1 } -> is_same<T>; }
>>> + static void f();
>>> +};
>>> +
>>> +A<int>::B<> a1;
>>> +A<bool>::B<> a2; // { dg-error "constraint" }
>>> +
>>> +int main() {
>>> + A<int>::f();
>>> + A<bool>::f(); // { dg-error "no match" }
>>> +}
>>
>>
>
prev parent reply other threads:[~2022-03-11 22:19 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-02-14 16:32 Patrick Palka
2022-02-14 16:37 ` Patrick Palka
2022-02-15 23:39 ` Jason Merrill
2022-02-16 19:56 ` Patrick Palka
2022-03-01 13:13 ` Patrick Palka
2022-03-10 19:43 ` Jason Merrill
2022-03-10 20:57 ` Patrick Palka
2022-03-11 22:19 ` Jason Merrill [this message]
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=d84b3155-1bbd-489f-113f-5977f7a06083@redhat.com \
--to=jason@redhat.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=ppalka@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).