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 <typename T>
> >    struct [[gnu::no_dangling(std::is_reference_v<T>)]] 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<const T&, const T&>.  */
> >     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 <typename T>
> > +struct [[gnu::no_dangling(std::is_reference_v<T>)]] S @{
> > +  @dots{}
> > +@};
> > +@end smallexample
> > +
> > +Or:
> > +
> > +@smallexample
> > +template <typename T>
> > +[[gnu::no_dangling(std::is_reference_v<T>)]] int foo (T& t) @{
> 
> I think this function should return a reference.
> 
> OK with those changes, thanks.
> 
> Jason
> 
> 

Reply via email to