From patchwork Tue Nov 19 20:21:57 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jason Merrill X-Patchwork-Id: 1197627 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-514068-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.b="IrTf8MYg"; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.b="jVhGbAOS"; 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 47Hcgd5lDyz9sPK for ; Wed, 20 Nov 2019 07:22:20 +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:subject:date:message-id:content-type :content-transfer-encoding; q=dns; s=default; b=iySPNpUm39Ae3Gv0 amG2KyZ4wVN1CmDZ7yPcPbMWB+JLDW9TAkT6ttL6szi53pCgo2VPjDUXBN0jRc6P sah7iIoZHduV3xxwXLcyWk0NUk+HaT0cOdffPmf4PT46LYgMvh1HVBnMDg/7ByDj 8hlPsdKHUhxhGsoMTwpvEdVPNzo= 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:subject:date:message-id:content-type :content-transfer-encoding; s=default; bh=aMrGfhzLYgECGCUoAxeHNP V1XHk=; b=IrTf8MYgtTFtEI5nWt5KTypT1gXmCpavfNbDed2Gn1IuHEc7Ltwyl3 W5xLPe3jFoPI55WtP23BIZBersjnDMlukGNUhxZeSBCoOaZo9O6Wi7+D72MZdl1x yDgXOYIndnkfJQxiIb7+NDGzFLC3FrCX1TNrGpvlfCyHRKi6uaUUQ= Received: (qmail 18779 invoked by alias); 19 Nov 2019 20:22:10 -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 18770 invoked by uid 89); 19 Nov 2019 20:22:10 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, SPF_PASS autolearn=ham version=3.3.1 spammy=sk:spacesh, silence, op_location_t, Warn 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; Tue, 19 Nov 2019 20:22:08 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1574194926; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=LvByfmnEN47N6Cm3N8GeR4jYTNN1JedrlXj2SEaUKkw=; b=jVhGbAOSWn/UAr0iLKgdk7Soh7AuSnMXExMIP8ZUF7+a7lvNwRCmlv0r1W0aXjlsyIITJN YNBRl2UpYgPX2LvnKG349OoXD3BhwtPE+VsBcYISJBCtgKHqM9kCEz9DEYdhm+YwESPE8x rDZHNh+EUwrFWfa2kVvtIYs4bWJz7To= 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-228-_WFNIObrPOK6Iwg65FN3Xg-1; Tue, 19 Nov 2019 15:22:04 -0500 Received: by mail-qt1-f199.google.com with SMTP id u23so15413800qtb.22 for ; Tue, 19 Nov 2019 12:22:04 -0800 (PST) Received: from barrymore.cygnus.csb ([172.58.220.69]) by smtp.gmail.com with ESMTPSA id h4sm241766qkk.128.2019.11.19.12.22.00 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 19 Nov 2019 12:22:02 -0800 (PST) From: Jason Merrill To: gcc-patches@gcc.gnu.org Subject: [C++ PATCH] Consider parm types equivalence for operator rewrite tiebreaker. Date: Tue, 19 Nov 2019 15:21:57 -0500 Message-Id: <20191119202157.20433-1-jason@redhat.com> X-Mimecast-Spam-Score: 0 X-IsSubscribed: yes The C++ committee continues to discuss how best to avoid breaking existing code with the new rules for reversed operators. A recent suggestion was to base the tie-breaker on the parameter types of the candidates, which made a lot of sense to me, so this patch implements that. This patch also mentions that a candidate was reversed or rewritten when printing the list of candidates, and warns about a comparison that becomes recursive under the new rules. There is no flag for this warning; people can silence it by swapping the operands. Tested x86_64-pc-linux-gnu, applying to trunk. * call.c (same_fn_or_template): Change to cand_parms_match. (joust): Adjust. (print_z_candidate): Mark rewritten/reversed candidates. (build_new_op_1): Warn about recursive call with reversed arguments. --- gcc/cp/call.c | 63 ++++++++++++------- .../g++.dg/cpp2a/spaceship-rewrite2.C | 12 ++++ .../g++.dg/cpp2a/spaceship-rewrite3.C | 10 +++ .../g++.dg/cpp2a/spaceship-rewrite4.C | 12 ++++ 4 files changed, 73 insertions(+), 24 deletions(-) create mode 100644 gcc/testsuite/g++.dg/cpp2a/spaceship-rewrite2.C create mode 100644 gcc/testsuite/g++.dg/cpp2a/spaceship-rewrite3.C create mode 100644 gcc/testsuite/g++.dg/cpp2a/spaceship-rewrite4.C base-commit: 73073838701384acd8d341bdf9a38ad8b9f4053f diff --git a/gcc/cp/call.c b/gcc/cp/call.c index 13639a1c901..f4dfa7b3f56 100644 --- a/gcc/cp/call.c +++ b/gcc/cp/call.c @@ -3694,6 +3694,10 @@ print_z_candidate (location_t loc, const char *msgstr, inform (cloc, "%s%#qD (near match)", msg, fn); else if (DECL_DELETED_FN (fn)) inform (cloc, "%s%#qD (deleted)", msg, fn); + else if (candidate->reversed ()) + inform (cloc, "%s%#qD (reversed)", msg, fn); + else if (candidate->rewritten ()) + inform (cloc, "%s%#qD (rewritten)", msg, fn); else inform (cloc, "%s%#qD", msg, fn); if (fn != candidate->fn) @@ -6219,8 +6223,14 @@ build_new_op_1 (const op_location_t &loc, enum tree_code code, int flags, else { if (cand->reversed ()) - /* We swapped these in add_candidate, swap them back now. */ - std::swap (cand->convs[0], cand->convs[1]); + { + /* We swapped these in add_candidate, swap them back now. */ + std::swap (cand->convs[0], cand->convs[1]); + if (cand->fn == current_function_decl) + warning_at (loc, 0, "in C++20 this comparison calls the " + "current function recursively with reversed " + "arguments"); + } result = build_over_call (cand, LOOKUP_NORMAL, complain); } @@ -10995,18 +11005,32 @@ joust_maybe_elide_copy (z_candidate *&cand) return false; } -/* True if cand1 and cand2 represent the same function or function - template. */ +/* True if the defining declarations of the two candidates have equivalent + parameters. */ -static bool -same_fn_or_template (z_candidate *cand1, z_candidate *cand2) +bool +cand_parms_match (z_candidate *c1, z_candidate *c2) { - if (cand1->fn == cand2->fn) + tree fn1 = c1->template_decl; + tree fn2 = c2->template_decl; + if (fn1 && fn2) + { + fn1 = most_general_template (TI_TEMPLATE (fn1)); + fn1 = DECL_TEMPLATE_RESULT (fn1); + fn2 = most_general_template (TI_TEMPLATE (fn2)); + fn2 = DECL_TEMPLATE_RESULT (fn2); + } + else + { + fn1 = c1->fn; + fn2 = c2->fn; + } + if (fn1 == fn2) return true; - if (!cand1->template_decl || !cand2->template_decl) + if (identifier_p (fn1) || identifier_p (fn2)) return false; - return (most_general_template (TI_TEMPLATE (cand1->template_decl)) - == most_general_template (TI_TEMPLATE (cand2->template_decl))); + return compparms (TYPE_ARG_TYPES (TREE_TYPE (fn1)), + TYPE_ARG_TYPES (TREE_TYPE (fn2))); } /* Compare two candidates for overloading as described in @@ -11155,20 +11179,11 @@ joust (struct z_candidate *cand1, struct z_candidate *cand2, bool warn, if (winner && comp != winner) { - if (same_fn_or_template (cand1, cand2)) - { - /* Ambiguity between normal and reversed versions of the - same comparison operator; prefer the normal one. - https://lists.isocpp.org/core/2019/10/7438.php */ - if (cand1->reversed ()) - winner = -1; - else - { - gcc_checking_assert (cand2->reversed ()); - winner = 1; - } - break; - } + /* Ambiguity between normal and reversed comparison operators + with the same parameter types; prefer the normal one. */ + if ((cand1->reversed () != cand2->reversed ()) + && cand_parms_match (cand1, cand2)) + return cand1->reversed () ? -1 : 1; winner = 0; goto tweak; diff --git a/gcc/testsuite/g++.dg/cpp2a/spaceship-rewrite2.C b/gcc/testsuite/g++.dg/cpp2a/spaceship-rewrite2.C new file mode 100644 index 00000000000..4f3d12757ff --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp2a/spaceship-rewrite2.C @@ -0,0 +1,12 @@ +// Test that C++20 overload changes don't break sloppy code. + +struct C { + bool operator==(const C&); + bool operator!=(const C&); +}; + +int main() { + C c1, c2; + (void)(c1 == c2); + (void)(c1 != c2); +} diff --git a/gcc/testsuite/g++.dg/cpp2a/spaceship-rewrite3.C b/gcc/testsuite/g++.dg/cpp2a/spaceship-rewrite3.C new file mode 100644 index 00000000000..ef29cffbd0b --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp2a/spaceship-rewrite3.C @@ -0,0 +1,10 @@ +// Test that very different operators still cause ambiguity with reversed. + +struct X { operator int(); }; +bool operator==(X, int); // #1 { dg-message "reversed" "" { target c++2a } } +struct Y { operator int(); }; +bool operator==(Y, int); // #2 { dg-message "reversed" "" { target c++2a } } + +X x; Y y; +bool b1 = x == y; // { dg-error "ambiguous" "" { target c++2a } } +bool b2 = y == x; // { dg-error "ambiguous" "" { target c++2a } } diff --git a/gcc/testsuite/g++.dg/cpp2a/spaceship-rewrite4.C b/gcc/testsuite/g++.dg/cpp2a/spaceship-rewrite4.C new file mode 100644 index 00000000000..c068b5ab294 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp2a/spaceship-rewrite4.C @@ -0,0 +1,12 @@ +// { dg-do compile { target c++11 } } + +struct iterator; +struct const_iterator { + const_iterator(const iterator&); + bool operator==(const const_iterator &ci) const = delete; +}; +struct iterator { + bool operator==(const const_iterator &ci) const { + return ci == *this; // { dg-error "deleted" "" { target c++17_down } } + } // { dg-warning "reversed" "" { target c++2a } .-1 } +};