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 C600F3858D3C for ; Fri, 11 Mar 2022 22:19:49 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org C600F3858D3C Received: from mail-qt1-f199.google.com (mail-qt1-f199.google.com [209.85.160.199]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-152-dVgL0Wr2N5qrzRKXA9FGww-1; Fri, 11 Mar 2022 17:19:47 -0500 X-MC-Unique: dVgL0Wr2N5qrzRKXA9FGww-1 Received: by mail-qt1-f199.google.com with SMTP id t26-20020ac8739a000000b002e06aa03d4cso7557781qtp.13 for ; Fri, 11 Mar 2022 14:19:47 -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:cc:references:from:in-reply-to :content-transfer-encoding; bh=e0w7+UpTtmqcJBoM2D5MkQf6QYQnA7eb8IoybJh4hqk=; b=dcMskPARQW+IJ0UvnqKijkSMS8lqL+ZzA1DZ1q56SLwtMK8W6hzncykzkfE2NuxEJ0 1bzVGzJ5suyVrk42OXrVRccAllSIh4oY7rCUV6KNj9UWfRr3ybGDZyFUDHhFT71WDfHU h9+XbcZtMXopUcBN+i0PwqUqmjv30Ot+HPgicMkFqp89pkzTT93v8mUWS7B49H6O97uo 6gcBhwXy3JQZJWf1pnryE8mqAIm6yDYbTJgzeOMeSE2hooKl/ekD1DSdjSnqxl1EDdq7 kBCcOn8G6zoZNwGjOIscKHqOKjU+JJKm9cET63YobStuyAQCKH5Acdgjgbnfe5bktNU6 ouew== X-Gm-Message-State: AOAM532Uk2gTP4KY8gQXHN+t76FmoWQTVWPI3EKhu5VMtM4gLqjy9SWQ z0Xhgs93L8VmJ1rVlP0H1HGn9gpdVRD9IGtKrw3ne4oljxmmBusaqtKD8KwGc5hn/Z9c4JWEPHh 2f5QD8KBxdXyCQSFFqw== X-Received: by 2002:a37:b842:0:b0:479:2ecc:e51e with SMTP id i63-20020a37b842000000b004792ecce51emr7949327qkf.206.1647037186588; Fri, 11 Mar 2022 14:19:46 -0800 (PST) X-Google-Smtp-Source: ABdhPJynq/EuMKmVVFVLemGv7dQ9py43rkZJKCZ9DsKLEh0TGyWyqKHBv+fG//GTRxuJUQvoilowfg== X-Received: by 2002:a37:b842:0:b0:479:2ecc:e51e with SMTP id i63-20020a37b842000000b004792ecce51emr7949301qkf.206.1647037185994; Fri, 11 Mar 2022 14:19:45 -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 e7-20020a37ac07000000b0067d7cd47af4sm1291180qkm.31.2022.03.11.14.19.44 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 11 Mar 2022 14:19:45 -0800 (PST) Message-ID: Date: Fri, 11 Mar 2022 17:19:44 -0500 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.6.1 Subject: Re: [PATCH] c++: return-type-req in constraint using only outer tparms [PR104527] To: Patrick Palka Cc: gcc-patches@gcc.gnu.org References: <20220214163250.3454164-1-ppalka@redhat.com> <4c1b4638-0ad6-f229-4d9b-ac2fb362d058@redhat.com> <508cd6cd-3d60-a846-411e-20599702c9d8@idea> <211b752f-ac4a-36c1-4212-49077259459b@idea> From: Jason Merrill In-Reply-To: <211b752f-ac4a-36c1-4212-49077259459b@idea> 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=-12.7 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, NICE_REPLY_A, 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: Fri, 11 Mar 2022 22:19:52 -0000 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 > 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" } >>> +} >> >> >