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 [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 2789D3858D35 for ; Fri, 1 Mar 2024 21:23:46 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 2789D3858D35 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 2789D3858D35 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.129.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1709328228; cv=none; b=Mvrd8b77X5YsqSh4pjJubNsNcchQXjlqqthFcAFjZSNzGf2HjmWyE/+I5MCE+v/j59QqqQYd82R4xOcmR0S9Q0tzhnH7aDMqVzsxAA8jfS21vGZKxztW1Dya5+xft7lOHM4BpGORZ/jRDzyAAy4+r1Pd/btIlMV/YimTWrBJqKI= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1709328228; c=relaxed/simple; bh=JYr8UWHb+ovWUK2cIU3GPfoPSJ12XNfOqVNE7nviUP4=; h=DKIM-Signature:From:Date:To:Subject:Message-ID:MIME-Version; b=wZ1uz0k5q8aLTOtEqpGNS2r3XKa/eVylWheqyqRRinSmBKsKI3/V5Fxmg2jan9wrrO7XC3cjaofy/OAq5qt4jSMMOMgR4JBGe33ndA6mgIQjTgkSvfN3Q7xYEw7vQHBc1pRYWIM8Rp4MkEojkLr/cqVKoXmQ9j0yyaVMvuy9ElQ= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1709328225; 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: in-reply-to:in-reply-to:references:references; bh=3gQ7Z07aRxvjHsTAB4JCDJjkVMPI5GEx8rliByWp2Dc=; b=Zh16VI3x4pf7cNgShHSDvNUCDPF9FgWLJ3xXUeAWWZFMl0qK9N34CepnCjy0vpAcy7jaSr zTY4maT3Sx0wO4lAv4VhUDWQPu09EkW4V/Kmcf6QSp9p9rnwpOERHTDk03s8lRzwzrNpoL rextyn8nVeHM1C6CQVLJ6VnNQw9JWB4= Received: from mail-qk1-f198.google.com (mail-qk1-f198.google.com [209.85.222.198]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-511-N6ovNQq3MDKrEhJgi9MuRQ-1; Fri, 01 Mar 2024 16:23:44 -0500 X-MC-Unique: N6ovNQq3MDKrEhJgi9MuRQ-1 Received: by mail-qk1-f198.google.com with SMTP id af79cd13be357-7876cf4554eso285235285a.3 for ; Fri, 01 Mar 2024 13:23:43 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1709328222; x=1709933022; h=mime-version:references:message-id:in-reply-to:subject:cc:to:date :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=3gQ7Z07aRxvjHsTAB4JCDJjkVMPI5GEx8rliByWp2Dc=; b=P1PRvYrglwSJaOuqlrlHZ2fRgZENug4fV94mAaBdSCx4qynkfqsBOXoFRvw/DfLOgH ClxcdtcXQTO1C+YIEYnp2wC0N6F9AUrvTkR02+XakJqJASgDyeFDX8U+Tco6EqSKNuin ExyimlGRthkoYKWD6WvxjE0GoHgSZ3zrqACCIhpv8kSNy5BwGQA4jaoBI2mJDz4GUxkf JNcNdcyPROHJy0YBUcGwOQ3ponnlZuouqp1eRpGbLUfhtWnDop8SzT0rRkDbLavedH2t pE2pyDLWOtPRPcxA3j/0diQ4KutwLgDxoJ7apX976yqihtJSaGtdkzS1Adkgdxmj/8hd qz1w== X-Forwarded-Encrypted: i=1; AJvYcCWpLbs+jVSumxtZa07WMGXstjp1N86luXyTZTvLj6ouPsnnJ40lJIoKWG92ppn+7RNLBdODKjj45h5T1u1ZiG1NXbo8uUajGw== X-Gm-Message-State: AOJu0YyMCAZsAuuZLAa/3DDDKk3yxZ8VSaPSnNiUHbNkWL/THcnvg+mp 1RpXnBtMU4PVi7OqxpLG8SO7xiuKGRoBYQt1QLzIa/hEfj89BcTxhX1tO+UCEc9r72xXiMdcFhd GOjX6RibkPWW/jPbYSjvLDdYKa+UfrUnRaQRqqvOVp4YARPeU3t7uJqQp9jUKF4M= X-Received: by 2002:a05:620a:57d6:b0:785:d857:2190 with SMTP id wl22-20020a05620a57d600b00785d8572190mr3085917qkn.64.1709328222398; Fri, 01 Mar 2024 13:23:42 -0800 (PST) X-Google-Smtp-Source: AGHT+IFp8czZd0eCHULr/EfpYzXKiB3yVEERzvBhOUAqJrxaEzLHA4e6A6SP7QIHrWScNIbK5+xzvA== X-Received: by 2002:a05:620a:57d6:b0:785:d857:2190 with SMTP id wl22-20020a05620a57d600b00785d8572190mr3085903qkn.64.1709328221992; Fri, 01 Mar 2024 13:23:41 -0800 (PST) Received: from [192.168.1.130] (ool-457670bb.dyn.optonline.net. [69.118.112.187]) by smtp.gmail.com with ESMTPSA id a7-20020a05620a124700b00787bd9296c8sm1994019qkl.46.2024.03.01.13.23.40 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 01 Mar 2024 13:23:41 -0800 (PST) From: Patrick Palka X-Google-Original-From: Patrick Palka Date: Fri, 1 Mar 2024 16:23:40 -0500 (EST) To: Jason Merrill cc: Marek Polacek , GCC Patches Subject: Re: [PATCH v5] c++: implement [[gnu::non_owning]] [PR110358] In-Reply-To: <584f955c-14eb-40b2-9365-a02279c3a763@redhat.com> Message-ID: <370b3ed4-c671-91c5-653f-ea11d231db0c@idea> References: <20240126013736.70125-1-polacek@redhat.com> <0e4c47b6-604c-4d30-b458-825959c0e1d6@redhat.com> <584f955c-14eb-40b2-9365-a02279c3a763@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII X-Spam-Status: No, score=-13.8 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,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Fri, 1 Mar 2024, Jason Merrill wrote: > On 3/1/24 14:24, Marek Polacek wrote: > > On Fri, Mar 01, 2024 at 01:19:40PM -0500, Jason Merrill wrote: > > > On 3/1/24 12:39, Marek Polacek wrote: > > > > @option{-Wdangling-reference} also warns about code like > > > > @smallexample > > > > @@ -3932,6 +3935,10 @@ struct Span @{ > > > > as @code{std::span}-like; that is, the class is a non-union class > > > > that has a pointer data member and a trivial destructor. > > > > +The warning can be disabled by using the @code{gnu::no_dangling} > > > > attribute > > > > +on a function (@pxref{Common Function Attributes}), or a class type > > > > +(@pxref{C++ Attributes}). > > > > > > It seems surprising that one is in a generic attributes section and the > > > other in the C++-specific section. Maybe both uses could be covered in > > > the > > > C++ attributes section? > > > > Arg yes, definitely. Done here. > > > > > > This warning is enabled by @option{-Wall}. > > > > @opindex Wdelete-non-virtual-dtor > > > > diff --git a/gcc/testsuite/g++.dg/ext/attr-no-dangling1.C > > > > b/gcc/testsuite/g++.dg/ext/attr-no-dangling1.C > > > > new file mode 100644 > > > > index 00000000000..02eabbc5003 > > > > --- /dev/null > > > > +++ b/gcc/testsuite/g++.dg/ext/attr-no-dangling1.C > > > > @@ -0,0 +1,38 @@ > > > > +// { dg-do compile { target c++11 } } > > > > +// { dg-options "-Wdangling-reference" } > > > > + > > > > +int g = 42; > > > > + > > > > +struct [[gnu::no_dangling]] A { > > > > + int *i; > > > > + int &foo() { return *i; } > > > > +}; > > > > + > > > > +struct A2 { > > > > + int *i; > > > > + [[gnu::no_dangling]] int &foo() { return *i; } > > > > + [[gnu::no_dangling]] static int &bar (const int &) { return *&g; } > > > > +}; > > > > + > > > > +union [[gnu::no_dangling]] U { }; > > > > + > > > > +A a() { return A{&g}; } > > > > +A2 a2() { return A2{&g}; } > > > > + > > > > +class X { }; > > > > +const X x1; > > > > +const X x2; > > > > + > > > > +[[gnu::no_dangling]] const X& get(const int& i) > > > > +{ > > > > + return i == 0 ? x1 : x2; > > > > +} > > > > + > > > > +void > > > > +test () > > > > +{ > > > > + [[maybe_unused]] const X& x = get (10); // { dg-bogus > > > > "dangling" } > > > > + [[maybe_unused]] const int &i = a().foo(); // { dg-bogus > > > > "dangling" } > > > > + [[maybe_unused]] const int &j = a2().foo(); // { dg-bogus > > > > "dangling" } > > > > + [[maybe_unused]] const int &k = a2().bar(10); // { dg-bogus > > > > "dangling" } > > > > +} > > > > > > Do you want to add destructors to A/A2 like you did in other tests? > > > > Added. I think this test predates the recent heuristic. > > > > Ok for trunk? > > > > -- >8 -- > > Since -Wdangling-reference has false positives that can't be > > prevented, we should offer an easy way to suppress the warning. > > Currently, that is only possible by using a #pragma, either around the > > enclosing class or around the call site. But #pragma GCC diagnostic tend > > to be onerous. A better solution would be to have an attribute. > > > > To that end, this patch adds a new attribute, [[gnu::no_dangling]]. > > This attribute takes an optional bool argument to support cases like: > > > > template > > struct [[gnu::no_dangling(std::is_reference_v)]] S { > > // ... > > }; > > > > PR c++/110358 > > PR c++/109642 > > > > gcc/cp/ChangeLog: > > > > * call.cc (no_dangling_p): New. > > (reference_like_class_p): Use it. > > (do_warn_dangling_reference): Use it. Don't warn when the function > > or its enclosing class has attribute gnu::no_dangling. > > * tree.cc (cxx_gnu_attributes): Add gnu::no_dangling. > > (handle_no_dangling_attribute): New. > > > > gcc/ChangeLog: > > > > * doc/extend.texi: Document gnu::no_dangling. > > * doc/invoke.texi: Mention that gnu::no_dangling disables > > -Wdangling-reference. > > > > gcc/testsuite/ChangeLog: > > > > * g++.dg/ext/attr-no-dangling1.C: New test. > > * g++.dg/ext/attr-no-dangling2.C: New test. > > * g++.dg/ext/attr-no-dangling3.C: New test. > > * g++.dg/ext/attr-no-dangling4.C: New test. > > * g++.dg/ext/attr-no-dangling5.C: New test. > > * g++.dg/ext/attr-no-dangling6.C: New test. > > * g++.dg/ext/attr-no-dangling7.C: New test. > > * g++.dg/ext/attr-no-dangling8.C: New test. > > * g++.dg/ext/attr-no-dangling9.C: New test. > > --- > > gcc/cp/call.cc | 38 ++++++++++-- > > gcc/cp/tree.cc | 26 ++++++++ > > gcc/doc/extend.texi | 47 ++++++++++++++ > > gcc/doc/invoke.texi | 6 ++ > > gcc/testsuite/g++.dg/ext/attr-no-dangling1.C | 40 ++++++++++++ > > gcc/testsuite/g++.dg/ext/attr-no-dangling2.C | 29 +++++++++ > > gcc/testsuite/g++.dg/ext/attr-no-dangling3.C | 24 ++++++++ > > gcc/testsuite/g++.dg/ext/attr-no-dangling4.C | 14 +++++ > > gcc/testsuite/g++.dg/ext/attr-no-dangling5.C | 31 ++++++++++ > > gcc/testsuite/g++.dg/ext/attr-no-dangling6.C | 65 ++++++++++++++++++++ > > gcc/testsuite/g++.dg/ext/attr-no-dangling7.C | 31 ++++++++++ > > gcc/testsuite/g++.dg/ext/attr-no-dangling8.C | 30 +++++++++ > > gcc/testsuite/g++.dg/ext/attr-no-dangling9.C | 25 ++++++++ > > 13 files changed, 400 insertions(+), 6 deletions(-) > > create mode 100644 gcc/testsuite/g++.dg/ext/attr-no-dangling1.C > > create mode 100644 gcc/testsuite/g++.dg/ext/attr-no-dangling2.C > > create mode 100644 gcc/testsuite/g++.dg/ext/attr-no-dangling3.C > > create mode 100644 gcc/testsuite/g++.dg/ext/attr-no-dangling4.C > > create mode 100644 gcc/testsuite/g++.dg/ext/attr-no-dangling5.C > > create mode 100644 gcc/testsuite/g++.dg/ext/attr-no-dangling6.C > > create mode 100644 gcc/testsuite/g++.dg/ext/attr-no-dangling7.C > > create mode 100644 gcc/testsuite/g++.dg/ext/attr-no-dangling8.C > > create mode 100644 gcc/testsuite/g++.dg/ext/attr-no-dangling9.C > > > > diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc > > index c40ef2e3028..9e4c8073600 100644 > > --- a/gcc/cp/call.cc > > +++ b/gcc/cp/call.cc > > @@ -14033,11 +14033,7 @@ std_pair_ref_ref_p (tree t) > > return true; > > } > > -/* Return true if a class CTYPE is either std::reference_wrapper or > > - std::ref_view, or a reference wrapper class. We consider a class > > - a reference wrapper class if it has a reference member. We no > > - longer check that it has a constructor taking the same reference type > > - since that approach still generated too many false positives. */ > > +/* Return true if a class T has a reference member. */ > > static bool > > class_has_reference_member_p (tree t) > > @@ -14061,12 +14057,41 @@ class_has_reference_member_p_r (tree binfo, void > > *) > > ? integer_one_node : NULL_TREE); > > } > > + > > +/* Return true if T (either a class or a function) has been marked as > > + not-dangling. */ > > + > > +static bool > > +no_dangling_p (tree t) > > +{ > > + t = lookup_attribute ("no_dangling", TYPE_ATTRIBUTES (t)); > > + if (!t) > > + return false; > > + > > + t = TREE_VALUE (t); > > + if (!t) > > + return true; > > + > > + t = build_converted_constant_bool_expr (TREE_VALUE (t), > > tf_warning_or_error); > > + t = cxx_constant_value (t); > > + return t == boolean_true_node; > > +} > > + > > +/* Return true if a class CTYPE is either std::reference_wrapper or > > + std::ref_view, or a reference wrapper class. We consider a class > > + a reference wrapper class if it has a reference member. We no > > + longer check that it has a constructor taking the same reference type > > + since that approach still generated too many false positives. */ > > + > > static bool > > reference_like_class_p (tree ctype) > > { > > if (!CLASS_TYPE_P (ctype)) > > return false; > > + if (no_dangling_p (ctype)) > > + return true; > > + > > /* Also accept a std::pair. */ > > if (std_pair_ref_ref_p (ctype)) > > return true; > > @@ -14173,7 +14198,8 @@ do_warn_dangling_reference (tree expr, bool arg_p) > > but probably not to one of its arguments. */ > > || (DECL_OBJECT_MEMBER_FUNCTION_P (fndecl) > > && DECL_OVERLOADED_OPERATOR_P (fndecl) > > - && DECL_OVERLOADED_OPERATOR_IS (fndecl, INDIRECT_REF))) > > + && DECL_OVERLOADED_OPERATOR_IS (fndecl, INDIRECT_REF)) > > + || no_dangling_p (TREE_TYPE (fndecl))) > > return NULL_TREE; > > tree rettype = TREE_TYPE (TREE_TYPE (fndecl)); > > diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc > > index ad312710f68..e75be9a4e66 100644 > > --- a/gcc/cp/tree.cc > > +++ b/gcc/cp/tree.cc > > @@ -47,6 +47,7 @@ static tree verify_stmt_tree_r (tree *, int *, void *); > > static tree handle_init_priority_attribute (tree *, tree, tree, int, bool > > *); > > static tree handle_abi_tag_attribute (tree *, tree, tree, int, bool *); > > static tree handle_contract_attribute (tree *, tree, tree, int, bool *); > > +static tree handle_no_dangling_attribute (tree *, tree, tree, int, bool *); > > /* If REF is an lvalue, returns the kind of lvalue that REF is. > > Otherwise, returns clk_none. */ > > @@ -5102,6 +5103,8 @@ static const attribute_spec cxx_gnu_attributes[] = > > handle_init_priority_attribute, NULL }, > > { "abi_tag", 1, -1, false, false, false, true, > > handle_abi_tag_attribute, NULL }, > > + { "no_dangling", 0, 1, false, true, false, false, > > + handle_no_dangling_attribute, NULL }, > > }; > > const scoped_attribute_specs cxx_gnu_attribute_table = > > @@ -5391,6 +5394,29 @@ handle_contract_attribute (tree *ARG_UNUSED (node), > > tree ARG_UNUSED (name), > > return NULL_TREE; > > } > > +/* Handle a "no_dangling" attribute; arguments as in > > + struct attribute_spec.handler. */ > > + > > +tree > > +handle_no_dangling_attribute (tree *node, tree name, tree args, int, > > + bool *no_add_attrs) > > +{ > > + if (args && TREE_CODE (TREE_VALUE (args)) == STRING_CST) > > + { > > + error ("%qE attribute argument must be an expression that evaluates " > > + "to true or false", name); > > + *no_add_attrs = true; > > + } > > + else if (!FUNC_OR_METHOD_TYPE_P (*node) > > + && !RECORD_OR_UNION_TYPE_P (*node)) Sorry for not asking this sooner, but does it matter whether we attach the attribute to the function type rather than the function declaration? I noticed e.g. nodiscard gets attached to the decl. > > + { > > + warning (OPT_Wattributes, "%qE attribute ignored", name); > > + *no_add_attrs = true; > > + } > > + > > + return NULL_TREE; > > +} > > + > > /* Return a new PTRMEM_CST of the indicated TYPE. The MEMBER is the > > thing pointed to by the constant. */ > > diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi > > index 6c2c7ae5d8a..8e1751eae6c 100644 > > --- a/gcc/doc/extend.texi > > +++ b/gcc/doc/extend.texi > > @@ -29327,6 +29327,53 @@ Some_Class B __attribute__ ((init_priority > > (543))); > > Note that the particular values of @var{priority} do not matter; only > > their > > relative ordering. > > +@cindex @code{no_dangling} type attribute > > +@cindex @code{no_dangling} function attribute And we document it as a function attribute despite attaching it to the function type. > > +@item no_dangling > > + > > +This attribute can be applied on a class type, function, or member > > +function. Dangling references to classes marked with this attribute > > +will have the @option{-Wdangling-reference} diagnostic suppressed; so > > +will the @code{gnu::no_dangling}-marked functions. For example: > > ...; so will references returned from... > > > +@smallexample > > +class [[gnu::no_dangling]] S @{ @dots{} @}; > > +@end smallexample > > + > > +Or: > > + > > +@smallexample > > +class A @{ > > + int *p; > > + [[gnu::no_dangling]] int &foo() @{ return *p; @} > > +@}; > > + > > +[[gnu::no_dangling]] const int & > > +foo (const int &i) > > +@{ > > + @dots{} > > +@} > > +@end smallexample > > + > > +This attribute takes an optional argument, which must be an expression that > > +evaluates to true or false: > > + > > +@smallexample > > +template > > +struct [[gnu::no_dangling(std::is_reference_v)]] S @{ > > + @dots{} > > +@}; > > +@end smallexample > > + > > +Or: > > + > > +@smallexample > > +template > > +[[gnu::no_dangling(std::is_reference_v)]] int foo (T& t) @{ > > I think this function should return a reference. > > OK with those changes, thanks. > > Jason > >