From patchwork Wed Feb 12 16:15:57 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Palka X-Patchwork-Id: 1236929 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=gcc-patches-return-519418-incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.a=rsa-sha1 header.s=default header.b=THPYlnPb; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=Vd1AVavp; dkim-atps=neutral Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 48HlBr3dJRz9s3x for ; Thu, 13 Feb 2020 03:16:34 +1100 (AEDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:cc:subject:date:message-id:mime-version:content-type :content-transfer-encoding; q=dns; s=default; b=o/aPG56Eo0Ap/tQI I2cwLGAdyqltJv9H6m8VgNGDPhE3sM0s7MedccsKgZw2sFYNk6geuqlX51UY2RbV TCCLRfRuQTheF56WM8tQ1tvcULLFsP2xWn4bMB0apvD+uwweD8O7YmjfR0oeKJSr 6aV4vM8buOi9nXLYBvzaTHMYffU= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:cc:subject:date:message-id:mime-version:content-type :content-transfer-encoding; s=default; bh=8CUYiKKJKUehtB+Gzh9qTf fIT1c=; b=THPYlnPbLUXeZUi48PbBla6pcNf7yMx/LowIY4ppDJkVghVfglNOqu /GM8xLpSw0jBH0HWurhuWzg36Fi8PJZVjovhsbLwPeaJ4lcOTZkXpayXTj6vdQhn 77OOkgccx399mw4tjYLMJZGStLsJqpId6pR1AXCivheyUSH6znakQ= Received: (qmail 82932 invoked by alias); 12 Feb 2020 16:16:25 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 82901 invoked by uid 89); 12 Feb 2020 16:16:24 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-24.1 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.1 spammy= X-HELO: us-smtp-delivery-1.mimecast.com Received: from us-smtp-2.mimecast.com (HELO us-smtp-delivery-1.mimecast.com) (205.139.110.61) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 12 Feb 2020 16:16:08 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1581524166; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=A/5xtB1z/4vY2Cz03UC9ZehGq428Kn0EvgQtBkcza/w=; b=Vd1AVavpJDxvBwdFvrUS2NhsRFmPcGI1/BAt+Dzz6HvouhMaSW3uRZ/alPnW+ujAb4/gzr bq7/sExqGE3qhV8oAwja6Oar9sxKzFt5e/A4leCr6SPW1vjN5K1vV0Te+Hc23C/HgUYx9C YQtvY6SCe/tQPjkIJL7Wy6SxoMYR+1c= Received: from mail-qt1-f200.google.com (mail-qt1-f200.google.com [209.85.160.200]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-26-Fp5UVtjaOCWYFipHN8-8ow-1; Wed, 12 Feb 2020 11:16:01 -0500 Received: by mail-qt1-f200.google.com with SMTP id k20so1575119qtm.11 for ; Wed, 12 Feb 2020 08:16:01 -0800 (PST) Received: from localhost.localdomain (ool-457d493a.dyn.optonline.net. [69.125.73.58]) by smtp.gmail.com with ESMTPSA id 85sm436710qko.49.2020.02.12.08.15.58 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 12 Feb 2020 08:15:59 -0800 (PST) From: Patrick Palka To: gcc-patches@gcc.gnu.org Cc: jason@redhat.com, Patrick Palka Subject: [PATCH] c++: Fix hashing and testing for equality of ATOMIC_CONST_EXPRs Date: Wed, 12 Feb 2020 11:15:57 -0500 Message-Id: <20200212161557.3623035-1-ppalka@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-IsSubscribed: yes Two equal atomic constraint expressions do not necessarily share the same tree, so we can't assume that two ATOMIC_CONST_EXPRs are equal if and only if they point to the same tree. The main consequence of this invalid assumption is that the constraint subsumption checker may reject a valid partial specialization for a constrained template because the checker decides that the constraints on the partial specialization do not subsume the constraints on the primary template, as in the test case below. Another consequence is that it probably makes the constraint caches less effective. This patch changes atomic_constraints_identical_p to test for equality of ATOMIC_CONST_EXPRs using cp_tree_equal, and changes hash_atomic_constraint to hash the ATOMIC_CONST_EXPR with iterative_hash_template_arg. Originally I used template_args_equal but cp_tree_equal seems to suffice. Does this look like an OK solution to this issue? Are the testcase adjustments valid? gcc/cp/ChangeLog: * constraint.cc (atomic_constraints_identical_p): Use cp_tree_equal to test for equality between ATOMIC_CONST_EXPRs. (hash_atomic_constraint): Use iterative_hash_template to hash the ATOMIC_CONST_EXPR. gcc/testsuite/ChangeLog: * g++.dg/concepts/variadic4.C: Adjust test to no longere expect an error. * g++.dg/cpp2a/concepts-pr84551.C: Likewise. * g++.dg/cpp2a/concepts-partial-spec7.C: New test. --- gcc/cp/constraint.cc | 4 +- gcc/testsuite/g++.dg/concepts/variadic4.C | 4 +- .../g++.dg/cpp2a/concepts-partial-spec7.C | 40 +++++++++++++++++++ gcc/testsuite/g++.dg/cpp2a/concepts-pr84551.C | 2 +- 4 files changed, 44 insertions(+), 6 deletions(-) create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-partial-spec7.C diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc index 58044cd0f9d..935c3556272 100644 --- a/gcc/cp/constraint.cc +++ b/gcc/cp/constraint.cc @@ -933,7 +933,7 @@ atomic_constraints_identical_p (tree t1, tree t2) gcc_assert (TREE_CODE (t1) == ATOMIC_CONSTR); gcc_assert (TREE_CODE (t2) == ATOMIC_CONSTR); - if (ATOMIC_CONSTR_EXPR (t1) != ATOMIC_CONSTR_EXPR (t2)) + if (!cp_tree_equal (ATOMIC_CONSTR_EXPR (t1), ATOMIC_CONSTR_EXPR (t2))) return false; if (!parameter_mapping_equivalent_p (t1, t2)) @@ -981,7 +981,7 @@ hash_atomic_constraint (tree t) gcc_assert (TREE_CODE (t) == ATOMIC_CONSTR); /* Hash the identity of the expression. */ - hashval_t val = htab_hash_pointer (ATOMIC_CONSTR_EXPR (t)); + hashval_t val = iterative_hash_template_arg (ATOMIC_CONSTR_EXPR (t), 0); /* Hash the targets of the parameter map. */ tree p = ATOMIC_CONSTR_MAP (t); diff --git a/gcc/testsuite/g++.dg/concepts/variadic4.C b/gcc/testsuite/g++.dg/concepts/variadic4.C index d6eea49b958..b073bcce5e9 100644 --- a/gcc/testsuite/g++.dg/concepts/variadic4.C +++ b/gcc/testsuite/g++.dg/concepts/variadic4.C @@ -12,9 +12,7 @@ struct zip; template requires requires { typename list; } // && (Sequence && ...) -struct zip {}; // { dg-error "does not specialize" } -// The constraints of the specialization and the sequence are not -// comparable; the specializations are unordered. +struct zip {}; int main() { diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-partial-spec7.C b/gcc/testsuite/g++.dg/cpp2a/concepts-partial-spec7.C new file mode 100644 index 00000000000..ed77ed4d362 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-partial-spec7.C @@ -0,0 +1,40 @@ +// { dg-do compile { target c++2a } } + +template + inline constexpr bool foo = true; + +template + inline constexpr bool bar = true; + +// The following two partial specializations are both valid, but the constraint +// subsumption checker used to compare ATOMIC_CONSTR_EXPRs by comparing their +// pointers, leading to it accepting the partial specialization of test1 and +// rejecting the partial specialization of test2: in the test1 case, the +// TEMPLATE_ID_EXPR of foo in the constraints was shared between the primary +// template and partial specialization, and in the test2 case the +// TEMPLATE_ID_EXPRs of foo in the constraints were different (but equal) +// trees. + +template + concept baz = foo; + +template + requires baz + struct test1 + { }; + +template + requires baz && bar + struct test1 + { }; + +template + requires foo + struct test2 + { }; + +template + requires foo && bar + struct test2 + { }; + diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-pr84551.C b/gcc/testsuite/g++.dg/cpp2a/concepts-pr84551.C index e40796f4975..e7498437178 100644 --- a/gcc/testsuite/g++.dg/cpp2a/concepts-pr84551.C +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-pr84551.C @@ -8,4 +8,4 @@ template requires C class TT> struct A {}; template requires true struct B {}; -A a; // { dg-error "" } +A a;