diff mbox series

c++: implement [[gnu::non_owning]] [PR110358]

Message ID 20240126013736.70125-1-polacek@redhat.com
State New
Headers show
Series c++: implement [[gnu::non_owning]] [PR110358] | expand

Commit Message

Marek Polacek Jan. 26, 2024, 1:37 a.m. UTC
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.

[*] As I'm typing this, it's occurring to me that we might consider
having a general attribute allowing users to do [[gnu::ignore("-Wfoo")]].

	PR c++/110358
	PR c++/109642

gcc/cp/ChangeLog:

	* call.cc (do_warn_dangling_reference): 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.
---
 gcc/cp/call.cc                              |  9 ++++-
 gcc/cp/tree.cc                              | 20 +++++++++++
 gcc/doc/extend.texi                         | 15 ++++++++
 gcc/doc/invoke.texi                         | 21 ++++++++++++
 gcc/testsuite/g++.dg/ext/attr-non-owning1.C | 38 +++++++++++++++++++++
 gcc/testsuite/g++.dg/ext/attr-non-owning2.C | 28 +++++++++++++++
 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 | 29 ++++++++++++++++
 9 files changed, 197 insertions(+), 1 deletion(-)
 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


base-commit: fd620bd3351c6b9821c299035ed17e655d7954b5

Comments

Marek Polacek Jan. 26, 2024, 2:34 a.m. UTC | #1
On Thu, Jan 25, 2024 at 08:37:36PM -0500, Marek Polacek wrote:
> +/* 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)

I overlooked error: unused parameter 'args'.  Fixed.

Marek
Jason Merrill Jan. 26, 2024, 9:04 p.m. UTC | #2
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?

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?

Jason
diff mbox series

Patch

diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
index 9de0d77c423..88ddba825a9 100644
--- a/gcc/cp/call.cc
+++ b/gcc/cp/call.cc
@@ -14157,9 +14157,16 @@  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))
+	    || lookup_attribute ("non_owning",
+				 TYPE_ATTRIBUTES (TREE_TYPE (fndecl))))
 	  return NULL_TREE;
 
+	if (tree ctx = CP_DECL_CONTEXT (fndecl))
+	  if (TYPE_P (ctx)
+	      && lookup_attribute ("non_owning", TYPE_ATTRIBUTES (ctx)))
+	    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.
diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc
index 77f57e0f9ac..2adf59b22d4 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.  */
@@ -5096,6 +5097,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, 0, false, true, false, false,
+    handle_non_owning_attribute, NULL },
 };
 
 const scoped_attribute_specs cxx_gnu_attribute_table =
@@ -5385,6 +5388,23 @@  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 (!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 18c485c11b0..09e68cc8759 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -29299,6 +29299,21 @@  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
+
+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 6ec56493e59..9986c1add39 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -3906,6 +3906,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
@@ -3916,6 +3919,24 @@  where @code{std::minmax} returns @code{std::pair<const int&, const int&>}, and
 both references dangle after the end of the full expression that contains
 the call to @code{std::minmax}.
 
+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:
+
+@smallexample
+class [[gnu::non_owning]] A @{
+  int *p;
+  int &foo() @{ return *p; @}
+@};
+
+[[gnu::non_owning]] const int &
+foo (const int &i)
+@{
+  @dots{}
+@}
+@end smallexample
+
 This warning is enabled by @option{-Wall}.
 
 @opindex Wdelete-non-virtual-dtor
diff --git a/gcc/testsuite/g++.dg/ext/attr-non-owning1.C b/gcc/testsuite/g++.dg/ext/attr-non-owning1.C
new file mode 100644
index 00000000000..000f7f4019d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ext/attr-non-owning1.C
@@ -0,0 +1,38 @@ 
+// { dg-do compile { target c++11 } }
+// { dg-options "-Wdangling-reference" }
+
+int g = 42;
+
+struct [[gnu::non_owning]] A {
+  int *i;
+  int &foo() { return *i; }
+};
+
+struct A2 {
+  int *i;
+  [[gnu::non_owning]] int &foo() { return *i; }
+  [[gnu::non_owning]] static int &bar (const int &) { return *&g; }
+};
+
+union [[gnu::non_owning]] U { };
+
+A a() { return A{&g}; }
+A2 a2() { return A2{&g}; }
+
+class X { };
+const X x1;
+const X x2;
+
+[[gnu::non_owning]] 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" }
+}
diff --git a/gcc/testsuite/g++.dg/ext/attr-non-owning2.C b/gcc/testsuite/g++.dg/ext/attr-non-owning2.C
new file mode 100644
index 00000000000..6f1d649736a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ext/attr-non-owning2.C
@@ -0,0 +1,28 @@ 
+// { dg-do compile { target c++11 } }
+// Negative tests.
+
+struct [[non_owning]] A {	    // { dg-warning "ignored" }
+ [[non_owning]] int &foo (int &);   // { dg-warning "ignored" }
+};
+
+[[non_owning]] int &bar (int &);    // { dg-warning "ignored" }
+
+[[gnu::non_owning]] int i;	    // { dg-warning "ignored" }
+[[gnu::non_owning]] double d;	    // { dg-warning "ignored" }
+[[gnu::non_owning]] typedef int T;  // { dg-warning "ignored" }
+
+[[gnu::non_owning()]] int &fn1 (int &);	    // { dg-error "attribute does not take any arguments" }
+[[gnu::non_owning("a")]] int &fn2 (int &);  // { dg-error "attribute does not take any arguments" }
+
+enum [[gnu::non_owning]] E {	    // { dg-warning "ignored" }
+  X [[gnu::non_owning]]		    // { dg-warning "ignored" }
+};
+
+[[gnu::non_owning]];		    // { dg-warning "ignored" }
+
+void
+g ()
+{
+  goto L;
+[[gnu::non_owning]] L:;		    // { dg-warning "ignored" }
+}
diff --git a/gcc/testsuite/g++.dg/ext/attr-non-owning3.C b/gcc/testsuite/g++.dg/ext/attr-non-owning3.C
new file mode 100644
index 00000000000..81c6491643b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ext/attr-non-owning3.C
@@ -0,0 +1,24 @@ 
+// { dg-do compile { target c++11 } }
+// { dg-options "-Wdangling-reference" }
+
+template <typename T>
+struct [[gnu::non_owning]] Span {
+    T* data_;
+    int len_;
+    // So that our heuristic doesn't suppress the warning anyway.
+    ~Span();
+
+    [[nodiscard]] constexpr auto operator[](int n) const noexcept -> T& { return data_[n]; }
+    [[nodiscard]] constexpr auto front() const noexcept -> T& { return data_[0]; }
+    [[nodiscard]] constexpr auto back() const noexcept -> T& { return data_[len_ - 1]; }
+};
+
+auto get() -> Span<int>;
+
+auto f() -> int {
+    int const& a = get().front(); // { dg-bogus "dangling" }
+    int const& b = get().back();  // { dg-bogus "dangling" }
+    int const& c = get()[0];      // { dg-bogus "dangling" }
+
+    return a + b + c;
+}
diff --git a/gcc/testsuite/g++.dg/ext/attr-non-owning4.C b/gcc/testsuite/g++.dg/ext/attr-non-owning4.C
new file mode 100644
index 00000000000..c50382c3e8f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ext/attr-non-owning4.C
@@ -0,0 +1,14 @@ 
+// { dg-do compile { target c++11 } }
+
+#if !__has_attribute(non_owning)
+#error unsupported
+#endif
+
+#ifdef __has_cpp_attribute
+# if !__has_cpp_attribute(non_owning)
+#  error non_owning
+# endif
+#endif
+
+struct [[gnu::non_owning]] S { };
+static_assert (__builtin_has_attribute (S, non_owning), "");
diff --git a/gcc/testsuite/g++.dg/ext/attr-non-owning5.C b/gcc/testsuite/g++.dg/ext/attr-non-owning5.C
new file mode 100644
index 00000000000..6122d583ccc
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ext/attr-non-owning5.C
@@ -0,0 +1,29 @@ 
+// PR c++/110358
+// { dg-do compile { target c++20 } }
+// { dg-options "-Wdangling-reference" }
+
+template <typename T>
+struct Span {
+    T* data_;
+    int len_;
+
+    [[nodiscard]] constexpr auto operator[](int n) const noexcept -> T& { return data_[n]; }
+};
+
+template <>
+struct [[gnu::non_owning]] Span<int> {
+    int* data_;
+    int len_;
+
+    [[nodiscard]] constexpr auto operator[](int n) const noexcept -> int& { return data_[n]; }
+};
+
+auto getch() -> Span<char>;
+auto geti() -> Span<int>;
+
+void
+f ()
+{
+  [[maybe_unused]] const auto &a = getch()[0];	// { dg-warning "dangling reference" }
+  [[maybe_unused]] const auto &b = geti()[0];   // { dg-bogus "dangling reference" }
+}