On Fri, Jan 26, 2024 at 04:04:35PM -0500, Jason Merrill wrote:
On 1/25/24 20:37, Marek Polacek wrote:
Bootstrapped/regtested on x86_64-pc-linux-gnu, 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. Such
an attribute should not be tied to this particular warning though. [*]
The warning bogusly triggers for classes that are like std::span,
std::reference_wrapper, and std::ranges::ref_view. The common property
seems to be that these classes are only wrappers around some data. So
I chose the name non_owning, but I'm not attached to it. I hope that
in the future the attribute can be used for something other than this
diagnostic.
You decided not to pursue Barry's request for a bool argument to the
attribute?
At first I thought it'd be an unnecessary complication but it was actually
pretty easy. Better to accept the optional argument from the get-go
otherwise people would have to add > GCC 14 checks.
Might it be more useful for the attribute to make reference_like_class_p
return true, so that we still warn about a temporary of another type passing
through it?
Good point. Fixed.
Bootstrapped/regtested on x86_64-pc-linux-gnu, 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. Such
an attribute should not be tied to this particular warning though.
The warning bogusly triggers for classes that are like std::span,
std::reference_wrapper, and std::ranges::ref_view. The common property
seems to be that these classes are only wrappers around some data. So
I chose the name non_owning, but I'm not attached to it. I hope that
in the future the attribute can be used for something other than this
diagnostic.
This attribute takes an optional bool argument to support cases like:
template <typename T>
struct [[gnu::non_owning(std::is_reference_v<T>)]] S {
// ...
};
PR c++/110358
PR c++/109642
gcc/cp/ChangeLog:
* call.cc (non_owning_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::non_owning.
* tree.cc (cxx_gnu_attributes): Add gnu::non_owning.
(handle_non_owning_attribute): New.
gcc/ChangeLog:
* doc/extend.texi: Document gnu::non_owning.
* doc/invoke.texi: Mention that gnu::non_owning disables
-Wdangling-reference.
gcc/testsuite/ChangeLog:
* g++.dg/ext/attr-non-owning1.C: New test.
* g++.dg/ext/attr-non-owning2.C: New test.
* g++.dg/ext/attr-non-owning3.C: New test.
* g++.dg/ext/attr-non-owning4.C: New test.
* g++.dg/ext/attr-non-owning5.C: New test.
* g++.dg/ext/attr-non-owning6.C: New test.
* g++.dg/ext/attr-non-owning7.C: New test.
* g++.dg/ext/attr-non-owning8.C: New test.
* g++.dg/ext/attr-non-owning9.C: New test.
---
gcc/cp/call.cc | 38 ++++++++++--
gcc/cp/tree.cc | 26 +++++++++
gcc/doc/extend.texi | 25 ++++++++
gcc/doc/invoke.texi | 21 +++++++
gcc/testsuite/g++.dg/ext/attr-non-owning1.C | 38 ++++++++++++
gcc/testsuite/g++.dg/ext/attr-non-owning2.C | 29 +++++++++
gcc/testsuite/g++.dg/ext/attr-non-owning3.C | 24 ++++++++
gcc/testsuite/g++.dg/ext/attr-non-owning4.C | 14 +++++
gcc/testsuite/g++.dg/ext/attr-non-owning5.C | 31 ++++++++++
gcc/testsuite/g++.dg/ext/attr-non-owning6.C | 65 +++++++++++++++++++++
gcc/testsuite/g++.dg/ext/attr-non-owning7.C | 31 ++++++++++
gcc/testsuite/g++.dg/ext/attr-non-owning8.C | 30 ++++++++++
gcc/testsuite/g++.dg/ext/attr-non-owning9.C | 25 ++++++++
13 files changed, 391 insertions(+), 6 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/ext/attr-non-owning1.C
create mode 100644 gcc/testsuite/g++.dg/ext/attr-non-owning2.C
create mode 100644 gcc/testsuite/g++.dg/ext/attr-non-owning3.C
create mode 100644 gcc/testsuite/g++.dg/ext/attr-non-owning4.C
create mode 100644 gcc/testsuite/g++.dg/ext/attr-non-owning5.C
create mode 100644 gcc/testsuite/g++.dg/ext/attr-non-owning6.C
create mode 100644 gcc/testsuite/g++.dg/ext/attr-non-owning7.C
create mode 100644 gcc/testsuite/g++.dg/ext/attr-non-owning8.C
create mode 100644 gcc/testsuite/g++.dg/ext/attr-non-owning9.C
diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
index 1dac1470d3b..e4bf9c963bd 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
+ non-owning. */
+
+static bool
+non_owning_p (tree t)
+{
+ t = lookup_attribute ("non_owning", 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 (non_owning_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))
+ || non_owning_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..017da7e294a 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_non_owning_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 },
+ { "non_owning", 0, 1, false, true, false, false,
+ handle_non_owning_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 "non_owning" attribute; arguments as in
+ struct attribute_spec.handler. */
+
+tree
+handle_non_owning_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))
+ {
+ 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 2135dfde9c8..9132add8267 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -29320,6 +29320,31 @@ Some_Class B __attribute__ ((init_priority (543)));
Note that the particular values of @var{priority} do not matter; only their
relative ordering.
+@cindex @code{non_owning} type attribute
+@item non_owning
+
+This attribute can be applied on a class type, function, or member
+function and indicates that it does not own its associated data. For
+example, classes like @code{std::span} or @code{std::reference_wrapper}
+are considered non-owning.
+
+@smallexample
+class [[gnu::non_owning]] S @{ @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::non_owning(std::is_reference_v<T>)]] S @{
+ @dots{}
+@};
+@end smallexample
+
+Currently, the only effect this attribute has is to suppress the
+@option{-Wdangling-reference} diagnostic.
+
@cindex @code{warn_unused} type attribute
@item warn_unused
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index b4e4ee9fb81..0df2e2ded34 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -3908,6 +3908,9 @@ const T& foo (const T&) @{ @dots{} @}
#pragma GCC diagnostic pop
@end smallexample
+The @code{#pragma} can also surround the class; in that case, the warning
+will be disabled for all the member functions.
+
@option{-Wdangling-reference} also warns about code like
@smallexample
@@ -3932,6 +3935,24 @@ 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::non_owning} attribute,
+which can be applied on the enclosing class type (in which case it disables
+the warning for all its member functions), member function, or a regular
+function. For example:
