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 [63.128.21.124]) by sourceware.org (Postfix) with ESMTP id BE4F0385782E for ; Fri, 25 Sep 2020 20:44:17 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org BE4F0385782E Received: from mail-qt1-f199.google.com (mail-qt1-f199.google.com [209.85.160.199]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-300-Bbw8TKZJMNybFFbGY80F4g-1; Fri, 25 Sep 2020 16:44:12 -0400 X-MC-Unique: Bbw8TKZJMNybFFbGY80F4g-1 Received: by mail-qt1-f199.google.com with SMTP id b18so3179860qto.4 for ; Fri, 25 Sep 2020 13:44:12 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=lHAo+nOgNS5R964ykhiekui1lq8mgxnpGgsDvTp7Qtc=; b=oqiJwxf0P3L31TJSAjfG/ufUgQji5b+hTdSfjGsXt0/zrDeZBMZPIEd6sb+kJ3x47V tGcVfFpnl7pKFoAy2eYW79Ujp6bjm2bu8Q4jXVBcmyEBltJpatwEMMwqmDPImuuB0byn brOYViA2fiCTa6sZu39IcojuxCZQKwEJBYC5n4+y8MQhyvaIzCWm/6/zuml2o1Sl3Wm4 JpyEg572KsjP1UAQktGYEQuIXgn8rFYny1uOjPenSEiN1ePlQitfXHUvpCFPSYWLPsms TVkEDtCF7/gt0vrGDlBoOh8SvU+n/oXJM+e9HTZs4rRMvTL6tR50oxz4VampV2t1gZks QYlA== X-Gm-Message-State: AOAM531/1Gy9w07ZBjj/jGvsDEds55S3s6mSK8aC2pgZ4G8MCFV+R7Y7 bpAOLf4xdL4TvgQtQ7rg916vMqlIvxOMYQC7Vweh4hveitcpiLpCpRyPsL1pz+7o9ITM8Qb1jSA MW/65KFiJYV6wKh+DuQ== X-Received: by 2002:a37:a143:: with SMTP id k64mr1988473qke.266.1601066651682; Fri, 25 Sep 2020 13:44:11 -0700 (PDT) X-Google-Smtp-Source: ABdhPJwZrx+WyRu3A/wppa3ERBMu8MSuZLYX2QTTM3pYG4WpBDDVwUgWHSHBLYA7Uj0BE4WfhqhlWA== X-Received: by 2002:a37:a143:: with SMTP id k64mr1988440qke.266.1601066651298; Fri, 25 Sep 2020 13:44:11 -0700 (PDT) Received: from localhost.localdomain (ool-457d493a.dyn.optonline.net. [69.125.73.58]) by smtp.gmail.com with ESMTPSA id t43sm2879714qtc.54.2020.09.25.13.44.10 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 25 Sep 2020 13:44:10 -0700 (PDT) From: Patrick Palka To: gcc-patches@gcc.gnu.org Subject: [PATCH] c++: Incomplete parameter mappings during normalization Date: Fri, 25 Sep 2020 16:44:09 -0400 Message-Id: <20200925204409.1038658-1-ppalka@redhat.com> X-Mailer: git-send-email 2.28.0.585.ge1cfff6765 MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset="US-ASCII" X-Spam-Status: No, score=-16.2 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_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) 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, 25 Sep 2020 20:44:19 -0000 In the testcase concepts7.C below, we currently reject the call to f1 but we accept the call to f2, even though their associated constraints are functionally equivalent. The reason satisfaction differs for (!!C is due to normalization: the former is already an atom, and the latter is not. Normalization of the former yields itself, whereas normalization of the latter yields the atom 'true' with an empty parameter mapping (since the atom uses no template parameters). So when building the latter atom we threw away the T::type term that would later result in substitution failure during satisfaction. However, [temp.constr.normal]/1 says: - ... - The normal form of a concept-id C is the normal form of the constraint-expression of C, after substituting A1, A2, ..., An for C's respective template parameters in the parameter mappings in each atomic constraint. - The normal form of any other expression E is the atomic constraint whose expression is E and whose parameter mapping is the identity mapping. I believe these two bullet points imply that the atom 'true' in the normal form of C should have the mapping R |-> T::type instead of the empty mapping that we give it, because according to the last bullet point, each atom should start out with the identity mapping that includes all template parameters. This patch fixes this issue by always giving the first atom in the normal form of each concept a 'complete' parameter mapping, i.e. one that includes all template parameters. I think it suffices to do this only for the first atom so that we catch substitution failures like in concepts7.C at the right time. For the other atoms, their mappings can continue to include only template parameters used in the atom. I noticed that PR92268 alludes to this issue, so this patch refers to that PR and adds the PR's first testcase which we now accept. Is the above interpretation of the standard correct here? If so, does this seem like a good approach? gcc/cp/ChangeLog: PR c++/92268 * constraint.cc (build_parameter_mapping): Add a bool parameter 'complete'. When 'complete' is true, then include all in-scope template parameters in the mapping. (norm_info::update_context): Pass false as the 'complete' argument to build_parameter_mapping. (norm_info::normalized_first_atom_p): New bool data member. (normalize_logical_operation): Set info.normalized_first_atom_p after normalizing the left operand. (normalize_concept_check): Reset info.normalized_first_atom_p before normalizing this concept. (normalize_atom): Always give the first atom of a concept definition a complete parameter mapping. gcc/testsuite/ChangeLog: PR c++/92268 * g++.dg/cpp2a/concepts-pr92268.C: New test. * g++.dg/cpp2a/concepts-return-req1.C: Don't expect an error, as the call is no longer ambiguous. * g++.dg/cpp2a/concepts7.C: New test. * g++.dg/cpp2a/concepts8.C: New test. --- gcc/cp/constraint.cc | 44 ++++++++++++++++--- gcc/testsuite/g++.dg/cpp2a/concepts-pr92268.C | 42 ++++++++++++++++++ .../g++.dg/cpp2a/concepts-return-req1.C | 2 +- gcc/testsuite/g++.dg/cpp2a/concepts7.C | 9 ++++ gcc/testsuite/g++.dg/cpp2a/concepts8.C | 25 +++++++++++ 5 files changed, 116 insertions(+), 6 deletions(-) create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-pr92268.C create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts7.C create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts8.C diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc index d49957a6c4a..729d02b73d7 100644 --- a/gcc/cp/constraint.cc +++ b/gcc/cp/constraint.cc @@ -559,10 +559,14 @@ map_arguments (tree parms, tree args) return parms; } -/* Build the parameter mapping for EXPR using ARGS. */ +/* Build the parameter mapping for EXPR using ARGS. + + If COMPLETE then return the complete parameter mapping that includes + all in-scope template parameters. Otherwise include only the + parameters used by EXPR. */ static tree -build_parameter_mapping (tree expr, tree args, tree decl) +build_parameter_mapping (tree expr, tree args, tree decl, bool complete) { tree ctx_parms = NULL_TREE; if (decl) @@ -579,6 +583,24 @@ build_parameter_mapping (tree expr, tree args, tree decl) ctx_parms = current_template_parms; } + if (!ctx_parms) + return NULL_TREE; + + if (complete) + { + if (!processing_template_parmlist) + /* Search through ctx_parms to build a complete mapping. */ + expr = template_parms_to_args (ctx_parms); + else + /* The expression might use parameters introduced in the currently + open template parameter list, which ctx_parms doesn't yet have. + So we need to search through the expression in addition to + ctx_parms. */ + expr = tree_cons (NULL_TREE, expr, + build_tree_list (NULL_TREE, + template_parms_to_args (ctx_parms))); + } + tree parms = find_template_parameters (expr, ctx_parms); tree map = map_arguments (parms, args); return map; @@ -635,7 +657,8 @@ struct norm_info : subst_info { if (generate_diagnostics ()) { - tree map = build_parameter_mapping (expr, args, in_decl); + tree map = build_parameter_mapping (expr, args, in_decl, + /*complete=*/false); context = tree_cons (map, expr, context); } in_decl = get_concept_check_template (expr); @@ -647,6 +670,9 @@ struct norm_info : subst_info for that check. */ tree context; + + /* True if we've normalized the first atom of this concept. */ + bool normalized_first_atom_p = false; }; static tree normalize_expression (tree, tree, norm_info); @@ -658,6 +684,7 @@ static tree normalize_logical_operation (tree t, tree args, tree_code c, norm_info info) { tree t0 = normalize_expression (TREE_OPERAND (t, 0), args, info); + info.normalized_first_atom_p = true; tree t1 = normalize_expression (TREE_OPERAND (t, 1), args, info); /* Build a new info object for the constraint. */ @@ -706,6 +733,7 @@ normalize_concept_check (tree check, tree args, norm_info info) return error_mark_node; info.update_context (check, args); + info.normalized_first_atom_p = false; return normalize_expression (def, subst, info); } @@ -722,8 +750,14 @@ normalize_atom (tree t, tree args, norm_info info) if (concept_check_p (t)) return normalize_concept_check (t, args, info); - /* Build the parameter mapping for the atom. */ - tree map = build_parameter_mapping (t, args, info.in_decl); + /* Build the parameter mapping for the atom. If T is the first + atom of a concept definition then we want this mapping to be + complete, so that during satisfaction we check all substituted + template parameters first and foremost. */ + bool complete = (!info.normalized_first_atom_p + && info.in_decl != NULL_TREE + && concept_definition_p (info.in_decl)); + tree map = build_parameter_mapping (t, args, info.in_decl, complete); /* Build a new info object for the atom. */ tree ci = build_tree_list (t, info.context); diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-pr92268.C b/gcc/testsuite/g++.dg/cpp2a/concepts-pr92268.C new file mode 100644 index 00000000000..817cd3187bd --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-pr92268.C @@ -0,0 +1,42 @@ +// PR c++/92268 +// { dg-do compile { target c++20 } } + +template + struct common_reference { }; + +template + using common_reference_t = typename common_reference::type; + +template concept foo = true; +template concept bar = true; +template concept baz = true; + +template + concept common_reference_with + = foo, common_reference_t> + && bar, common_reference_t> + && baz, common_reference_t>; + +template + using iter_reference_t = decltype(((T*)0)->f()); + +template + concept forward_iterator + = common_reference_with&&, typename I::value_type&>; + +struct test_range +{ + struct iterator + { + using value_type = int; + + char f() const; + }; + + iterator begin(); +}; + +template +concept F = requires (T& t) { { t.begin() } -> forward_iterator; }; + +static_assert( !F ); diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-return-req1.C b/gcc/testsuite/g++.dg/cpp2a/concepts-return-req1.C index d21a49be14e..84c9ae9d6da 100644 --- a/gcc/testsuite/g++.dg/cpp2a/concepts-return-req1.C +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-return-req1.C @@ -15,5 +15,5 @@ int f(...); int main() { - f(); // { dg-error "ambiguous" } + f(); } diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts7.C b/gcc/testsuite/g++.dg/cpp2a/concepts7.C new file mode 100644 index 00000000000..780cfc859ce --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp2a/concepts7.C @@ -0,0 +1,9 @@ +// { dg-do compile { target c++20 } } + +template concept C = true; + +template int f1(T) requires (!!C); // { dg-error "'int'" } +template int f2(T) requires C; + +int i1 = f1(0); // { dg-error "no match" } +int i2 = f2(0); // { dg-error "no match|'int'" } diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts8.C b/gcc/testsuite/g++.dg/cpp2a/concepts8.C new file mode 100644 index 00000000000..29d3e24e323 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp2a/concepts8.C @@ -0,0 +1,25 @@ +// { dg-do compile { target c++20 } } + +template concept C = true; + +template struct W { using type = T::type; }; + +struct S { using type = int; }; + +template int f1() requires (C + && C::type>); + +int i1 = f1(); // { dg-error "no match|'int'" } +int i2 = f1(); + +template int f2() requires (C + && C::type>); + +int i3 = f2(); // { dg-error "no match|'int'" } +int i4 = f2(); +int i5 = f2(); // { dg-error "no match|'int" } + +template int f3() requires C::type>; + +int i6 = f3(); // { dg-error "no match|'int'" } +int i7 = f3(); -- 2.28.0.585.ge1cfff6765