From patchwork Wed Jan 18 17:52:00 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marek Polacek X-Patchwork-Id: 63361 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 11D3D38582A1 for ; Wed, 18 Jan 2023 17:52:36 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 11D3D38582A1 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1674064356; bh=cWot3PvHeD+YNXccJN5A8fpvlesJ7tfz//uzs2a+ww8=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=uSZGpoST2zk2BvTVRYaAjQtvB7DRKF6PDLdveA+tgUfJJxbNx/bs3EMX7TGNRs5S3 gUWQKCluTwxK4jDZVdEgmMte2w5RvEyCY0O6jQYZNRSipXPA0qHRc2pYw9254nW6Ek dqJPbg1PlmTjzXD2tF2NMfxCMQj0UvkyayznrQ6g= 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.129.124]) by sourceware.org (Postfix) with ESMTPS id D748D3858D28 for ; Wed, 18 Jan 2023 17:52:07 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org D748D3858D28 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-477-_dHiR7zzPRioDr8SBFvvIw-1; Wed, 18 Jan 2023 12:52:06 -0500 X-MC-Unique: _dHiR7zzPRioDr8SBFvvIw-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id DA5C718E004B for ; Wed, 18 Jan 2023 17:52:05 +0000 (UTC) Received: from pdp-11.lan (unknown [10.22.10.46]) by smtp.corp.redhat.com (Postfix) with ESMTP id B4D691121315; Wed, 18 Jan 2023 17:52:05 +0000 (UTC) To: Jason Merrill , GCC Patches Subject: [PATCH] c++: -Wdangling-reference with reference wrapper [PR107532] Date: Wed, 18 Jan 2023 12:52:00 -0500 Message-Id: <20230118175200.365397-1-polacek@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.3 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+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" Here, -Wdangling-reference triggers where it probably shouldn't, causing some grief. The code in question uses a reference wrapper with a member function returning a reference to a subobject of a non-temporary object: const Plane & meta = fm.planes().inner(); I've tried a few approaches, e.g., checking that the member function's return type is the same as the type of the enclosing class (which is the case for member functions returning *this), but that then breaks Wdangling-reference4.C with std::optional. So I figured that perhaps we want to look at the object we're invoking the member function(s) on and see if that is a temporary, as in, don't warn about const Plane & meta = fm.planes().inner(); but do warn about const Plane & meta = FrameMetadata().planes().inner(); It's ugly, but better than asking users to add #pragmas into their code. Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? PR c++/107532 gcc/cp/ChangeLog: * call.cc (do_warn_dangling_reference): Don't warn when the object member functions are invoked on is not a temporary. gcc/testsuite/ChangeLog: * g++.dg/warn/Wdangling-reference8.C: New test. --- gcc/cp/call.cc | 33 +++++++- .../g++.dg/warn/Wdangling-reference8.C | 77 +++++++++++++++++++ 2 files changed, 109 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/g++.dg/warn/Wdangling-reference8.C base-commit: c6a011119bfa038ccbfc9f123ede14a3d6237fab diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc index 0780b5840a3..43e65c3dffb 100644 --- a/gcc/cp/call.cc +++ b/gcc/cp/call.cc @@ -13850,7 +13850,38 @@ do_warn_dangling_reference (tree expr) if (TREE_CODE (arg) == ADDR_EXPR) arg = TREE_OPERAND (arg, 0); if (expr_represents_temporary_p (arg)) - return expr; + { + /* An ugly attempt to reduce the number of -Wdangling-reference + false positives concerning reference wrappers (c++/107532). + Don't warn about s.a().b() but do warn about S().a().b(), + supposing that the member function is returning a reference + to a subobject of the (non-temporary) object. */ + if (DECL_NONSTATIC_MEMBER_FUNCTION_P (fndecl) + && !DECL_OVERLOADED_OPERATOR_P (fndecl) + && i == 0) + { + tree t = arg; + while (handled_component_p (t)) + t = TREE_OPERAND (t, 0); + t = TARGET_EXPR_INITIAL (arg); + /* Quite likely we don't have a chain of member functions + (like a().b().c()). */ + if (TREE_CODE (t) != CALL_EXPR) + return expr; + /* Walk the call chain to the original object and see if + it was a temporary. */ + do + t = tree_strip_nop_conversions (CALL_EXPR_ARG (t, 0)); + while (TREE_CODE (t) == CALL_EXPR); + /* If the object argument is &TARGET_EXPR<>, we've started + off the chain with a temporary and we want to warn. */ + if (TREE_CODE (t) == ADDR_EXPR) + t = TREE_OPERAND (t, 0); + if (!expr_represents_temporary_p (t)) + break; + } + return expr; + } /* Don't warn about member function like: std::any a(...); S& s = a.emplace({0}, 0); diff --git a/gcc/testsuite/g++.dg/warn/Wdangling-reference8.C b/gcc/testsuite/g++.dg/warn/Wdangling-reference8.C new file mode 100644 index 00000000000..32280f3e282 --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Wdangling-reference8.C @@ -0,0 +1,77 @@ +// PR c++/107532 +// { dg-do compile { target c++11 } } +// { dg-options "-Wdangling-reference" } + +struct Plane { unsigned int bytesused; }; + +// Passes a reference through. Does not change lifetime. +template +struct Ref { + const T& i_; + Ref(const T & i) : i_(i) {} + const T & inner(); +}; + +struct FrameMetadata { + Ref planes() const { return p_; } + + Plane p_; +}; + +void bar(const Plane & meta); +void foo(const FrameMetadata & fm) +{ + const Plane & meta = fm.planes().inner(); + bar(meta); + const Plane & meta2 = FrameMetadata().planes().inner(); // { dg-warning "dangling reference" } + bar(meta2); +} + +struct S { + const S& self () { return *this; } +} s; + +const S& r1 = s.self(); +const S& r2 = S().self(); // { dg-warning "dangling reference" } + +struct D { +}; + +struct C { + D d; + Ref get() const { return d; } +}; + +struct B { + C c; + const C& get() const { return c; } + B(); +}; + +struct A { + B b; + const B& get() const { return b; } +}; + +void +g (const A& a) +{ + const auto& d1 = a.get().get().get().inner(); + (void) d1; + const auto& d2 = A().get().get().get().inner(); // { dg-warning "dangling reference" } + (void) d2; + const auto& d3 = A().b.get().get().inner(); // { dg-warning "dangling reference" } + (void) d3; + const auto& d4 = a.b.get().get().inner(); + (void) d4; + const auto& d5 = a.b.c.get().inner(); + (void) d5; + const auto& d6 = A().b.c.get().inner(); // { dg-warning "dangling reference" } + (void) d6; + Plane p; + Ref r(p); + const auto& d7 = r.inner(); + (void) d7; + const auto& d8 = Ref(p).inner(); // { dg-warning "dangling reference" } + (void) d8; +}