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.133.124]) by sourceware.org (Postfix) with ESMTPS id 11B573858D39 for ; Thu, 10 Mar 2022 20:57:08 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 11B573858D39 Received: from mail-qk1-f199.google.com (mail-qk1-f199.google.com [209.85.222.199]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-256-kFRsIuDfN_WeLICeMab6KQ-1; Thu, 10 Mar 2022 15:57:05 -0500 X-MC-Unique: kFRsIuDfN_WeLICeMab6KQ-1 Received: by mail-qk1-f199.google.com with SMTP id c19-20020a05620a0cf300b005f17891c015so4746379qkj.18 for ; Thu, 10 Mar 2022 12:57:05 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:date:to:cc:subject:in-reply-to:message-id :references:mime-version; bh=IDm4rmrXiy+SAxS0i7TxNAb3NUf/ZBdDh5VbY9CQkm4=; b=artf2Em9GpwkdEuGDTvMRe5uBebi8ROriP+ozjxz0GH6PPuKWY2EEycCbwdVJsFK9k 0nOT6vmgEvvZhIZsGi4oRi7yleOfNWxY5rM20vp9hCv6/8AkK6IANUOkXAKwLCdhP6NX rBJv8ccgl8xasvzYirja2J1jZNbeOuhagMvgR2a2iBU2VYb3/liTgIMKeujs4WsPbMnB 0M+5trR09hh8muMHXseeHG3dokdSqxw7CFTOxMiU8Mjbf9lsDNEUw9LenhcghaindAAE 5rmSsNhEmWKL88OEsBDMhSyjDHsDnz5i0LolRUhgTji13UBm2/cAsLxeDCL9k2YF/Gau S+nQ== X-Gm-Message-State: AOAM53393YTgRJ7Bhxm1mbI1DB5kBcMbnX06D0ThCdRXjxjRC723WmTK l7pyduaj76wSukoqJKbdVUY3VQsKAi/k6qTl9Xa9oMTVM4hjD73pIOrNmxWx+P1zexpr6TtTIj4 vlodWGb6nPYVo5TboSQ== X-Received: by 2002:a37:ab11:0:b0:67d:6742:2988 with SMTP id u17-20020a37ab11000000b0067d67422988mr2221062qke.51.1646945824995; Thu, 10 Mar 2022 12:57:04 -0800 (PST) X-Google-Smtp-Source: ABdhPJzh8MWQTwtOGpAT2WCqhD6YOGIS780U1zdUwmSxas8KIwaPLgW0U54Qq5rGncYPCM9PuzThHQ== X-Received: by 2002:a37:ab11:0:b0:67d:6742:2988 with SMTP id u17-20020a37ab11000000b0067d67422988mr2221046qke.51.1646945824587; Thu, 10 Mar 2022 12:57:04 -0800 (PST) Received: from [192.168.1.130] (ool-18e40894.dyn.optonline.net. [24.228.8.148]) by smtp.gmail.com with ESMTPSA id v29-20020a05620a091d00b0067b12a51de5sm2696223qkv.50.2022.03.10.12.57.03 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 10 Mar 2022 12:57:03 -0800 (PST) From: Patrick Palka X-Google-Original-From: Patrick Palka Date: Thu, 10 Mar 2022 15:57:02 -0500 (EST) To: Jason Merrill cc: Patrick Palka , gcc-patches@gcc.gnu.org Subject: Re: [PATCH] c++: return-type-req in constraint using only outer tparms [PR104527] In-Reply-To: Message-ID: <211b752f-ac4a-36c1-4212-49077259459b@idea> References: <20220214163250.3454164-1-ppalka@redhat.com> <4c1b4638-0ad6-f229-4d9b-ac2fb362d058@redhat.com> <508cd6cd-3d60-a846-411e-20599702c9d8@idea> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII X-Spam-Status: No, score=-14.4 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H5, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE 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, 10 Mar 2022 20:57:10 -0000 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.. > > > 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 > lists (count); > > - lists.quick_grow_cleared (count); > > + auto_vec< vec > 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 &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 &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 > > +concept is_same = __is_same(T, U); > > + > > +template > > +struct A { > > + template > > + requires requires { { 0 } -> is_same; } > > + struct B {}; > > + > > + template > > + requires requires { { 1 } -> is_same; } > > + static void f(); > > +}; > > + > > +A::B<> a1; > > +A::B<> a2; // { dg-error "constraint" } > > + > > +int main() { > > + A::f(); > > + A::f(); // { dg-error "no match" } > > +} > >