From patchwork Thu Nov 10 01:56:08 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marek Polacek X-Patchwork-Id: 17914 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a5d:6687:0:0:0:0:0 with SMTP id l7csp670792wru; Wed, 9 Nov 2022 17:57:13 -0800 (PST) X-Google-Smtp-Source: AMsMyM43W8m9FW6+kwk6CcFnbnMN/UjK3GPaaEDfTDHLfdbdY5yi4Ki6b+FFjSL/MtaKrCU/ni/Q X-Received: by 2002:a17:907:2063:b0:7ad:fa6b:e84b with SMTP id qp3-20020a170907206300b007adfa6be84bmr40433603ejb.69.1668045431607; Wed, 09 Nov 2022 17:57:11 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1668045431; cv=none; d=google.com; s=arc-20160816; b=EfSBYAmJ6KxW50Ifl2hs3O/9im8FY7jW693MBEpDvaUeWW9P0EdochGAsah5EtHNNq g6FEMqyeSyIwFQYRXY4E6pAAppzRLXqrNCQpRp3f8xBmQsIT6qfT+MnlBXPm1x//5rqf Kkh2OOJCpuFtitNM/f53WPRi0Dle0Q4DuG4hraXqTUGU8ipWv6g9NBHq9HdjcQxoOcRL u7yOIBugV+aTte8VXRRuBWu7+AaqJWhLdM2c0Q1/oqW1icMU6eoA+hWlXTp3360gVhmu o14pByXafH+3430iaW0awVX09T1KqeQtTQDlLoUgHyDHty6H1IT/3LxIiK558xneqyqy D7+w== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:reply-to:from:list-subscribe:list-help:list-post :list-archive:list-unsubscribe:list-id:precedence :content-transfer-encoding:mime-version:message-id:date:subject:to :dmarc-filter:delivered-to:dkim-signature:dkim-filter; bh=te+wi+qi32tFvRN/MuxWa/33ivC4EwKU/XevAz5i0oA=; b=XmVjCQ1PD0ztfuUwt4MEu40sm/0YCizVF18YO/dy0GsgU+SM5nVYDwBoh2HDGNBM+3 dNt5vv9VgjuhNsgSdYQHfWSjImxdRSL+XpJp6dj8xmNMi9L/KBd4cMs5rNx9zuApGHU9 04dhNysf7NP4JevngBO262LgeyQNKSEkI73MCfdQGI1XbVf2Jxc1c7c+fz1JAZ/DJWcc T3E+UDyYdxNVwhF45+wksat0KjNwUMGGFzi5zr0y9GM2qoUfAw+KNXYZtkppyT41Spkj +uISK8IbOHyKpF6xazf7gbXY6TjU0Ij9E06xT5NYKbaRf5LWcFI7MJ1y1VECVXyT4RO+ gVIQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gcc.gnu.org header.s=default header.b=Y09VW7d3; spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 8.43.85.97 as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=gnu.org Received: from sourceware.org (ip-8-43-85-97.sourceware.org. [8.43.85.97]) by mx.google.com with ESMTPS id m23-20020aa7d357000000b00461e3e1a628si14537342edr.110.2022.11.09.17.57.11 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 09 Nov 2022 17:57:11 -0800 (PST) Received-SPF: pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 8.43.85.97 as permitted sender) client-ip=8.43.85.97; Authentication-Results: mx.google.com; dkim=pass header.i=@gcc.gnu.org header.s=default header.b=Y09VW7d3; spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 8.43.85.97 as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=gnu.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 8E9643858C52 for ; Thu, 10 Nov 2022 01:57:09 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 8E9643858C52 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1668045429; bh=te+wi+qi32tFvRN/MuxWa/33ivC4EwKU/XevAz5i0oA=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=Y09VW7d3AlcCa3+s8MSwCTkpMmCTbHxFo21oIW8mPea9c4xDUErJvhAS20lmhHi99 ZgzIiR41h8SwQCNt9V6Nt8sZohTJ2x/4kLLD2ovmfhyTPEfclA+6NuGdlw6mgVrd+w vvFz+NG3QavonfrhxFU+KKHOeUVZnQ3v0MT/iIEM= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org 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 A407C3858C52 for ; Thu, 10 Nov 2022 01:56:24 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org A407C3858C52 Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-198-78r0VQP8NK6puWo-JOpMAw-1; Wed, 09 Nov 2022 20:56:22 -0500 X-MC-Unique: 78r0VQP8NK6puWo-JOpMAw-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.rdu2.redhat.com [10.11.54.1]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id EC2D8101A54E for ; Thu, 10 Nov 2022 01:56:20 +0000 (UTC) Received: from pdp-11.lan (unknown [10.22.17.249]) by smtp.corp.redhat.com (Postfix) with ESMTP id CBFD040C2086; Thu, 10 Nov 2022 01:56:20 +0000 (UTC) To: GCC Patches , Jason Merrill Subject: [PATCH] c++: Extend -Wdangling-reference for std::minmax Date: Wed, 9 Nov 2022 20:56:08 -0500 Message-Id: <20221110015608.454675-1-polacek@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.1 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-11.9 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_H2, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) 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-Patchwork-Original-From: Marek Polacek via Gcc-patches From: Marek Polacek Reply-To: Marek Polacek Errors-To: gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org Sender: "Gcc-patches" X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1749072406435788941?= X-GMAIL-MSGID: =?utf-8?q?1749072406435788941?= This patch extends -Wdangling-reference to also warn for auto v = std::minmax(1, 2); which dangles because this overload of std::minmax returns a std::pair where the two references are bound to the temporaries created for the arguments of std::minmax. This is a common footgun, also described at in Notes. It works by extending do_warn_dangling_reference to also warn when the function returns a std::pair. std_pair_ref_ref_p is a new helper to check that. Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? gcc/cp/ChangeLog: * call.cc (std_pair_ref_ref_p): New. (do_warn_dangling_reference): Also warn when the function returns std::pair. Recurse into TARGET_EXPR_INITIAL. (maybe_warn_dangling_reference): Don't return early if we're initializing a std_pair_ref_ref_p. gcc/ChangeLog: * doc/gcc/gcc-command-options/options-controlling-c++-dialect.rst: Extend the description of -Wdangling-reference. gcc/testsuite/ChangeLog: * g++.dg/warn/Wdangling-reference6.C: New test. --- gcc/cp/call.cc | 52 ++++++++++++++++--- .../options-controlling-c++-dialect.rst | 10 ++++ .../g++.dg/warn/Wdangling-reference6.C | 38 ++++++++++++++ 3 files changed, 94 insertions(+), 6 deletions(-) create mode 100644 gcc/testsuite/g++.dg/warn/Wdangling-reference6.C base-commit: 1cdfd0e5cd5fc1f493d0832ed65d31320f9585b7 diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc index 492db9b59ad..bd3b64a7e26 100644 --- a/gcc/cp/call.cc +++ b/gcc/cp/call.cc @@ -13527,6 +13527,34 @@ initialize_reference (tree type, tree expr, return expr; } +/* Return true if T is std::pair. */ + +static bool +std_pair_ref_ref_p (tree t) +{ + /* First, check if we have std::pair. */ + if (!NON_UNION_CLASS_TYPE_P (t) + || !CLASSTYPE_TEMPLATE_INSTANTIATION (t)) + return false; + tree tdecl = TYPE_NAME (TYPE_MAIN_VARIANT (t)); + if (!decl_in_std_namespace_p (tdecl)) + return false; + tree name = DECL_NAME (tdecl); + if (!name || !id_equal (name, "pair")) + return false; + + /* Now see if the template arguments are both const T&. */ + tree args = CLASSTYPE_TI_ARGS (t); + if (TREE_VEC_LENGTH (args) != 2) + return false; + for (int i = 0; i < 2; i++) + if (!TYPE_REF_OBJ_P (TREE_VEC_ELT (args, i)) + || !CP_TYPE_CONST_P (TREE_TYPE (TREE_VEC_ELT (args, i)))) + return false; + + return true; +} + /* Helper for maybe_warn_dangling_reference to find a problematic CALL_EXPR that initializes the LHS (and at least one of its arguments represents a temporary, as outlined in maybe_warn_dangling_reference), or NULL_TREE @@ -13556,11 +13584,6 @@ do_warn_dangling_reference (tree expr) || warning_suppressed_p (fndecl, OPT_Wdangling_reference) || !warning_enabled_at (DECL_SOURCE_LOCATION (fndecl), OPT_Wdangling_reference) - /* If the function doesn't return a reference, don't warn. This - can be e.g. - const int& z = std::min({1, 2, 3, 4, 5, 6, 7}); - which doesn't dangle: std::min here returns an int. */ - || !TYPE_REF_OBJ_P (TREE_TYPE (TREE_TYPE (fndecl))) /* Don't emit a false positive for: std::vector v = ...; std::vector::const_iterator it = v.begin(); @@ -13573,6 +13596,20 @@ do_warn_dangling_reference (tree expr) && DECL_OVERLOADED_OPERATOR_IS (fndecl, INDIRECT_REF))) return NULL_TREE; + tree rettype = TREE_TYPE (TREE_TYPE (fndecl)); + /* If the function doesn't return a reference, don't warn. This + can be e.g. + const int& z = std::min({1, 2, 3, 4, 5, 6, 7}); + which doesn't dangle: std::min here returns an int. + + If the function returns a std::pair, we + warn, to detect e.g. + std::pair v = std::minmax(1, 2); + which also creates a dangling reference, because std::minmax + returns std::pair(b, a). */ + if (!(TYPE_REF_OBJ_P (rettype) || std_pair_ref_ref_p (rettype))) + return NULL_TREE; + /* Here we're looking to see if any of the arguments is a temporary initializing a reference parameter. */ for (int i = 0; i < call_expr_nargs (expr); ++i) @@ -13614,6 +13651,8 @@ do_warn_dangling_reference (tree expr) return do_warn_dangling_reference (TREE_OPERAND (expr, 2)); case PAREN_EXPR: return do_warn_dangling_reference (TREE_OPERAND (expr, 0)); + case TARGET_EXPR: + return do_warn_dangling_reference (TARGET_EXPR_INITIAL (expr)); default: return NULL_TREE; } @@ -13640,7 +13679,8 @@ maybe_warn_dangling_reference (const_tree decl, tree init) { if (!warn_dangling_reference) return; - if (!TYPE_REF_P (TREE_TYPE (decl))) + if (!(TYPE_REF_OBJ_P (TREE_TYPE (decl)) + || std_pair_ref_ref_p (TREE_TYPE (decl)))) return; /* Don't suppress the diagnostic just because the call comes from a system header. If the DECL is not in a system header, or if diff --git a/gcc/doc/gcc/gcc-command-options/options-controlling-c++-dialect.rst b/gcc/doc/gcc/gcc-command-options/options-controlling-c++-dialect.rst index 5b05d31aae9..8d2a2789ef6 100644 --- a/gcc/doc/gcc/gcc-command-options/options-controlling-c++-dialect.rst +++ b/gcc/doc/gcc/gcc-command-options/options-controlling-c++-dialect.rst @@ -855,6 +855,16 @@ In addition, these warning options have meanings only for C++ programs: const T& foo (const T&) { ... } #pragma GCC diagnostic pop + :option:`-Wdangling-reference` also warns about code like + + .. code-block:: c++ + + auto p = std::minmax(1, 2); + + where ``std::minmax`` returns ``std::pair``, and + both references dangle after the end of the full expression that contains + the call to `std::minmax``. + This warning is enabled by :option:`-Wall`. .. option:: -Wno-dangling-reference diff --git a/gcc/testsuite/g++.dg/warn/Wdangling-reference6.C b/gcc/testsuite/g++.dg/warn/Wdangling-reference6.C new file mode 100644 index 00000000000..bf849e290d9 --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Wdangling-reference6.C @@ -0,0 +1,38 @@ +// { dg-do compile { target c++17 } } +// { dg-options "-Wdangling-reference" } +// Test -Wdangling-reference with std::minmax. + +#include + +using U = std::pair; + +int +fn1 () +{ + std::pair v = std::minmax(1, 2); // { dg-warning "dangling reference" } + U v2 = std::minmax(1, 2); // { dg-warning "dangling reference" } + auto v3 = std::minmax(1, 2); // { dg-warning "dangling reference" } + return v.first + v2.second + v3.first; +} + +int +fn2 () +{ + int n = 1; + auto p = std::minmax(n, n + 1); // { dg-warning "dangling reference" } + int m = p.first; // ok + int x = p.second; // undefined behavior + + // Note that structured bindings have the same issue + auto [mm, xx] = std::minmax(n, n + 1); // { dg-warning "dangling reference" } + (void) xx; // undefined behavior + + return m + x; +} + +int +fn3 () +{ + auto v = std::minmax({1, 2}); + return v.first; +}