* [PATCH] c++: avoid -Wdangling-reference for std::span-like classes [PR110358]
@ 2024-01-22 23:13 Marek Polacek
2024-01-26 1:36 ` [PATCH v2] " Marek Polacek
0 siblings, 1 reply; 11+ messages in thread
From: Marek Polacek @ 2024-01-22 23:13 UTC (permalink / raw)
To: Jason Merrill, GCC Patches
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
-- >8 --
Real-world experience shows that -Wdangling-reference triggers for
user-defined std::span-like classes a lot. We can easily avoid that
by considering classes like
template<typename T>
struct Span {
T* data_;
std::size len_;
};
to be std::span-like, and not warning for them.
PR c++/110358
PR c++/109640
gcc/cp/ChangeLog:
* call.cc (span_like_class_p): New.
(do_warn_dangling_reference): Use it.
gcc/ChangeLog:
* doc/invoke.texi: Update -Wdangling-reference documentation.
gcc/testsuite/ChangeLog:
* g++.dg/warn/Wdangling-reference18.C: New test.
* g++.dg/warn/Wdangling-reference19.C: New test.
* g++.dg/warn/Wdangling-reference20.C: New test.
---
gcc/cp/call.cc | 38 ++++++++++++++++-
gcc/doc/invoke.texi | 15 +++++++
.../g++.dg/warn/Wdangling-reference18.C | 24 +++++++++++
.../g++.dg/warn/Wdangling-reference19.C | 25 +++++++++++
.../g++.dg/warn/Wdangling-reference20.C | 42 +++++++++++++++++++
5 files changed, 143 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/g++.dg/warn/Wdangling-reference18.C
create mode 100644 gcc/testsuite/g++.dg/warn/Wdangling-reference19.C
create mode 100644 gcc/testsuite/g++.dg/warn/Wdangling-reference20.C
diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
index 77f51bacce3..d6bdb3cc9bd 100644
--- a/gcc/cp/call.cc
+++ b/gcc/cp/call.cc
@@ -14082,6 +14082,40 @@ reference_like_class_p (tree ctype)
return false;
}
+/* Return true if class TYPE looks like std::span: it's a class template
+ and has a T* member followed by a field of integral type. For example,
+
+ template<typename T>
+ struct Span {
+ T* data_;
+ std::size len_;
+ };
+
+ is considered std::span-like. */
+
+static bool
+span_like_class_p (tree type)
+{
+ if (!NON_UNION_CLASS_TYPE_P (type)
+ || !CLASSTYPE_TEMPLATE_INSTANTIATION (type))
+ return false;
+
+ tree args = CLASSTYPE_TI_ARGS (type);
+ if (TREE_VEC_LENGTH (args) != 1)
+ return false;
+
+ tree f = next_aggregate_field (TYPE_FIELDS (type));
+ if (f && TYPE_PTR_P (TREE_TYPE (f)))
+ {
+ f = next_aggregate_field (DECL_CHAIN (f));
+ if (f && INTEGRAL_TYPE_P (TREE_TYPE (f))
+ && !next_aggregate_field (DECL_CHAIN (f)))
+ return true;
+ }
+
+ return false;
+}
+
/* Helper for maybe_warn_dangling_reference to find a problematic CALL_EXPR
that initializes the LHS (and at least one of its arguments represents
a temporary, as outlined in maybe_warn_dangling_reference), or NULL_TREE
@@ -14126,7 +14160,9 @@ do_warn_dangling_reference (tree expr, bool arg_p)
tree type = TREE_TYPE (e);
/* If the temporary represents a lambda, we don't really know
what's going on here. */
- if (!reference_like_class_p (type) && !LAMBDA_TYPE_P (type))
+ if (!reference_like_class_p (type)
+ && !LAMBDA_TYPE_P (type)
+ && !span_like_class_p (type))
return expr;
}
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 278c931b6a3..509779c8fd8 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -3914,6 +3914,21 @@ 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 does not warn for @code{std::span}-like classes. We consider
+classes of the form:
+
+@smallexample
+template<typename T>
+struct Span @{
+ T* data_;
+ std::size len_;
+@};
+@end smallexample
+
+as @code{std::span}-like; that is, the class is a class template that
+has a pointer data member followed by an integral data member, and does
+not have any other data members.
+
This warning is enabled by @option{-Wall}.
@opindex Wdelete-non-virtual-dtor
diff --git a/gcc/testsuite/g++.dg/warn/Wdangling-reference18.C b/gcc/testsuite/g++.dg/warn/Wdangling-reference18.C
new file mode 100644
index 00000000000..e088c177769
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wdangling-reference18.C
@@ -0,0 +1,24 @@
+// PR c++/110358
+// { dg-do compile { target c++11 } }
+// { dg-options "-Wdangling-reference" }
+// Don't warn for std::span-like classes.
+
+template <typename T>
+struct Span {
+ T* data_;
+ int len_;
+
+ [[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 reference" }
+ int const& b = get().back(); // { dg-bogus "dangling reference" }
+ int const& c = get()[0]; // { dg-bogus "dangling reference" }
+
+ return a + b + c;
+}
diff --git a/gcc/testsuite/g++.dg/warn/Wdangling-reference19.C b/gcc/testsuite/g++.dg/warn/Wdangling-reference19.C
new file mode 100644
index 00000000000..3f74ab701c1
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wdangling-reference19.C
@@ -0,0 +1,25 @@
+// PR c++/110358
+// { dg-do compile { target c++11 } }
+// { dg-options "-Wdangling-reference" }
+// Like Wdangling-reference18.C but not actually a span-like class.
+
+template <typename T>
+struct Span {
+ T* data_;
+ int len_;
+ T foo_;
+
+ [[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-warning "dangling reference" }
+ int const& b = get().back(); // { dg-warning "dangling reference" }
+ int const& c = get()[0]; // { dg-warning "dangling reference" }
+
+ return a + b + c;
+}
diff --git a/gcc/testsuite/g++.dg/warn/Wdangling-reference20.C b/gcc/testsuite/g++.dg/warn/Wdangling-reference20.C
new file mode 100644
index 00000000000..463c7380283
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wdangling-reference20.C
@@ -0,0 +1,42 @@
+// PR c++/109640
+// { dg-do compile { target c++20 } }
+// { dg-options "-Wdangling-reference" }
+// Don't warn for std::span-like classes.
+
+#include <iterator>
+#include <span>
+
+template <typename T>
+struct MySpan
+{
+ MySpan(T* data, std::size_t size) :
+ data_(data),
+ size_(size)
+ {}
+
+ T& operator[](std::size_t idx) { return data_[idx]; }
+
+private:
+ T* data_;
+ std::size_t size_;
+};
+
+template <typename T, std::size_t n>
+MySpan<T const> make_my_span(T const(&x)[n])
+{
+ return MySpan(std::begin(x), n);
+}
+
+template <typename T, std::size_t n>
+std::span<T const> make_span(T const(&x)[n])
+{
+ return std::span(std::begin(x), n);
+}
+
+int main()
+{
+ int x[10]{};
+ int const& y{make_my_span(x)[0]};
+ int const& y2{make_span(x)[0]};
+ (void) y, (void) y2;
+}
base-commit: c596ce03120cc22e141186401c6656009ddebdaa
prerequisite-patch-id: 5929438e96b89b465c26c2fbd5b92d2444d1901d
--
2.43.0
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] c++: avoid -Wdangling-reference for std::span-like classes [PR110358]
2024-01-22 23:13 [PATCH] c++: avoid -Wdangling-reference for std::span-like classes [PR110358] Marek Polacek
@ 2024-01-26 1:36 ` Marek Polacek
2024-01-26 3:13 ` Jason Merrill
0 siblings, 1 reply; 11+ messages in thread
From: Marek Polacek @ 2024-01-26 1:36 UTC (permalink / raw)
To: Jason Merrill, GCC Patches
Better version:
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
-- >8 --
Real-world experience shows that -Wdangling-reference triggers for
user-defined std::span-like classes a lot. We can easily avoid that
by considering classes like
template<typename T>
struct Span {
T* data_;
std::size len_;
};
to be std::span-like, and not warning for them. Unlike the previous
patch, this one considers a non-union class template that has a pointer
data member and a trivial destructor as std::span-like.
PR c++/110358
PR c++/109640
gcc/cp/ChangeLog:
* call.cc (reference_like_class_p): Don't warn for std::span-like
classes.
gcc/ChangeLog:
* doc/invoke.texi: Update -Wdangling-reference description.
gcc/testsuite/ChangeLog:
* g++.dg/warn/Wdangling-reference18.C: New test.
* g++.dg/warn/Wdangling-reference19.C: New test.
* g++.dg/warn/Wdangling-reference20.C: New test.
---
gcc/cp/call.cc | 18 ++++++++
gcc/doc/invoke.texi | 14 +++++++
.../g++.dg/warn/Wdangling-reference18.C | 24 +++++++++++
.../g++.dg/warn/Wdangling-reference19.C | 25 +++++++++++
.../g++.dg/warn/Wdangling-reference20.C | 42 +++++++++++++++++++
5 files changed, 123 insertions(+)
create mode 100644 gcc/testsuite/g++.dg/warn/Wdangling-reference18.C
create mode 100644 gcc/testsuite/g++.dg/warn/Wdangling-reference19.C
create mode 100644 gcc/testsuite/g++.dg/warn/Wdangling-reference20.C
diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
index 9de0d77c423..afd3e1ff024 100644
--- a/gcc/cp/call.cc
+++ b/gcc/cp/call.cc
@@ -14082,6 +14082,24 @@ reference_like_class_p (tree ctype)
return true;
}
+ /* Avoid warning if CTYPE looks like std::span: it's a class template,
+ has a T* member, and a trivial destructor. For example,
+
+ template<typename T>
+ struct Span {
+ T* data_;
+ std::size len_;
+ };
+
+ is considered std::span-like. */
+ if (NON_UNION_CLASS_TYPE_P (ctype)
+ && CLASSTYPE_TEMPLATE_INSTANTIATION (ctype)
+ && TYPE_HAS_TRIVIAL_DESTRUCTOR (ctype))
+ for (tree field = next_aggregate_field (TYPE_FIELDS (ctype));
+ field; field = next_aggregate_field (DECL_CHAIN (field)))
+ if (TYPE_PTR_P (TREE_TYPE (field)))
+ return true;
+
/* Some classes, such as std::tuple, have the reference member in its
(non-direct) base class. */
if (dfs_walk_once (TYPE_BINFO (ctype), class_has_reference_member_p_r,
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 6ec56493e59..e0ff18a86f5 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -3916,6 +3916,20 @@ 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 does not warn for @code{std::span}-like classes. We consider
+classes of the form:
+
+@smallexample
+template<typename T>
+struct Span @{
+ T* data_;
+ std::size len_;
+@};
+@end smallexample
+
+as @code{std::span}-like; that is, the class is a non-union class template
+that has a pointer data member and a trivial destructor.
+
This warning is enabled by @option{-Wall}.
@opindex Wdelete-non-virtual-dtor
diff --git a/gcc/testsuite/g++.dg/warn/Wdangling-reference18.C b/gcc/testsuite/g++.dg/warn/Wdangling-reference18.C
new file mode 100644
index 00000000000..e088c177769
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wdangling-reference18.C
@@ -0,0 +1,24 @@
+// PR c++/110358
+// { dg-do compile { target c++11 } }
+// { dg-options "-Wdangling-reference" }
+// Don't warn for std::span-like classes.
+
+template <typename T>
+struct Span {
+ T* data_;
+ int len_;
+
+ [[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 reference" }
+ int const& b = get().back(); // { dg-bogus "dangling reference" }
+ int const& c = get()[0]; // { dg-bogus "dangling reference" }
+
+ return a + b + c;
+}
diff --git a/gcc/testsuite/g++.dg/warn/Wdangling-reference19.C b/gcc/testsuite/g++.dg/warn/Wdangling-reference19.C
new file mode 100644
index 00000000000..053467d822f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wdangling-reference19.C
@@ -0,0 +1,25 @@
+// PR c++/110358
+// { dg-do compile { target c++11 } }
+// { dg-options "-Wdangling-reference" }
+// Like Wdangling-reference18.C but not actually a span-like class.
+
+template <typename T>
+struct Span {
+ T* data_;
+ int len_;
+ ~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-warning "dangling reference" }
+ int const& b = get().back(); // { dg-warning "dangling reference" }
+ int const& c = get()[0]; // { dg-warning "dangling reference" }
+
+ return a + b + c;
+}
diff --git a/gcc/testsuite/g++.dg/warn/Wdangling-reference20.C b/gcc/testsuite/g++.dg/warn/Wdangling-reference20.C
new file mode 100644
index 00000000000..463c7380283
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wdangling-reference20.C
@@ -0,0 +1,42 @@
+// PR c++/109640
+// { dg-do compile { target c++20 } }
+// { dg-options "-Wdangling-reference" }
+// Don't warn for std::span-like classes.
+
+#include <iterator>
+#include <span>
+
+template <typename T>
+struct MySpan
+{
+ MySpan(T* data, std::size_t size) :
+ data_(data),
+ size_(size)
+ {}
+
+ T& operator[](std::size_t idx) { return data_[idx]; }
+
+private:
+ T* data_;
+ std::size_t size_;
+};
+
+template <typename T, std::size_t n>
+MySpan<T const> make_my_span(T const(&x)[n])
+{
+ return MySpan(std::begin(x), n);
+}
+
+template <typename T, std::size_t n>
+std::span<T const> make_span(T const(&x)[n])
+{
+ return std::span(std::begin(x), n);
+}
+
+int main()
+{
+ int x[10]{};
+ int const& y{make_my_span(x)[0]};
+ int const& y2{make_span(x)[0]};
+ (void) y, (void) y2;
+}
base-commit: fd620bd3351c6b9821c299035ed17e655d7954b5
--
2.43.0
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] c++: avoid -Wdangling-reference for std::span-like classes [PR110358]
2024-01-26 1:36 ` [PATCH v2] " Marek Polacek
@ 2024-01-26 3:13 ` Jason Merrill
2024-01-30 18:15 ` Marek Polacek
0 siblings, 1 reply; 11+ messages in thread
From: Jason Merrill @ 2024-01-26 3:13 UTC (permalink / raw)
To: Marek Polacek, GCC Patches
On 1/25/24 20:36, Marek Polacek wrote:
> Better version:
>
> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
>
> -- >8 --
> Real-world experience shows that -Wdangling-reference triggers for
> user-defined std::span-like classes a lot. We can easily avoid that
> by considering classes like
>
> template<typename T>
> struct Span {
> T* data_;
> std::size len_;
> };
>
> to be std::span-like, and not warning for them. Unlike the previous
> patch, this one considers a non-union class template that has a pointer
> data member and a trivial destructor as std::span-like.
>
> PR c++/110358
> PR c++/109640
>
> gcc/cp/ChangeLog:
>
> * call.cc (reference_like_class_p): Don't warn for std::span-like
> classes.
>
> gcc/ChangeLog:
>
> * doc/invoke.texi: Update -Wdangling-reference description.
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/warn/Wdangling-reference18.C: New test.
> * g++.dg/warn/Wdangling-reference19.C: New test.
> * g++.dg/warn/Wdangling-reference20.C: New test.
> ---
> gcc/cp/call.cc | 18 ++++++++
> gcc/doc/invoke.texi | 14 +++++++
> .../g++.dg/warn/Wdangling-reference18.C | 24 +++++++++++
> .../g++.dg/warn/Wdangling-reference19.C | 25 +++++++++++
> .../g++.dg/warn/Wdangling-reference20.C | 42 +++++++++++++++++++
> 5 files changed, 123 insertions(+)
> create mode 100644 gcc/testsuite/g++.dg/warn/Wdangling-reference18.C
> create mode 100644 gcc/testsuite/g++.dg/warn/Wdangling-reference19.C
> create mode 100644 gcc/testsuite/g++.dg/warn/Wdangling-reference20.C
>
> diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
> index 9de0d77c423..afd3e1ff024 100644
> --- a/gcc/cp/call.cc
> +++ b/gcc/cp/call.cc
> @@ -14082,6 +14082,24 @@ reference_like_class_p (tree ctype)
> return true;
> }
>
> + /* Avoid warning if CTYPE looks like std::span: it's a class template,
> + has a T* member, and a trivial destructor. For example,
> +
> + template<typename T>
> + struct Span {
> + T* data_;
> + std::size len_;
> + };
> +
> + is considered std::span-like. */
> + if (NON_UNION_CLASS_TYPE_P (ctype)
> + && CLASSTYPE_TEMPLATE_INSTANTIATION (ctype)
> + && TYPE_HAS_TRIVIAL_DESTRUCTOR (ctype))
> + for (tree field = next_aggregate_field (TYPE_FIELDS (ctype));
> + field; field = next_aggregate_field (DECL_CHAIN (field)))
> + if (TYPE_PTR_P (TREE_TYPE (field)))
> + return true;
> +
> /* Some classes, such as std::tuple, have the reference member in its
> (non-direct) base class. */
> if (dfs_walk_once (TYPE_BINFO (ctype), class_has_reference_member_p_r,
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index 6ec56493e59..e0ff18a86f5 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -3916,6 +3916,20 @@ 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 does not warn for @code{std::span}-like classes. We consider
> +classes of the form:
> +
> +@smallexample
> +template<typename T>
> +struct Span @{
> + T* data_;
> + std::size len_;
> +@};
> +@end smallexample
> +
> +as @code{std::span}-like; that is, the class is a non-union class template
> +that has a pointer data member and a trivial destructor.
> +
> This warning is enabled by @option{-Wall}.
>
> @opindex Wdelete-non-virtual-dtor
> diff --git a/gcc/testsuite/g++.dg/warn/Wdangling-reference18.C b/gcc/testsuite/g++.dg/warn/Wdangling-reference18.C
> new file mode 100644
> index 00000000000..e088c177769
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/warn/Wdangling-reference18.C
> @@ -0,0 +1,24 @@
> +// PR c++/110358
> +// { dg-do compile { target c++11 } }
> +// { dg-options "-Wdangling-reference" }
> +// Don't warn for std::span-like classes.
> +
> +template <typename T>
> +struct Span {
> + T* data_;
> + int len_;
> +
> + [[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 reference" }
> + int const& b = get().back(); // { dg-bogus "dangling reference" }
> + int const& c = get()[0]; // { dg-bogus "dangling reference" }
> +
> + return a + b + c;
> +}
> diff --git a/gcc/testsuite/g++.dg/warn/Wdangling-reference19.C b/gcc/testsuite/g++.dg/warn/Wdangling-reference19.C
> new file mode 100644
> index 00000000000..053467d822f
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/warn/Wdangling-reference19.C
> @@ -0,0 +1,25 @@
> +// PR c++/110358
> +// { dg-do compile { target c++11 } }
> +// { dg-options "-Wdangling-reference" }
> +// Like Wdangling-reference18.C but not actually a span-like class.
> +
> +template <typename T>
> +struct Span {
> + T* data_;
> + int len_;
> + ~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-warning "dangling reference" }
> + int const& b = get().back(); // { dg-warning "dangling reference" }
> + int const& c = get()[0]; // { dg-warning "dangling reference" }
> +
> + return a + b + c;
> +}
> diff --git a/gcc/testsuite/g++.dg/warn/Wdangling-reference20.C b/gcc/testsuite/g++.dg/warn/Wdangling-reference20.C
> new file mode 100644
> index 00000000000..463c7380283
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/warn/Wdangling-reference20.C
> @@ -0,0 +1,42 @@
> +// PR c++/109640
> +// { dg-do compile { target c++20 } }
> +// { dg-options "-Wdangling-reference" }
> +// Don't warn for std::span-like classes.
> +
> +#include <iterator>
> +#include <span>
> +
> +template <typename T>
> +struct MySpan
> +{
> + MySpan(T* data, std::size_t size) :
> + data_(data),
> + size_(size)
> + {}
> +
> + T& operator[](std::size_t idx) { return data_[idx]; }
> +
> +private:
> + T* data_;
> + std::size_t size_;
> +};
> +
> +template <typename T, std::size_t n>
> +MySpan<T const> make_my_span(T const(&x)[n])
> +{
> + return MySpan(std::begin(x), n);
> +}
> +
> +template <typename T, std::size_t n>
> +std::span<T const> make_span(T const(&x)[n])
> +{
> + return std::span(std::begin(x), n);
> +}
> +
> +int main()
> +{
> + int x[10]{};
> + int const& y{make_my_span(x)[0]};
> + int const& y2{make_span(x)[0]};
Let's also test that we do warn if the argument to make_*span is a
temporary. OK with that change, assuming it works as expected.
Jason
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] c++: avoid -Wdangling-reference for std::span-like classes [PR110358]
2024-01-26 3:13 ` Jason Merrill
@ 2024-01-30 18:15 ` Marek Polacek
2024-01-31 19:44 ` Alex Coplan
0 siblings, 1 reply; 11+ messages in thread
From: Marek Polacek @ 2024-01-30 18:15 UTC (permalink / raw)
To: Jason Merrill; +Cc: GCC Patches
On Thu, Jan 25, 2024 at 10:13:10PM -0500, Jason Merrill wrote:
> On 1/25/24 20:36, Marek Polacek wrote:
> > Better version:
> >
> > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> >
> > -- >8 --
> > Real-world experience shows that -Wdangling-reference triggers for
> > user-defined std::span-like classes a lot. We can easily avoid that
> > by considering classes like
> >
> > template<typename T>
> > struct Span {
> > T* data_;
> > std::size len_;
> > };
> >
> > to be std::span-like, and not warning for them. Unlike the previous
> > patch, this one considers a non-union class template that has a pointer
> > data member and a trivial destructor as std::span-like.
> >
> > PR c++/110358
> > PR c++/109640
> >
> > gcc/cp/ChangeLog:
> >
> > * call.cc (reference_like_class_p): Don't warn for std::span-like
> > classes.
> >
> > gcc/ChangeLog:
> >
> > * doc/invoke.texi: Update -Wdangling-reference description.
> >
> > gcc/testsuite/ChangeLog:
> >
> > * g++.dg/warn/Wdangling-reference18.C: New test.
> > * g++.dg/warn/Wdangling-reference19.C: New test.
> > * g++.dg/warn/Wdangling-reference20.C: New test.
> > ---
> > gcc/cp/call.cc | 18 ++++++++
> > gcc/doc/invoke.texi | 14 +++++++
> > .../g++.dg/warn/Wdangling-reference18.C | 24 +++++++++++
> > .../g++.dg/warn/Wdangling-reference19.C | 25 +++++++++++
> > .../g++.dg/warn/Wdangling-reference20.C | 42 +++++++++++++++++++
> > 5 files changed, 123 insertions(+)
> > create mode 100644 gcc/testsuite/g++.dg/warn/Wdangling-reference18.C
> > create mode 100644 gcc/testsuite/g++.dg/warn/Wdangling-reference19.C
> > create mode 100644 gcc/testsuite/g++.dg/warn/Wdangling-reference20.C
> >
> > diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
> > index 9de0d77c423..afd3e1ff024 100644
> > --- a/gcc/cp/call.cc
> > +++ b/gcc/cp/call.cc
> > @@ -14082,6 +14082,24 @@ reference_like_class_p (tree ctype)
> > return true;
> > }
> > + /* Avoid warning if CTYPE looks like std::span: it's a class template,
> > + has a T* member, and a trivial destructor. For example,
> > +
> > + template<typename T>
> > + struct Span {
> > + T* data_;
> > + std::size len_;
> > + };
> > +
> > + is considered std::span-like. */
> > + if (NON_UNION_CLASS_TYPE_P (ctype)
> > + && CLASSTYPE_TEMPLATE_INSTANTIATION (ctype)
> > + && TYPE_HAS_TRIVIAL_DESTRUCTOR (ctype))
> > + for (tree field = next_aggregate_field (TYPE_FIELDS (ctype));
> > + field; field = next_aggregate_field (DECL_CHAIN (field)))
> > + if (TYPE_PTR_P (TREE_TYPE (field)))
> > + return true;
> > +
> > /* Some classes, such as std::tuple, have the reference member in its
> > (non-direct) base class. */
> > if (dfs_walk_once (TYPE_BINFO (ctype), class_has_reference_member_p_r,
> > diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> > index 6ec56493e59..e0ff18a86f5 100644
> > --- a/gcc/doc/invoke.texi
> > +++ b/gcc/doc/invoke.texi
> > @@ -3916,6 +3916,20 @@ 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 does not warn for @code{std::span}-like classes. We consider
> > +classes of the form:
> > +
> > +@smallexample
> > +template<typename T>
> > +struct Span @{
> > + T* data_;
> > + std::size len_;
> > +@};
> > +@end smallexample
> > +
> > +as @code{std::span}-like; that is, the class is a non-union class template
> > +that has a pointer data member and a trivial destructor.
> > +
> > This warning is enabled by @option{-Wall}.
> > @opindex Wdelete-non-virtual-dtor
> > diff --git a/gcc/testsuite/g++.dg/warn/Wdangling-reference18.C b/gcc/testsuite/g++.dg/warn/Wdangling-reference18.C
> > new file mode 100644
> > index 00000000000..e088c177769
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/warn/Wdangling-reference18.C
> > @@ -0,0 +1,24 @@
> > +// PR c++/110358
> > +// { dg-do compile { target c++11 } }
> > +// { dg-options "-Wdangling-reference" }
> > +// Don't warn for std::span-like classes.
> > +
> > +template <typename T>
> > +struct Span {
> > + T* data_;
> > + int len_;
> > +
> > + [[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 reference" }
> > + int const& b = get().back(); // { dg-bogus "dangling reference" }
> > + int const& c = get()[0]; // { dg-bogus "dangling reference" }
> > +
> > + return a + b + c;
> > +}
> > diff --git a/gcc/testsuite/g++.dg/warn/Wdangling-reference19.C b/gcc/testsuite/g++.dg/warn/Wdangling-reference19.C
> > new file mode 100644
> > index 00000000000..053467d822f
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/warn/Wdangling-reference19.C
> > @@ -0,0 +1,25 @@
> > +// PR c++/110358
> > +// { dg-do compile { target c++11 } }
> > +// { dg-options "-Wdangling-reference" }
> > +// Like Wdangling-reference18.C but not actually a span-like class.
> > +
> > +template <typename T>
> > +struct Span {
> > + T* data_;
> > + int len_;
> > + ~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-warning "dangling reference" }
> > + int const& b = get().back(); // { dg-warning "dangling reference" }
> > + int const& c = get()[0]; // { dg-warning "dangling reference" }
> > +
> > + return a + b + c;
> > +}
> > diff --git a/gcc/testsuite/g++.dg/warn/Wdangling-reference20.C b/gcc/testsuite/g++.dg/warn/Wdangling-reference20.C
> > new file mode 100644
> > index 00000000000..463c7380283
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/warn/Wdangling-reference20.C
> > @@ -0,0 +1,42 @@
> > +// PR c++/109640
> > +// { dg-do compile { target c++20 } }
> > +// { dg-options "-Wdangling-reference" }
> > +// Don't warn for std::span-like classes.
> > +
> > +#include <iterator>
> > +#include <span>
> > +
> > +template <typename T>
> > +struct MySpan
> > +{
> > + MySpan(T* data, std::size_t size) :
> > + data_(data),
> > + size_(size)
> > + {}
> > +
> > + T& operator[](std::size_t idx) { return data_[idx]; }
> > +
> > +private:
> > + T* data_;
> > + std::size_t size_;
> > +};
> > +
> > +template <typename T, std::size_t n>
> > +MySpan<T const> make_my_span(T const(&x)[n])
> > +{
> > + return MySpan(std::begin(x), n);
> > +}
> > +
> > +template <typename T, std::size_t n>
> > +std::span<T const> make_span(T const(&x)[n])
> > +{
> > + return std::span(std::begin(x), n);
> > +}
> > +
> > +int main()
> > +{
> > + int x[10]{};
> > + int const& y{make_my_span(x)[0]};
> > + int const& y2{make_span(x)[0]};
>
> Let's also test that we do warn if the argument to make_*span is a
> temporary. OK with that change, assuming it works as expected.
We do warn then, I've added
using T = int[10];
[[maybe_unused]] int const& y3{make_my_span(T{})[0]};
[[maybe_unused]] int const& y4{make_span(T{})[0]};
and get two warnings. I'll push with that adjusted; thanks.
Marek
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] c++: avoid -Wdangling-reference for std::span-like classes [PR110358]
2024-01-30 18:15 ` Marek Polacek
@ 2024-01-31 19:44 ` Alex Coplan
2024-01-31 19:57 ` Jason Merrill
2024-01-31 20:53 ` Marek Polacek
0 siblings, 2 replies; 11+ messages in thread
From: Alex Coplan @ 2024-01-31 19:44 UTC (permalink / raw)
To: Marek Polacek; +Cc: Jason Merrill, GCC Patches
Hi Marek,
On 30/01/2024 13:15, Marek Polacek wrote:
> On Thu, Jan 25, 2024 at 10:13:10PM -0500, Jason Merrill wrote:
> > On 1/25/24 20:36, Marek Polacek wrote:
> > > Better version:
> > >
> > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> > >
> > > -- >8 --
> > > Real-world experience shows that -Wdangling-reference triggers for
> > > user-defined std::span-like classes a lot. We can easily avoid that
> > > by considering classes like
> > >
> > > template<typename T>
> > > struct Span {
> > > T* data_;
> > > std::size len_;
> > > };
> > >
> > > to be std::span-like, and not warning for them. Unlike the previous
> > > patch, this one considers a non-union class template that has a pointer
> > > data member and a trivial destructor as std::span-like.
> > >
> > > PR c++/110358
> > > PR c++/109640
> > >
> > > gcc/cp/ChangeLog:
> > >
> > > * call.cc (reference_like_class_p): Don't warn for std::span-like
> > > classes.
> > >
> > > gcc/ChangeLog:
> > >
> > > * doc/invoke.texi: Update -Wdangling-reference description.
> > >
> > > gcc/testsuite/ChangeLog:
> > >
> > > * g++.dg/warn/Wdangling-reference18.C: New test.
> > > * g++.dg/warn/Wdangling-reference19.C: New test.
> > > * g++.dg/warn/Wdangling-reference20.C: New test.
> > > ---
> > > gcc/cp/call.cc | 18 ++++++++
> > > gcc/doc/invoke.texi | 14 +++++++
> > > .../g++.dg/warn/Wdangling-reference18.C | 24 +++++++++++
> > > .../g++.dg/warn/Wdangling-reference19.C | 25 +++++++++++
> > > .../g++.dg/warn/Wdangling-reference20.C | 42 +++++++++++++++++++
> > > 5 files changed, 123 insertions(+)
> > > create mode 100644 gcc/testsuite/g++.dg/warn/Wdangling-reference18.C
> > > create mode 100644 gcc/testsuite/g++.dg/warn/Wdangling-reference19.C
> > > create mode 100644 gcc/testsuite/g++.dg/warn/Wdangling-reference20.C
> > >
> > > diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
> > > index 9de0d77c423..afd3e1ff024 100644
> > > --- a/gcc/cp/call.cc
> > > +++ b/gcc/cp/call.cc
> > > @@ -14082,6 +14082,24 @@ reference_like_class_p (tree ctype)
> > > return true;
> > > }
> > > + /* Avoid warning if CTYPE looks like std::span: it's a class template,
> > > + has a T* member, and a trivial destructor. For example,
> > > +
> > > + template<typename T>
> > > + struct Span {
> > > + T* data_;
> > > + std::size len_;
> > > + };
> > > +
> > > + is considered std::span-like. */
> > > + if (NON_UNION_CLASS_TYPE_P (ctype)
> > > + && CLASSTYPE_TEMPLATE_INSTANTIATION (ctype)
> > > + && TYPE_HAS_TRIVIAL_DESTRUCTOR (ctype))
> > > + for (tree field = next_aggregate_field (TYPE_FIELDS (ctype));
> > > + field; field = next_aggregate_field (DECL_CHAIN (field)))
> > > + if (TYPE_PTR_P (TREE_TYPE (field)))
> > > + return true;
> > > +
> > > /* Some classes, such as std::tuple, have the reference member in its
> > > (non-direct) base class. */
> > > if (dfs_walk_once (TYPE_BINFO (ctype), class_has_reference_member_p_r,
> > > diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> > > index 6ec56493e59..e0ff18a86f5 100644
> > > --- a/gcc/doc/invoke.texi
> > > +++ b/gcc/doc/invoke.texi
> > > @@ -3916,6 +3916,20 @@ 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 does not warn for @code{std::span}-like classes. We consider
> > > +classes of the form:
> > > +
> > > +@smallexample
> > > +template<typename T>
> > > +struct Span @{
> > > + T* data_;
> > > + std::size len_;
> > > +@};
> > > +@end smallexample
> > > +
> > > +as @code{std::span}-like; that is, the class is a non-union class template
> > > +that has a pointer data member and a trivial destructor.
> > > +
> > > This warning is enabled by @option{-Wall}.
> > > @opindex Wdelete-non-virtual-dtor
> > > diff --git a/gcc/testsuite/g++.dg/warn/Wdangling-reference18.C b/gcc/testsuite/g++.dg/warn/Wdangling-reference18.C
> > > new file mode 100644
> > > index 00000000000..e088c177769
> > > --- /dev/null
> > > +++ b/gcc/testsuite/g++.dg/warn/Wdangling-reference18.C
> > > @@ -0,0 +1,24 @@
> > > +// PR c++/110358
> > > +// { dg-do compile { target c++11 } }
> > > +// { dg-options "-Wdangling-reference" }
> > > +// Don't warn for std::span-like classes.
> > > +
> > > +template <typename T>
> > > +struct Span {
> > > + T* data_;
> > > + int len_;
> > > +
> > > + [[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 reference" }
> > > + int const& b = get().back(); // { dg-bogus "dangling reference" }
> > > + int const& c = get()[0]; // { dg-bogus "dangling reference" }
> > > +
> > > + return a + b + c;
> > > +}
> > > diff --git a/gcc/testsuite/g++.dg/warn/Wdangling-reference19.C b/gcc/testsuite/g++.dg/warn/Wdangling-reference19.C
> > > new file mode 100644
> > > index 00000000000..053467d822f
> > > --- /dev/null
> > > +++ b/gcc/testsuite/g++.dg/warn/Wdangling-reference19.C
> > > @@ -0,0 +1,25 @@
> > > +// PR c++/110358
> > > +// { dg-do compile { target c++11 } }
> > > +// { dg-options "-Wdangling-reference" }
> > > +// Like Wdangling-reference18.C but not actually a span-like class.
> > > +
> > > +template <typename T>
> > > +struct Span {
> > > + T* data_;
> > > + int len_;
> > > + ~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-warning "dangling reference" }
> > > + int const& b = get().back(); // { dg-warning "dangling reference" }
> > > + int const& c = get()[0]; // { dg-warning "dangling reference" }
> > > +
> > > + return a + b + c;
> > > +}
> > > diff --git a/gcc/testsuite/g++.dg/warn/Wdangling-reference20.C b/gcc/testsuite/g++.dg/warn/Wdangling-reference20.C
> > > new file mode 100644
> > > index 00000000000..463c7380283
> > > --- /dev/null
> > > +++ b/gcc/testsuite/g++.dg/warn/Wdangling-reference20.C
> > > @@ -0,0 +1,42 @@
> > > +// PR c++/109640
> > > +// { dg-do compile { target c++20 } }
> > > +// { dg-options "-Wdangling-reference" }
> > > +// Don't warn for std::span-like classes.
> > > +
> > > +#include <iterator>
> > > +#include <span>
> > > +
> > > +template <typename T>
> > > +struct MySpan
> > > +{
> > > + MySpan(T* data, std::size_t size) :
> > > + data_(data),
> > > + size_(size)
> > > + {}
> > > +
> > > + T& operator[](std::size_t idx) { return data_[idx]; }
> > > +
> > > +private:
> > > + T* data_;
> > > + std::size_t size_;
> > > +};
> > > +
> > > +template <typename T, std::size_t n>
> > > +MySpan<T const> make_my_span(T const(&x)[n])
> > > +{
> > > + return MySpan(std::begin(x), n);
> > > +}
> > > +
> > > +template <typename T, std::size_t n>
> > > +std::span<T const> make_span(T const(&x)[n])
> > > +{
> > > + return std::span(std::begin(x), n);
> > > +}
> > > +
> > > +int main()
> > > +{
> > > + int x[10]{};
> > > + int const& y{make_my_span(x)[0]};
> > > + int const& y2{make_span(x)[0]};
> >
> > Let's also test that we do warn if the argument to make_*span is a
> > temporary. OK with that change, assuming it works as expected.
>
> We do warn then, I've added
> using T = int[10];
> [[maybe_unused]] int const& y3{make_my_span(T{})[0]};
> [[maybe_unused]] int const& y4{make_span(T{})[0]};
> and get two warnings. I'll push with that adjusted; thanks.
It looks like this patch breaks bootstrap on aarch64-linux-gnu, I see
the following building aarch64-early-ra.cc in stage2:
/work/builds/bstrap/./prev-gcc/xg++ -B/work/builds/bstrap/./prev-gcc/ -B/work/builds/bstrap/aarch64-unknown-linux-gnu/bin/ -nostdinc++ -B/work/builds/bstrap/prev-aarch64-unknown-linux-gnu/libstdc++-v3/src/.libs -B/work/builds/bstrap/prev-aarch64-unknown-linux-gnu/libstdc++-v3/libsupc++/.libs -I/work/builds/bstrap/prev-aarch64-unknown-linux-gnu/libstdc++-v3/include/aarch64-unknown-linux-gnu -I/work/builds/bstrap/prev-aarch64-unknown-linux-gnu/libstdc++-v3/include -I/home/alecop01/toolchain/src/gcc/libstdc++-v3/libsupc++ -L/work/builds/bstrap/prev-aarch64-unknown-linux-gnu/libstdc++-v3/src/.libs -L/work/builds/bstrap/prev-aarch64-unknown-linux-gnu/libstdc++-v3/libsupc++/.libs -fno-PIE -c -g -O2 -fno-checking -gtoggle -DIN_GCC -fno-exceptions -fno-rtti -fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual -Wmissing-format-attribute -Wconditionally-supported -Woverloaded-virtual -pedantic -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -Werror -fno-common -DHAVE_CONFIG_H -fno-PIE -I. -I. -I/home/alecop01/toolchain/src/gcc/gcc -I/home/alecop01/toolchain/src/gcc/gcc/. -I/home/alecop01/toolchain/src/gcc/gcc/../include -I/home/alecop01/toolchain/src/gcc/gcc/../libcpp/include -I/home/alecop01/toolchain/src/gcc/gcc/../libcody -I/home/alecop01/toolchain/src/gcc/gcc/../libdecnumber -I/home/alecop01/toolchain/src/gcc/gcc/../libdecnumber/bid -I../libdecnumber -I/home/alecop01/toolchain/src/gcc/gcc/../libbacktrace -I. -I. -I/home/alecop01/toolchain/src/gcc/gcc -I/home/alecop01/toolchain/src/gcc/gcc/. -I/home/alecop01/toolchain/src/gcc/gcc/../include -I/home/alecop01/toolchain/src/gcc/gcc/../libcpp/include -I/home/alecop01/toolchain/src/gcc/gcc/../libcody -I/home/alecop01/toolchain/src/gcc/gcc/../libdecnumber -I/home/alecop01/toolchain/src/gcc/gcc/../libdecnumber/bid -I../libdecnumber -I/home/alecop01/toolchain/src/gcc/gcc/../libbacktrace \
/home/alecop01/toolchain/src/gcc/gcc/config/aarch64/aarch64-early-ra.cc
/home/alecop01/toolchain/src/gcc/gcc/config/aarch64/aarch64-early-ra.cc: In member function ‘void {anonymous}::early_ra::record_constraints(rtx_insn*)’:
/home/alecop01/toolchain/src/gcc/gcc/config/aarch64/aarch64-early-ra.cc:1858:66: error: possibly dangling reference to a temporary [-Werror=dangling-reference]
1858 | for (auto &allocno : get_allocno_subgroup (op).allocnos ())
| ^
/home/alecop01/toolchain/src/gcc/gcc/config/aarch64/aarch64-early-ra.cc:1858:65: note: the temporary was destroyed at the end of the full expression ‘(({anonymous}::early_ra*)this)->{anonymous}::early_ra::get_allocno_subgroup(op).{anonymous}::early_ra::allocno_subgroup::allocnos()’
1858 | for (auto &allocno : get_allocno_subgroup (op).allocnos ())
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~
cc1plus: all warnings being treated as errors
make[3]: *** [/home/alecop01/toolchain/src/gcc/gcc/config/aarch64/t-aarch64:200: aarch64-early-ra.o] Error 1
make[3]: Leaving directory '/work/builds/bstrap/gcc'
make[2]: *** [Makefile:5096: all-stage2-gcc] Error 2
make[2]: Leaving directory '/work/builds/bstrap'
make[1]: *** [Makefile:25405: stage2-bubble] Error 2
make[1]: Leaving directory '/work/builds/bstrap'
make: *** [Makefile:1100: all] Error 2
Is the patch expected to introduce new warnings?
I'll try to reduce a testcase from this where we don't see the warning
before and we see the warning afterwards.
Thanks,
Alex
>
> Marek
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] c++: avoid -Wdangling-reference for std::span-like classes [PR110358]
2024-01-31 19:44 ` Alex Coplan
@ 2024-01-31 19:57 ` Jason Merrill
2024-01-31 20:56 ` Marek Polacek
2024-01-31 20:53 ` Marek Polacek
1 sibling, 1 reply; 11+ messages in thread
From: Jason Merrill @ 2024-01-31 19:57 UTC (permalink / raw)
To: Alex Coplan, Marek Polacek; +Cc: GCC Patches
On 1/31/24 14:44, Alex Coplan wrote:
> Hi Marek,
>
> On 30/01/2024 13:15, Marek Polacek wrote:
>> On Thu, Jan 25, 2024 at 10:13:10PM -0500, Jason Merrill wrote:
>>> On 1/25/24 20:36, Marek Polacek wrote:
>>>> Better version:
>>>>
>>>> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
>>>>
>>>> -- >8 --
>>>> Real-world experience shows that -Wdangling-reference triggers for
>>>> user-defined std::span-like classes a lot. We can easily avoid that
>>>> by considering classes like
>>>>
>>>> template<typename T>
>>>> struct Span {
>>>> T* data_;
>>>> std::size len_;
>>>> };
>>>>
>>>> to be std::span-like, and not warning for them. Unlike the previous
>>>> patch, this one considers a non-union class template that has a pointer
>>>> data member and a trivial destructor as std::span-like.
>>>>
>>>> PR c++/110358
>>>> PR c++/109640
>>>>
>>>> gcc/cp/ChangeLog:
>>>>
>>>> * call.cc (reference_like_class_p): Don't warn for std::span-like
>>>> classes.
>>>>
>>>> gcc/ChangeLog:
>>>>
>>>> * doc/invoke.texi: Update -Wdangling-reference description.
>>>>
>>>> gcc/testsuite/ChangeLog:
>>>>
>>>> * g++.dg/warn/Wdangling-reference18.C: New test.
>>>> * g++.dg/warn/Wdangling-reference19.C: New test.
>>>> * g++.dg/warn/Wdangling-reference20.C: New test.
>>>> ---
>>>> gcc/cp/call.cc | 18 ++++++++
>>>> gcc/doc/invoke.texi | 14 +++++++
>>>> .../g++.dg/warn/Wdangling-reference18.C | 24 +++++++++++
>>>> .../g++.dg/warn/Wdangling-reference19.C | 25 +++++++++++
>>>> .../g++.dg/warn/Wdangling-reference20.C | 42 +++++++++++++++++++
>>>> 5 files changed, 123 insertions(+)
>>>> create mode 100644 gcc/testsuite/g++.dg/warn/Wdangling-reference18.C
>>>> create mode 100644 gcc/testsuite/g++.dg/warn/Wdangling-reference19.C
>>>> create mode 100644 gcc/testsuite/g++.dg/warn/Wdangling-reference20.C
>>>>
>>>> diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
>>>> index 9de0d77c423..afd3e1ff024 100644
>>>> --- a/gcc/cp/call.cc
>>>> +++ b/gcc/cp/call.cc
>>>> @@ -14082,6 +14082,24 @@ reference_like_class_p (tree ctype)
>>>> return true;
>>>> }
>>>> + /* Avoid warning if CTYPE looks like std::span: it's a class template,
>>>> + has a T* member, and a trivial destructor. For example,
>>>> +
>>>> + template<typename T>
>>>> + struct Span {
>>>> + T* data_;
>>>> + std::size len_;
>>>> + };
>>>> +
>>>> + is considered std::span-like. */
>>>> + if (NON_UNION_CLASS_TYPE_P (ctype)
>>>> + && CLASSTYPE_TEMPLATE_INSTANTIATION (ctype)
>>>> + && TYPE_HAS_TRIVIAL_DESTRUCTOR (ctype))
>>>> + for (tree field = next_aggregate_field (TYPE_FIELDS (ctype));
>>>> + field; field = next_aggregate_field (DECL_CHAIN (field)))
>>>> + if (TYPE_PTR_P (TREE_TYPE (field)))
>>>> + return true;
>>>> +
>>>> /* Some classes, such as std::tuple, have the reference member in its
>>>> (non-direct) base class. */
>>>> if (dfs_walk_once (TYPE_BINFO (ctype), class_has_reference_member_p_r,
>>>> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
>>>> index 6ec56493e59..e0ff18a86f5 100644
>>>> --- a/gcc/doc/invoke.texi
>>>> +++ b/gcc/doc/invoke.texi
>>>> @@ -3916,6 +3916,20 @@ 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 does not warn for @code{std::span}-like classes. We consider
>>>> +classes of the form:
>>>> +
>>>> +@smallexample
>>>> +template<typename T>
>>>> +struct Span @{
>>>> + T* data_;
>>>> + std::size len_;
>>>> +@};
>>>> +@end smallexample
>>>> +
>>>> +as @code{std::span}-like; that is, the class is a non-union class template
>>>> +that has a pointer data member and a trivial destructor.
>>>> +
>>>> This warning is enabled by @option{-Wall}.
>>>> @opindex Wdelete-non-virtual-dtor
>>>> diff --git a/gcc/testsuite/g++.dg/warn/Wdangling-reference18.C b/gcc/testsuite/g++.dg/warn/Wdangling-reference18.C
>>>> new file mode 100644
>>>> index 00000000000..e088c177769
>>>> --- /dev/null
>>>> +++ b/gcc/testsuite/g++.dg/warn/Wdangling-reference18.C
>>>> @@ -0,0 +1,24 @@
>>>> +// PR c++/110358
>>>> +// { dg-do compile { target c++11 } }
>>>> +// { dg-options "-Wdangling-reference" }
>>>> +// Don't warn for std::span-like classes.
>>>> +
>>>> +template <typename T>
>>>> +struct Span {
>>>> + T* data_;
>>>> + int len_;
>>>> +
>>>> + [[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 reference" }
>>>> + int const& b = get().back(); // { dg-bogus "dangling reference" }
>>>> + int const& c = get()[0]; // { dg-bogus "dangling reference" }
>>>> +
>>>> + return a + b + c;
>>>> +}
>>>> diff --git a/gcc/testsuite/g++.dg/warn/Wdangling-reference19.C b/gcc/testsuite/g++.dg/warn/Wdangling-reference19.C
>>>> new file mode 100644
>>>> index 00000000000..053467d822f
>>>> --- /dev/null
>>>> +++ b/gcc/testsuite/g++.dg/warn/Wdangling-reference19.C
>>>> @@ -0,0 +1,25 @@
>>>> +// PR c++/110358
>>>> +// { dg-do compile { target c++11 } }
>>>> +// { dg-options "-Wdangling-reference" }
>>>> +// Like Wdangling-reference18.C but not actually a span-like class.
>>>> +
>>>> +template <typename T>
>>>> +struct Span {
>>>> + T* data_;
>>>> + int len_;
>>>> + ~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-warning "dangling reference" }
>>>> + int const& b = get().back(); // { dg-warning "dangling reference" }
>>>> + int const& c = get()[0]; // { dg-warning "dangling reference" }
>>>> +
>>>> + return a + b + c;
>>>> +}
>>>> diff --git a/gcc/testsuite/g++.dg/warn/Wdangling-reference20.C b/gcc/testsuite/g++.dg/warn/Wdangling-reference20.C
>>>> new file mode 100644
>>>> index 00000000000..463c7380283
>>>> --- /dev/null
>>>> +++ b/gcc/testsuite/g++.dg/warn/Wdangling-reference20.C
>>>> @@ -0,0 +1,42 @@
>>>> +// PR c++/109640
>>>> +// { dg-do compile { target c++20 } }
>>>> +// { dg-options "-Wdangling-reference" }
>>>> +// Don't warn for std::span-like classes.
>>>> +
>>>> +#include <iterator>
>>>> +#include <span>
>>>> +
>>>> +template <typename T>
>>>> +struct MySpan
>>>> +{
>>>> + MySpan(T* data, std::size_t size) :
>>>> + data_(data),
>>>> + size_(size)
>>>> + {}
>>>> +
>>>> + T& operator[](std::size_t idx) { return data_[idx]; }
>>>> +
>>>> +private:
>>>> + T* data_;
>>>> + std::size_t size_;
>>>> +};
>>>> +
>>>> +template <typename T, std::size_t n>
>>>> +MySpan<T const> make_my_span(T const(&x)[n])
>>>> +{
>>>> + return MySpan(std::begin(x), n);
>>>> +}
>>>> +
>>>> +template <typename T, std::size_t n>
>>>> +std::span<T const> make_span(T const(&x)[n])
>>>> +{
>>>> + return std::span(std::begin(x), n);
>>>> +}
>>>> +
>>>> +int main()
>>>> +{
>>>> + int x[10]{};
>>>> + int const& y{make_my_span(x)[0]};
>>>> + int const& y2{make_span(x)[0]};
>>>
>>> Let's also test that we do warn if the argument to make_*span is a
>>> temporary. OK with that change, assuming it works as expected.
>>
>> We do warn then, I've added
>> using T = int[10];
>> [[maybe_unused]] int const& y3{make_my_span(T{})[0]};
>> [[maybe_unused]] int const& y4{make_span(T{})[0]};
>> and get two warnings. I'll push with that adjusted; thanks.
>
> It looks like this patch breaks bootstrap on aarch64-linux-gnu, I see
> the following building aarch64-early-ra.cc in stage2:
>
> /work/builds/bstrap/./prev-gcc/xg++ -B/work/builds/bstrap/./prev-gcc/ -B/work/builds/bstrap/aarch64-unknown-linux-gnu/bin/ -nostdinc++ -B/work/builds/bstrap/prev-aarch64-unknown-linux-gnu/libstdc++-v3/src/.libs -B/work/builds/bstrap/prev-aarch64-unknown-linux-gnu/libstdc++-v3/libsupc++/.libs -I/work/builds/bstrap/prev-aarch64-unknown-linux-gnu/libstdc++-v3/include/aarch64-unknown-linux-gnu -I/work/builds/bstrap/prev-aarch64-unknown-linux-gnu/libstdc++-v3/include -I/home/alecop01/toolchain/src/gcc/libstdc++-v3/libsupc++ -L/work/builds/bstrap/prev-aarch64-unknown-linux-gnu/libstdc++-v3/src/.libs -L/work/builds/bstrap/prev-aarch64-unknown-linux-gnu/libstdc++-v3/libsupc++/.libs -fno-PIE -c -g -O2 -fno-checking -gtoggle -DIN_GCC -fno-exceptions -fno-rtti -fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual -Wmissing-format-attribute -Wconditionally-supported -Woverloaded-virtual -pedantic -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -Werror -fno-common -DHAVE_CONFIG_H -fno-PIE -I. -I. -I/home/alecop01/toolchain/src/gcc/gcc -I/home/alecop01/toolchain/src/gcc/gcc/. -I/home/alecop01/toolchain/src/gcc/gcc/../include -I/home/alecop01/toolchain/src/gcc/gcc/../libcpp/include -I/home/alecop01/toolchain/src/gcc/gcc/../libcody -I/home/alecop01/toolchain/src/gcc/gcc/../libdecnumber -I/home/alecop01/toolchain/src/gcc/gcc/../libdecnumber/bid -I../libdecnumber -I/home/alecop01/toolchain/src/gcc/gcc/../libbacktrace -I. -I. -I/home/alecop01/toolchain/src/gcc/gcc -I/home/alecop01/toolchain/src/gcc/gcc/. -I/home/alecop01/toolchain/src/gcc/gcc/../include -I/home/alecop01/toolchain/src/gcc/gcc/../libcpp/include -I/home/alecop01/toolchain/src/gcc/gcc/../libcody -I/home/alecop01/toolchain/src/gcc/gcc/../libdecnumber -I/home/alecop01/toolchain/src/gcc/gcc/../libdecnumber/bid -I../libdecnumber -I/home/alecop01/toolchain/src/gcc/gcc/../libbacktrace \
> /home/alecop01/toolchain/src/gcc/gcc/config/aarch64/aarch64-early-ra.cc
> /home/alecop01/toolchain/src/gcc/gcc/config/aarch64/aarch64-early-ra.cc: In member function ‘void {anonymous}::early_ra::record_constraints(rtx_insn*)’:
> /home/alecop01/toolchain/src/gcc/gcc/config/aarch64/aarch64-early-ra.cc:1858:66: error: possibly dangling reference to a temporary [-Werror=dangling-reference]
> 1858 | for (auto &allocno : get_allocno_subgroup (op).allocnos ())
> | ^
> /home/alecop01/toolchain/src/gcc/gcc/config/aarch64/aarch64-early-ra.cc:1858:65: note: the temporary was destroyed at the end of the full expression ‘(({anonymous}::early_ra*)this)->{anonymous}::early_ra::get_allocno_subgroup(op).{anonymous}::early_ra::allocno_subgroup::allocnos()’
> 1858 | for (auto &allocno : get_allocno_subgroup (op).allocnos ())
> | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~
IMO it's good that we now recognize array_slice (returned from allocnos)
as a span-like class, but we should also recognize allocno_subgroup as
one and avoid warning.
Jason
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] c++: avoid -Wdangling-reference for std::span-like classes [PR110358]
2024-01-31 19:44 ` Alex Coplan
2024-01-31 19:57 ` Jason Merrill
@ 2024-01-31 20:53 ` Marek Polacek
2024-02-01 14:32 ` Alex Coplan
1 sibling, 1 reply; 11+ messages in thread
From: Marek Polacek @ 2024-01-31 20:53 UTC (permalink / raw)
To: Alex Coplan; +Cc: Jason Merrill, GCC Patches
On Wed, Jan 31, 2024 at 07:44:41PM +0000, Alex Coplan wrote:
> Hi Marek,
>
> On 30/01/2024 13:15, Marek Polacek wrote:
> > On Thu, Jan 25, 2024 at 10:13:10PM -0500, Jason Merrill wrote:
> > > On 1/25/24 20:36, Marek Polacek wrote:
> > > > Better version:
> > > >
> > > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> > > >
> > > > -- >8 --
> > > > Real-world experience shows that -Wdangling-reference triggers for
> > > > user-defined std::span-like classes a lot. We can easily avoid that
> > > > by considering classes like
> > > >
> > > > template<typename T>
> > > > struct Span {
> > > > T* data_;
> > > > std::size len_;
> > > > };
> > > >
> > > > to be std::span-like, and not warning for them. Unlike the previous
> > > > patch, this one considers a non-union class template that has a pointer
> > > > data member and a trivial destructor as std::span-like.
> > > >
> > > > PR c++/110358
> > > > PR c++/109640
> > > >
> > > > gcc/cp/ChangeLog:
> > > >
> > > > * call.cc (reference_like_class_p): Don't warn for std::span-like
> > > > classes.
> > > >
> > > > gcc/ChangeLog:
> > > >
> > > > * doc/invoke.texi: Update -Wdangling-reference description.
> > > >
> > > > gcc/testsuite/ChangeLog:
> > > >
> > > > * g++.dg/warn/Wdangling-reference18.C: New test.
> > > > * g++.dg/warn/Wdangling-reference19.C: New test.
> > > > * g++.dg/warn/Wdangling-reference20.C: New test.
> > > > ---
> > > > gcc/cp/call.cc | 18 ++++++++
> > > > gcc/doc/invoke.texi | 14 +++++++
> > > > .../g++.dg/warn/Wdangling-reference18.C | 24 +++++++++++
> > > > .../g++.dg/warn/Wdangling-reference19.C | 25 +++++++++++
> > > > .../g++.dg/warn/Wdangling-reference20.C | 42 +++++++++++++++++++
> > > > 5 files changed, 123 insertions(+)
> > > > create mode 100644 gcc/testsuite/g++.dg/warn/Wdangling-reference18.C
> > > > create mode 100644 gcc/testsuite/g++.dg/warn/Wdangling-reference19.C
> > > > create mode 100644 gcc/testsuite/g++.dg/warn/Wdangling-reference20.C
> > > >
> > > > diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
> > > > index 9de0d77c423..afd3e1ff024 100644
> > > > --- a/gcc/cp/call.cc
> > > > +++ b/gcc/cp/call.cc
> > > > @@ -14082,6 +14082,24 @@ reference_like_class_p (tree ctype)
> > > > return true;
> > > > }
> > > > + /* Avoid warning if CTYPE looks like std::span: it's a class template,
> > > > + has a T* member, and a trivial destructor. For example,
> > > > +
> > > > + template<typename T>
> > > > + struct Span {
> > > > + T* data_;
> > > > + std::size len_;
> > > > + };
> > > > +
> > > > + is considered std::span-like. */
> > > > + if (NON_UNION_CLASS_TYPE_P (ctype)
> > > > + && CLASSTYPE_TEMPLATE_INSTANTIATION (ctype)
> > > > + && TYPE_HAS_TRIVIAL_DESTRUCTOR (ctype))
> > > > + for (tree field = next_aggregate_field (TYPE_FIELDS (ctype));
> > > > + field; field = next_aggregate_field (DECL_CHAIN (field)))
> > > > + if (TYPE_PTR_P (TREE_TYPE (field)))
> > > > + return true;
> > > > +
> > > > /* Some classes, such as std::tuple, have the reference member in its
> > > > (non-direct) base class. */
> > > > if (dfs_walk_once (TYPE_BINFO (ctype), class_has_reference_member_p_r,
> > > > diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> > > > index 6ec56493e59..e0ff18a86f5 100644
> > > > --- a/gcc/doc/invoke.texi
> > > > +++ b/gcc/doc/invoke.texi
> > > > @@ -3916,6 +3916,20 @@ 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 does not warn for @code{std::span}-like classes. We consider
> > > > +classes of the form:
> > > > +
> > > > +@smallexample
> > > > +template<typename T>
> > > > +struct Span @{
> > > > + T* data_;
> > > > + std::size len_;
> > > > +@};
> > > > +@end smallexample
> > > > +
> > > > +as @code{std::span}-like; that is, the class is a non-union class template
> > > > +that has a pointer data member and a trivial destructor.
> > > > +
> > > > This warning is enabled by @option{-Wall}.
> > > > @opindex Wdelete-non-virtual-dtor
> > > > diff --git a/gcc/testsuite/g++.dg/warn/Wdangling-reference18.C b/gcc/testsuite/g++.dg/warn/Wdangling-reference18.C
> > > > new file mode 100644
> > > > index 00000000000..e088c177769
> > > > --- /dev/null
> > > > +++ b/gcc/testsuite/g++.dg/warn/Wdangling-reference18.C
> > > > @@ -0,0 +1,24 @@
> > > > +// PR c++/110358
> > > > +// { dg-do compile { target c++11 } }
> > > > +// { dg-options "-Wdangling-reference" }
> > > > +// Don't warn for std::span-like classes.
> > > > +
> > > > +template <typename T>
> > > > +struct Span {
> > > > + T* data_;
> > > > + int len_;
> > > > +
> > > > + [[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 reference" }
> > > > + int const& b = get().back(); // { dg-bogus "dangling reference" }
> > > > + int const& c = get()[0]; // { dg-bogus "dangling reference" }
> > > > +
> > > > + return a + b + c;
> > > > +}
> > > > diff --git a/gcc/testsuite/g++.dg/warn/Wdangling-reference19.C b/gcc/testsuite/g++.dg/warn/Wdangling-reference19.C
> > > > new file mode 100644
> > > > index 00000000000..053467d822f
> > > > --- /dev/null
> > > > +++ b/gcc/testsuite/g++.dg/warn/Wdangling-reference19.C
> > > > @@ -0,0 +1,25 @@
> > > > +// PR c++/110358
> > > > +// { dg-do compile { target c++11 } }
> > > > +// { dg-options "-Wdangling-reference" }
> > > > +// Like Wdangling-reference18.C but not actually a span-like class.
> > > > +
> > > > +template <typename T>
> > > > +struct Span {
> > > > + T* data_;
> > > > + int len_;
> > > > + ~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-warning "dangling reference" }
> > > > + int const& b = get().back(); // { dg-warning "dangling reference" }
> > > > + int const& c = get()[0]; // { dg-warning "dangling reference" }
> > > > +
> > > > + return a + b + c;
> > > > +}
> > > > diff --git a/gcc/testsuite/g++.dg/warn/Wdangling-reference20.C b/gcc/testsuite/g++.dg/warn/Wdangling-reference20.C
> > > > new file mode 100644
> > > > index 00000000000..463c7380283
> > > > --- /dev/null
> > > > +++ b/gcc/testsuite/g++.dg/warn/Wdangling-reference20.C
> > > > @@ -0,0 +1,42 @@
> > > > +// PR c++/109640
> > > > +// { dg-do compile { target c++20 } }
> > > > +// { dg-options "-Wdangling-reference" }
> > > > +// Don't warn for std::span-like classes.
> > > > +
> > > > +#include <iterator>
> > > > +#include <span>
> > > > +
> > > > +template <typename T>
> > > > +struct MySpan
> > > > +{
> > > > + MySpan(T* data, std::size_t size) :
> > > > + data_(data),
> > > > + size_(size)
> > > > + {}
> > > > +
> > > > + T& operator[](std::size_t idx) { return data_[idx]; }
> > > > +
> > > > +private:
> > > > + T* data_;
> > > > + std::size_t size_;
> > > > +};
> > > > +
> > > > +template <typename T, std::size_t n>
> > > > +MySpan<T const> make_my_span(T const(&x)[n])
> > > > +{
> > > > + return MySpan(std::begin(x), n);
> > > > +}
> > > > +
> > > > +template <typename T, std::size_t n>
> > > > +std::span<T const> make_span(T const(&x)[n])
> > > > +{
> > > > + return std::span(std::begin(x), n);
> > > > +}
> > > > +
> > > > +int main()
> > > > +{
> > > > + int x[10]{};
> > > > + int const& y{make_my_span(x)[0]};
> > > > + int const& y2{make_span(x)[0]};
> > >
> > > Let's also test that we do warn if the argument to make_*span is a
> > > temporary. OK with that change, assuming it works as expected.
> >
> > We do warn then, I've added
> > using T = int[10];
> > [[maybe_unused]] int const& y3{make_my_span(T{})[0]};
> > [[maybe_unused]] int const& y4{make_span(T{})[0]};
> > and get two warnings. I'll push with that adjusted; thanks.
>
> It looks like this patch breaks bootstrap on aarch64-linux-gnu, I see
> the following building aarch64-early-ra.cc in stage2:
>
> /work/builds/bstrap/./prev-gcc/xg++ -B/work/builds/bstrap/./prev-gcc/ -B/work/builds/bstrap/aarch64-unknown-linux-gnu/bin/ -nostdinc++ -B/work/builds/bstrap/prev-aarch64-unknown-linux-gnu/libstdc++-v3/src/.libs -B/work/builds/bstrap/prev-aarch64-unknown-linux-gnu/libstdc++-v3/libsupc++/.libs -I/work/builds/bstrap/prev-aarch64-unknown-linux-gnu/libstdc++-v3/include/aarch64-unknown-linux-gnu -I/work/builds/bstrap/prev-aarch64-unknown-linux-gnu/libstdc++-v3/include -I/home/alecop01/toolchain/src/gcc/libstdc++-v3/libsupc++ -L/work/builds/bstrap/prev-aarch64-unknown-linux-gnu/libstdc++-v3/src/.libs -L/work/builds/bstrap/prev-aarch64-unknown-linux-gnu/libstdc++-v3/libsupc++/.libs -fno-PIE -c -g -O2 -fno-checking -gtoggle -DIN_GCC -fno-exceptions -fno-rtti -fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual -Wmissing-format-attribute -Wconditionally-supported -Woverloaded-virtual -pedantic -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -Werror -fno-common -DHAVE_CONFIG_H -fno-PIE -I. -I. -I/home/alecop01/toolchain/src/gcc/gcc -I/home/alecop01/toolchain/src/gcc/gcc/. -I/home/alecop01/toolchain/src/gcc/gcc/../include -I/home/alecop01/toolchain/src/gcc/gcc/../libcpp/include -I/home/alecop01/toolchain/src/gcc/gcc/../libcody -I/home/alecop01/toolchain/src/gcc/gcc/../libdecnumber -I/home/alecop01/toolchain/src/gcc/gcc/../libdecnumber/bid -I../libdecnumber -I/home/alecop01/toolchain/src/gcc/gcc/../libbacktrace -I. -I. -I/home/alecop01/toolchain/src/gcc/gcc -I/home/alecop01/toolchain/src/gcc/gcc/. -I/home/alecop01/toolchain/src/gcc/gcc/../include -I/home/alecop01/toolchain/src/gcc/gcc/../libcpp/include -I/home/alecop01/toolchain/src/gcc/gcc/../libcody -I/home/alecop01/toolchain/src/gcc/gcc/../libdecnumber -I/home/alecop01/toolchain/src/gcc/gcc/../libdecnumber/bid -I../libdecnumber -I/home/alecop01/toolchain/src/gcc/gcc/../libbacktrace \
> /home/alecop01/toolchain/src/gcc/gcc/config/aarch64/aarch64-early-ra.cc
> /home/alecop01/toolchain/src/gcc/gcc/config/aarch64/aarch64-early-ra.cc: In member function ‘void {anonymous}::early_ra::record_constraints(rtx_insn*)’:
> /home/alecop01/toolchain/src/gcc/gcc/config/aarch64/aarch64-early-ra.cc:1858:66: error: possibly dangling reference to a temporary [-Werror=dangling-reference]
> 1858 | for (auto &allocno : get_allocno_subgroup (op).allocnos ())
> | ^
> /home/alecop01/toolchain/src/gcc/gcc/config/aarch64/aarch64-early-ra.cc:1858:65: note: the temporary was destroyed at the end of the full expression ‘(({anonymous}::early_ra*)this)->{anonymous}::early_ra::get_allocno_subgroup(op).{anonymous}::early_ra::allocno_subgroup::allocnos()’
> 1858 | for (auto &allocno : get_allocno_subgroup (op).allocnos ())
> | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~
> cc1plus: all warnings being treated as errors
> make[3]: *** [/home/alecop01/toolchain/src/gcc/gcc/config/aarch64/t-aarch64:200: aarch64-early-ra.o] Error 1
> make[3]: Leaving directory '/work/builds/bstrap/gcc'
> make[2]: *** [Makefile:5096: all-stage2-gcc] Error 2
> make[2]: Leaving directory '/work/builds/bstrap'
> make[1]: *** [Makefile:25405: stage2-bubble] Error 2
> make[1]: Leaving directory '/work/builds/bstrap'
> make: *** [Makefile:1100: all] Error 2
Very sorry about that.
> Is the patch expected to introduce new warnings?
No, on the contrary, it was supposed to strictly reduce the # of warnings.
> I'll try to reduce a testcase from this where we don't see the warning
> before and we see the warning afterwards.
That would be great. I think the fix may be just removing one line.
Marek
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] c++: avoid -Wdangling-reference for std::span-like classes [PR110358]
2024-01-31 19:57 ` Jason Merrill
@ 2024-01-31 20:56 ` Marek Polacek
2024-02-01 2:52 ` Jason Merrill
0 siblings, 1 reply; 11+ messages in thread
From: Marek Polacek @ 2024-01-31 20:56 UTC (permalink / raw)
To: Jason Merrill; +Cc: Alex Coplan, GCC Patches
On Wed, Jan 31, 2024 at 02:57:09PM -0500, Jason Merrill wrote:
> On 1/31/24 14:44, Alex Coplan wrote:
> > Hi Marek,
> >
> > On 30/01/2024 13:15, Marek Polacek wrote:
> > > On Thu, Jan 25, 2024 at 10:13:10PM -0500, Jason Merrill wrote:
> > > > On 1/25/24 20:36, Marek Polacek wrote:
> > > > > Better version:
> > > > >
> > > > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> > > > >
> > > > > -- >8 --
> > > > > Real-world experience shows that -Wdangling-reference triggers for
> > > > > user-defined std::span-like classes a lot. We can easily avoid that
> > > > > by considering classes like
> > > > >
> > > > > template<typename T>
> > > > > struct Span {
> > > > > T* data_;
> > > > > std::size len_;
> > > > > };
> > > > >
> > > > > to be std::span-like, and not warning for them. Unlike the previous
> > > > > patch, this one considers a non-union class template that has a pointer
> > > > > data member and a trivial destructor as std::span-like.
> > > > >
> > > > > PR c++/110358
> > > > > PR c++/109640
> > > > >
> > > > > gcc/cp/ChangeLog:
> > > > >
> > > > > * call.cc (reference_like_class_p): Don't warn for std::span-like
> > > > > classes.
> > > > >
> > > > > gcc/ChangeLog:
> > > > >
> > > > > * doc/invoke.texi: Update -Wdangling-reference description.
> > > > >
> > > > > gcc/testsuite/ChangeLog:
> > > > >
> > > > > * g++.dg/warn/Wdangling-reference18.C: New test.
> > > > > * g++.dg/warn/Wdangling-reference19.C: New test.
> > > > > * g++.dg/warn/Wdangling-reference20.C: New test.
> > > > > ---
> > > > > gcc/cp/call.cc | 18 ++++++++
> > > > > gcc/doc/invoke.texi | 14 +++++++
> > > > > .../g++.dg/warn/Wdangling-reference18.C | 24 +++++++++++
> > > > > .../g++.dg/warn/Wdangling-reference19.C | 25 +++++++++++
> > > > > .../g++.dg/warn/Wdangling-reference20.C | 42 +++++++++++++++++++
> > > > > 5 files changed, 123 insertions(+)
> > > > > create mode 100644 gcc/testsuite/g++.dg/warn/Wdangling-reference18.C
> > > > > create mode 100644 gcc/testsuite/g++.dg/warn/Wdangling-reference19.C
> > > > > create mode 100644 gcc/testsuite/g++.dg/warn/Wdangling-reference20.C
> > > > >
> > > > > diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
> > > > > index 9de0d77c423..afd3e1ff024 100644
> > > > > --- a/gcc/cp/call.cc
> > > > > +++ b/gcc/cp/call.cc
> > > > > @@ -14082,6 +14082,24 @@ reference_like_class_p (tree ctype)
> > > > > return true;
> > > > > }
> > > > > + /* Avoid warning if CTYPE looks like std::span: it's a class template,
> > > > > + has a T* member, and a trivial destructor. For example,
> > > > > +
> > > > > + template<typename T>
> > > > > + struct Span {
> > > > > + T* data_;
> > > > > + std::size len_;
> > > > > + };
> > > > > +
> > > > > + is considered std::span-like. */
> > > > > + if (NON_UNION_CLASS_TYPE_P (ctype)
> > > > > + && CLASSTYPE_TEMPLATE_INSTANTIATION (ctype)
> > > > > + && TYPE_HAS_TRIVIAL_DESTRUCTOR (ctype))
> > > > > + for (tree field = next_aggregate_field (TYPE_FIELDS (ctype));
> > > > > + field; field = next_aggregate_field (DECL_CHAIN (field)))
> > > > > + if (TYPE_PTR_P (TREE_TYPE (field)))
> > > > > + return true;
> > > > > +
> > > > > /* Some classes, such as std::tuple, have the reference member in its
> > > > > (non-direct) base class. */
> > > > > if (dfs_walk_once (TYPE_BINFO (ctype), class_has_reference_member_p_r,
> > > > > diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> > > > > index 6ec56493e59..e0ff18a86f5 100644
> > > > > --- a/gcc/doc/invoke.texi
> > > > > +++ b/gcc/doc/invoke.texi
> > > > > @@ -3916,6 +3916,20 @@ 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 does not warn for @code{std::span}-like classes. We consider
> > > > > +classes of the form:
> > > > > +
> > > > > +@smallexample
> > > > > +template<typename T>
> > > > > +struct Span @{
> > > > > + T* data_;
> > > > > + std::size len_;
> > > > > +@};
> > > > > +@end smallexample
> > > > > +
> > > > > +as @code{std::span}-like; that is, the class is a non-union class template
> > > > > +that has a pointer data member and a trivial destructor.
> > > > > +
> > > > > This warning is enabled by @option{-Wall}.
> > > > > @opindex Wdelete-non-virtual-dtor
> > > > > diff --git a/gcc/testsuite/g++.dg/warn/Wdangling-reference18.C b/gcc/testsuite/g++.dg/warn/Wdangling-reference18.C
> > > > > new file mode 100644
> > > > > index 00000000000..e088c177769
> > > > > --- /dev/null
> > > > > +++ b/gcc/testsuite/g++.dg/warn/Wdangling-reference18.C
> > > > > @@ -0,0 +1,24 @@
> > > > > +// PR c++/110358
> > > > > +// { dg-do compile { target c++11 } }
> > > > > +// { dg-options "-Wdangling-reference" }
> > > > > +// Don't warn for std::span-like classes.
> > > > > +
> > > > > +template <typename T>
> > > > > +struct Span {
> > > > > + T* data_;
> > > > > + int len_;
> > > > > +
> > > > > + [[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 reference" }
> > > > > + int const& b = get().back(); // { dg-bogus "dangling reference" }
> > > > > + int const& c = get()[0]; // { dg-bogus "dangling reference" }
> > > > > +
> > > > > + return a + b + c;
> > > > > +}
> > > > > diff --git a/gcc/testsuite/g++.dg/warn/Wdangling-reference19.C b/gcc/testsuite/g++.dg/warn/Wdangling-reference19.C
> > > > > new file mode 100644
> > > > > index 00000000000..053467d822f
> > > > > --- /dev/null
> > > > > +++ b/gcc/testsuite/g++.dg/warn/Wdangling-reference19.C
> > > > > @@ -0,0 +1,25 @@
> > > > > +// PR c++/110358
> > > > > +// { dg-do compile { target c++11 } }
> > > > > +// { dg-options "-Wdangling-reference" }
> > > > > +// Like Wdangling-reference18.C but not actually a span-like class.
> > > > > +
> > > > > +template <typename T>
> > > > > +struct Span {
> > > > > + T* data_;
> > > > > + int len_;
> > > > > + ~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-warning "dangling reference" }
> > > > > + int const& b = get().back(); // { dg-warning "dangling reference" }
> > > > > + int const& c = get()[0]; // { dg-warning "dangling reference" }
> > > > > +
> > > > > + return a + b + c;
> > > > > +}
> > > > > diff --git a/gcc/testsuite/g++.dg/warn/Wdangling-reference20.C b/gcc/testsuite/g++.dg/warn/Wdangling-reference20.C
> > > > > new file mode 100644
> > > > > index 00000000000..463c7380283
> > > > > --- /dev/null
> > > > > +++ b/gcc/testsuite/g++.dg/warn/Wdangling-reference20.C
> > > > > @@ -0,0 +1,42 @@
> > > > > +// PR c++/109640
> > > > > +// { dg-do compile { target c++20 } }
> > > > > +// { dg-options "-Wdangling-reference" }
> > > > > +// Don't warn for std::span-like classes.
> > > > > +
> > > > > +#include <iterator>
> > > > > +#include <span>
> > > > > +
> > > > > +template <typename T>
> > > > > +struct MySpan
> > > > > +{
> > > > > + MySpan(T* data, std::size_t size) :
> > > > > + data_(data),
> > > > > + size_(size)
> > > > > + {}
> > > > > +
> > > > > + T& operator[](std::size_t idx) { return data_[idx]; }
> > > > > +
> > > > > +private:
> > > > > + T* data_;
> > > > > + std::size_t size_;
> > > > > +};
> > > > > +
> > > > > +template <typename T, std::size_t n>
> > > > > +MySpan<T const> make_my_span(T const(&x)[n])
> > > > > +{
> > > > > + return MySpan(std::begin(x), n);
> > > > > +}
> > > > > +
> > > > > +template <typename T, std::size_t n>
> > > > > +std::span<T const> make_span(T const(&x)[n])
> > > > > +{
> > > > > + return std::span(std::begin(x), n);
> > > > > +}
> > > > > +
> > > > > +int main()
> > > > > +{
> > > > > + int x[10]{};
> > > > > + int const& y{make_my_span(x)[0]};
> > > > > + int const& y2{make_span(x)[0]};
> > > >
> > > > Let's also test that we do warn if the argument to make_*span is a
> > > > temporary. OK with that change, assuming it works as expected.
> > >
> > > We do warn then, I've added
> > > using T = int[10];
> > > [[maybe_unused]] int const& y3{make_my_span(T{})[0]};
> > > [[maybe_unused]] int const& y4{make_span(T{})[0]};
> > > and get two warnings. I'll push with that adjusted; thanks.
> >
> > It looks like this patch breaks bootstrap on aarch64-linux-gnu, I see
> > the following building aarch64-early-ra.cc in stage2:
> >
> > /work/builds/bstrap/./prev-gcc/xg++ -B/work/builds/bstrap/./prev-gcc/ -B/work/builds/bstrap/aarch64-unknown-linux-gnu/bin/ -nostdinc++ -B/work/builds/bstrap/prev-aarch64-unknown-linux-gnu/libstdc++-v3/src/.libs -B/work/builds/bstrap/prev-aarch64-unknown-linux-gnu/libstdc++-v3/libsupc++/.libs -I/work/builds/bstrap/prev-aarch64-unknown-linux-gnu/libstdc++-v3/include/aarch64-unknown-linux-gnu -I/work/builds/bstrap/prev-aarch64-unknown-linux-gnu/libstdc++-v3/include -I/home/alecop01/toolchain/src/gcc/libstdc++-v3/libsupc++ -L/work/builds/bstrap/prev-aarch64-unknown-linux-gnu/libstdc++-v3/src/.libs -L/work/builds/bstrap/prev-aarch64-unknown-linux-gnu/libstdc++-v3/libsupc++/.libs -fno-PIE -c -g -O2 -fno-checking -gtoggle -DIN_GCC -fno-exceptions -fno-rtti -fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual -Wmissing-format-attribute -Wconditionally-supported -Woverloaded-virtual -pedantic -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -Werror -fno-common -DHAVE_CONFIG_H -fno-PIE -I. -I. -I/home/alecop01/toolchain/src/gcc/gcc -I/home/alecop01/toolchain/src/gcc/gcc/. -I/home/alecop01/toolchain/src/gcc/gcc/../include -I/home/alecop01/toolchain/src/gcc/gcc/../libcpp/include -I/home/alecop01/toolchain/src/gcc/gcc/../libcody -I/home/alecop01/toolchain/src/gcc/gcc/../libdecnumber -I/home/alecop01/toolchain/src/gcc/gcc/../libdecnumber/bid -I../libdecnumber -I/home/alecop01/toolchain/src/gcc/gcc/../libbacktrace -I. -I. -I/home/alecop01/toolchain/src/gcc/gcc -I/home/alecop01/toolchain/src/gcc/gcc/. -I/home/alecop01/toolchain/src/gcc/gcc/../include -I/home/alecop01/toolchain/src/gcc/gcc/../libcpp/include -I/home/alecop01/toolchain/src/gcc/gcc/../libcody -I/home/alecop01/toolchain/src/gcc/gcc/../libdecnumber -I/home/alecop01/toolchain/src/gcc/gcc/../libdecnumber/bid -I../libdecnumber -I/home/alecop01/toolchain/src/gcc/gcc/../libbacktrace \
> > /home/alecop01/toolchain/src/gcc/gcc/config/aarch64/aarch64-early-ra.cc
> > /home/alecop01/toolchain/src/gcc/gcc/config/aarch64/aarch64-early-ra.cc: In member function ‘void {anonymous}::early_ra::record_constraints(rtx_insn*)’:
> > /home/alecop01/toolchain/src/gcc/gcc/config/aarch64/aarch64-early-ra.cc:1858:66: error: possibly dangling reference to a temporary [-Werror=dangling-reference]
> > 1858 | for (auto &allocno : get_allocno_subgroup (op).allocnos ())
> > | ^
> > /home/alecop01/toolchain/src/gcc/gcc/config/aarch64/aarch64-early-ra.cc:1858:65: note: the temporary was destroyed at the end of the full expression ‘(({anonymous}::early_ra*)this)->{anonymous}::early_ra::get_allocno_subgroup(op).{anonymous}::early_ra::allocno_subgroup::allocnos()’
> > 1858 | for (auto &allocno : get_allocno_subgroup (op).allocnos ())
> > | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~
>
> IMO it's good that we now recognize array_slice (returned from allocnos) as
> a span-like class, but we should also recognize allocno_subgroup as one and
> avoid warning.
So we're talking about
struct allocno_subgroup
{
array_slice<allocno_info> allocnos ();
allocno_info *allocno (unsigned int);
// True if a subgroup is present.
operator bool () const { return count; }
// The containing group.
allocno_group_info *group;
// The offset of the subgroup from the start of GROUP.
unsigned int start;
// The number of allocnos in the subgroup.
unsigned int count;
};
which has a pointer member and a trivial dtor, but is not a template,
therefore not recognized as std::span-like. Did you want me to drop the
CLASSTYPE_TEMPLATE_INSTANTIATION check?
Marek
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] c++: avoid -Wdangling-reference for std::span-like classes [PR110358]
2024-01-31 20:56 ` Marek Polacek
@ 2024-02-01 2:52 ` Jason Merrill
0 siblings, 0 replies; 11+ messages in thread
From: Jason Merrill @ 2024-02-01 2:52 UTC (permalink / raw)
To: Marek Polacek; +Cc: Alex Coplan, GCC Patches
On 1/31/24 15:56, Marek Polacek wrote:
> On Wed, Jan 31, 2024 at 02:57:09PM -0500, Jason Merrill wrote:
>> On 1/31/24 14:44, Alex Coplan wrote:
>>> Hi Marek,
>>>
>>> On 30/01/2024 13:15, Marek Polacek wrote:
>>>> On Thu, Jan 25, 2024 at 10:13:10PM -0500, Jason Merrill wrote:
>>>>> On 1/25/24 20:36, Marek Polacek wrote:
>>>>>> Better version:
>>>>>>
>>>>>> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
>>>>>>
>>>>>> -- >8 --
>>>>>> Real-world experience shows that -Wdangling-reference triggers for
>>>>>> user-defined std::span-like classes a lot. We can easily avoid that
>>>>>> by considering classes like
>>>>>>
>>>>>> template<typename T>
>>>>>> struct Span {
>>>>>> T* data_;
>>>>>> std::size len_;
>>>>>> };
>>>>>>
>>>>>> to be std::span-like, and not warning for them. Unlike the previous
>>>>>> patch, this one considers a non-union class template that has a pointer
>>>>>> data member and a trivial destructor as std::span-like.
>>>>>>
>>>>>> PR c++/110358
>>>>>> PR c++/109640
>>>>>>
>>>>>> gcc/cp/ChangeLog:
>>>>>>
>>>>>> * call.cc (reference_like_class_p): Don't warn for std::span-like
>>>>>> classes.
>>>>>>
>>>>>> gcc/ChangeLog:
>>>>>>
>>>>>> * doc/invoke.texi: Update -Wdangling-reference description.
>>>>>>
>>>>>> gcc/testsuite/ChangeLog:
>>>>>>
>>>>>> * g++.dg/warn/Wdangling-reference18.C: New test.
>>>>>> * g++.dg/warn/Wdangling-reference19.C: New test.
>>>>>> * g++.dg/warn/Wdangling-reference20.C: New test.
>>>>>> ---
>>>>>> gcc/cp/call.cc | 18 ++++++++
>>>>>> gcc/doc/invoke.texi | 14 +++++++
>>>>>> .../g++.dg/warn/Wdangling-reference18.C | 24 +++++++++++
>>>>>> .../g++.dg/warn/Wdangling-reference19.C | 25 +++++++++++
>>>>>> .../g++.dg/warn/Wdangling-reference20.C | 42 +++++++++++++++++++
>>>>>> 5 files changed, 123 insertions(+)
>>>>>> create mode 100644 gcc/testsuite/g++.dg/warn/Wdangling-reference18.C
>>>>>> create mode 100644 gcc/testsuite/g++.dg/warn/Wdangling-reference19.C
>>>>>> create mode 100644 gcc/testsuite/g++.dg/warn/Wdangling-reference20.C
>>>>>>
>>>>>> diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
>>>>>> index 9de0d77c423..afd3e1ff024 100644
>>>>>> --- a/gcc/cp/call.cc
>>>>>> +++ b/gcc/cp/call.cc
>>>>>> @@ -14082,6 +14082,24 @@ reference_like_class_p (tree ctype)
>>>>>> return true;
>>>>>> }
>>>>>> + /* Avoid warning if CTYPE looks like std::span: it's a class template,
>>>>>> + has a T* member, and a trivial destructor. For example,
>>>>>> +
>>>>>> + template<typename T>
>>>>>> + struct Span {
>>>>>> + T* data_;
>>>>>> + std::size len_;
>>>>>> + };
>>>>>> +
>>>>>> + is considered std::span-like. */
>>>>>> + if (NON_UNION_CLASS_TYPE_P (ctype)
>>>>>> + && CLASSTYPE_TEMPLATE_INSTANTIATION (ctype)
>>>>>> + && TYPE_HAS_TRIVIAL_DESTRUCTOR (ctype))
>>>>>> + for (tree field = next_aggregate_field (TYPE_FIELDS (ctype));
>>>>>> + field; field = next_aggregate_field (DECL_CHAIN (field)))
>>>>>> + if (TYPE_PTR_P (TREE_TYPE (field)))
>>>>>> + return true;
>>>>>> +
>>>>>> /* Some classes, such as std::tuple, have the reference member in its
>>>>>> (non-direct) base class. */
>>>>>> if (dfs_walk_once (TYPE_BINFO (ctype), class_has_reference_member_p_r,
>>>>>> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
>>>>>> index 6ec56493e59..e0ff18a86f5 100644
>>>>>> --- a/gcc/doc/invoke.texi
>>>>>> +++ b/gcc/doc/invoke.texi
>>>>>> @@ -3916,6 +3916,20 @@ 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 does not warn for @code{std::span}-like classes. We consider
>>>>>> +classes of the form:
>>>>>> +
>>>>>> +@smallexample
>>>>>> +template<typename T>
>>>>>> +struct Span @{
>>>>>> + T* data_;
>>>>>> + std::size len_;
>>>>>> +@};
>>>>>> +@end smallexample
>>>>>> +
>>>>>> +as @code{std::span}-like; that is, the class is a non-union class template
>>>>>> +that has a pointer data member and a trivial destructor.
>>>>>> +
>>>>>> This warning is enabled by @option{-Wall}.
>>>>>> @opindex Wdelete-non-virtual-dtor
>>>>>> diff --git a/gcc/testsuite/g++.dg/warn/Wdangling-reference18.C b/gcc/testsuite/g++.dg/warn/Wdangling-reference18.C
>>>>>> new file mode 100644
>>>>>> index 00000000000..e088c177769
>>>>>> --- /dev/null
>>>>>> +++ b/gcc/testsuite/g++.dg/warn/Wdangling-reference18.C
>>>>>> @@ -0,0 +1,24 @@
>>>>>> +// PR c++/110358
>>>>>> +// { dg-do compile { target c++11 } }
>>>>>> +// { dg-options "-Wdangling-reference" }
>>>>>> +// Don't warn for std::span-like classes.
>>>>>> +
>>>>>> +template <typename T>
>>>>>> +struct Span {
>>>>>> + T* data_;
>>>>>> + int len_;
>>>>>> +
>>>>>> + [[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 reference" }
>>>>>> + int const& b = get().back(); // { dg-bogus "dangling reference" }
>>>>>> + int const& c = get()[0]; // { dg-bogus "dangling reference" }
>>>>>> +
>>>>>> + return a + b + c;
>>>>>> +}
>>>>>> diff --git a/gcc/testsuite/g++.dg/warn/Wdangling-reference19.C b/gcc/testsuite/g++.dg/warn/Wdangling-reference19.C
>>>>>> new file mode 100644
>>>>>> index 00000000000..053467d822f
>>>>>> --- /dev/null
>>>>>> +++ b/gcc/testsuite/g++.dg/warn/Wdangling-reference19.C
>>>>>> @@ -0,0 +1,25 @@
>>>>>> +// PR c++/110358
>>>>>> +// { dg-do compile { target c++11 } }
>>>>>> +// { dg-options "-Wdangling-reference" }
>>>>>> +// Like Wdangling-reference18.C but not actually a span-like class.
>>>>>> +
>>>>>> +template <typename T>
>>>>>> +struct Span {
>>>>>> + T* data_;
>>>>>> + int len_;
>>>>>> + ~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-warning "dangling reference" }
>>>>>> + int const& b = get().back(); // { dg-warning "dangling reference" }
>>>>>> + int const& c = get()[0]; // { dg-warning "dangling reference" }
>>>>>> +
>>>>>> + return a + b + c;
>>>>>> +}
>>>>>> diff --git a/gcc/testsuite/g++.dg/warn/Wdangling-reference20.C b/gcc/testsuite/g++.dg/warn/Wdangling-reference20.C
>>>>>> new file mode 100644
>>>>>> index 00000000000..463c7380283
>>>>>> --- /dev/null
>>>>>> +++ b/gcc/testsuite/g++.dg/warn/Wdangling-reference20.C
>>>>>> @@ -0,0 +1,42 @@
>>>>>> +// PR c++/109640
>>>>>> +// { dg-do compile { target c++20 } }
>>>>>> +// { dg-options "-Wdangling-reference" }
>>>>>> +// Don't warn for std::span-like classes.
>>>>>> +
>>>>>> +#include <iterator>
>>>>>> +#include <span>
>>>>>> +
>>>>>> +template <typename T>
>>>>>> +struct MySpan
>>>>>> +{
>>>>>> + MySpan(T* data, std::size_t size) :
>>>>>> + data_(data),
>>>>>> + size_(size)
>>>>>> + {}
>>>>>> +
>>>>>> + T& operator[](std::size_t idx) { return data_[idx]; }
>>>>>> +
>>>>>> +private:
>>>>>> + T* data_;
>>>>>> + std::size_t size_;
>>>>>> +};
>>>>>> +
>>>>>> +template <typename T, std::size_t n>
>>>>>> +MySpan<T const> make_my_span(T const(&x)[n])
>>>>>> +{
>>>>>> + return MySpan(std::begin(x), n);
>>>>>> +}
>>>>>> +
>>>>>> +template <typename T, std::size_t n>
>>>>>> +std::span<T const> make_span(T const(&x)[n])
>>>>>> +{
>>>>>> + return std::span(std::begin(x), n);
>>>>>> +}
>>>>>> +
>>>>>> +int main()
>>>>>> +{
>>>>>> + int x[10]{};
>>>>>> + int const& y{make_my_span(x)[0]};
>>>>>> + int const& y2{make_span(x)[0]};
>>>>>
>>>>> Let's also test that we do warn if the argument to make_*span is a
>>>>> temporary. OK with that change, assuming it works as expected.
>>>>
>>>> We do warn then, I've added
>>>> using T = int[10];
>>>> [[maybe_unused]] int const& y3{make_my_span(T{})[0]};
>>>> [[maybe_unused]] int const& y4{make_span(T{})[0]};
>>>> and get two warnings. I'll push with that adjusted; thanks.
>>>
>>> It looks like this patch breaks bootstrap on aarch64-linux-gnu, I see
>>> the following building aarch64-early-ra.cc in stage2:
>>>
>>> /work/builds/bstrap/./prev-gcc/xg++ -B/work/builds/bstrap/./prev-gcc/ -B/work/builds/bstrap/aarch64-unknown-linux-gnu/bin/ -nostdinc++ -B/work/builds/bstrap/prev-aarch64-unknown-linux-gnu/libstdc++-v3/src/.libs -B/work/builds/bstrap/prev-aarch64-unknown-linux-gnu/libstdc++-v3/libsupc++/.libs -I/work/builds/bstrap/prev-aarch64-unknown-linux-gnu/libstdc++-v3/include/aarch64-unknown-linux-gnu -I/work/builds/bstrap/prev-aarch64-unknown-linux-gnu/libstdc++-v3/include -I/home/alecop01/toolchain/src/gcc/libstdc++-v3/libsupc++ -L/work/builds/bstrap/prev-aarch64-unknown-linux-gnu/libstdc++-v3/src/.libs -L/work/builds/bstrap/prev-aarch64-unknown-linux-gnu/libstdc++-v3/libsupc++/.libs -fno-PIE -c -g -O2 -fno-checking -gtoggle -DIN_GCC -fno-exceptions -fno-rtti -fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual -Wmissing-format-attribute -Wconditionally-supported -Woverloaded-virtual -pedantic -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -Werror -fno-common -DHAVE_CONFIG_H -fno-PIE -I. -I. -I/home/alecop01/toolchain/src/gcc/gcc -I/home/alecop01/toolchain/src/gcc/gcc/. -I/home/alecop01/toolchain/src/gcc/gcc/../include -I/home/alecop01/toolchain/src/gcc/gcc/../libcpp/include -I/home/alecop01/toolchain/src/gcc/gcc/../libcody -I/home/alecop01/toolchain/src/gcc/gcc/../libdecnumber -I/home/alecop01/toolchain/src/gcc/gcc/../libdecnumber/bid -I../libdecnumber -I/home/alecop01/toolchain/src/gcc/gcc/../libbacktrace -I. -I. -I/home/alecop01/toolchain/src/gcc/gcc -I/home/alecop01/toolchain/src/gcc/gcc/. -I/home/alecop01/toolchain/src/gcc/gcc/../include -I/home/alecop01/toolchain/src/gcc/gcc/../libcpp/include -I/home/alecop01/toolchain/src/gcc/gcc/../libcody -I/home/alecop01/toolchain/src/gcc/gcc/../libdecnumber -I/home/alecop01/toolchain/src/gcc/gcc/../libdecnumber/bid -I../libdecnumber -I/home/alecop01/toolchain/src/gcc/gcc/../libbacktrace \
>>> /home/alecop01/toolchain/src/gcc/gcc/config/aarch64/aarch64-early-ra.cc
>>> /home/alecop01/toolchain/src/gcc/gcc/config/aarch64/aarch64-early-ra.cc: In member function ‘void {anonymous}::early_ra::record_constraints(rtx_insn*)’:
>>> /home/alecop01/toolchain/src/gcc/gcc/config/aarch64/aarch64-early-ra.cc:1858:66: error: possibly dangling reference to a temporary [-Werror=dangling-reference]
>>> 1858 | for (auto &allocno : get_allocno_subgroup (op).allocnos ())
>>> | ^
>>> /home/alecop01/toolchain/src/gcc/gcc/config/aarch64/aarch64-early-ra.cc:1858:65: note: the temporary was destroyed at the end of the full expression ‘(({anonymous}::early_ra*)this)->{anonymous}::early_ra::get_allocno_subgroup(op).{anonymous}::early_ra::allocno_subgroup::allocnos()’
>>> 1858 | for (auto &allocno : get_allocno_subgroup (op).allocnos ())
>>> | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~
>>
>> IMO it's good that we now recognize array_slice (returned from allocnos) as
>> a span-like class, but we should also recognize allocno_subgroup as one and
>> avoid warning.
>
> So we're talking about
>
> struct allocno_subgroup
> {
> array_slice<allocno_info> allocnos ();
> allocno_info *allocno (unsigned int);
>
> // True if a subgroup is present.
> operator bool () const { return count; }
>
> // The containing group.
> allocno_group_info *group;
>
> // The offset of the subgroup from the start of GROUP.
> unsigned int start;
>
> // The number of allocnos in the subgroup.
> unsigned int count;
> };
>
> which has a pointer member and a trivial dtor, but is not a template,
> therefore not recognized as std::span-like. Did you want me to drop the
> CLASSTYPE_TEMPLATE_INSTANTIATION check?
That seems like what we want, yes.
Jason
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] c++: avoid -Wdangling-reference for std::span-like classes [PR110358]
2024-01-31 20:53 ` Marek Polacek
@ 2024-02-01 14:32 ` Alex Coplan
2024-02-01 18:05 ` Marek Polacek
0 siblings, 1 reply; 11+ messages in thread
From: Alex Coplan @ 2024-02-01 14:32 UTC (permalink / raw)
To: Marek Polacek; +Cc: Jason Merrill, GCC Patches
On 31/01/2024 15:53, Marek Polacek wrote:
> On Wed, Jan 31, 2024 at 07:44:41PM +0000, Alex Coplan wrote:
> > Hi Marek,
> >
> > On 30/01/2024 13:15, Marek Polacek wrote:
> > > On Thu, Jan 25, 2024 at 10:13:10PM -0500, Jason Merrill wrote:
> > > > On 1/25/24 20:36, Marek Polacek wrote:
> > > > > Better version:
> > > > >
> > > > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> > > > >
> > > > > -- >8 --
> > > > > Real-world experience shows that -Wdangling-reference triggers for
> > > > > user-defined std::span-like classes a lot. We can easily avoid that
> > > > > by considering classes like
> > > > >
> > > > > template<typename T>
> > > > > struct Span {
> > > > > T* data_;
> > > > > std::size len_;
> > > > > };
> > > > >
> > > > > to be std::span-like, and not warning for them. Unlike the previous
> > > > > patch, this one considers a non-union class template that has a pointer
> > > > > data member and a trivial destructor as std::span-like.
> > > > >
> > > > > PR c++/110358
> > > > > PR c++/109640
> > > > >
> > > > > gcc/cp/ChangeLog:
> > > > >
> > > > > * call.cc (reference_like_class_p): Don't warn for std::span-like
> > > > > classes.
> > > > >
> > > > > gcc/ChangeLog:
> > > > >
> > > > > * doc/invoke.texi: Update -Wdangling-reference description.
> > > > >
> > > > > gcc/testsuite/ChangeLog:
> > > > >
> > > > > * g++.dg/warn/Wdangling-reference18.C: New test.
> > > > > * g++.dg/warn/Wdangling-reference19.C: New test.
> > > > > * g++.dg/warn/Wdangling-reference20.C: New test.
> > > > > ---
> > > > > gcc/cp/call.cc | 18 ++++++++
> > > > > gcc/doc/invoke.texi | 14 +++++++
> > > > > .../g++.dg/warn/Wdangling-reference18.C | 24 +++++++++++
> > > > > .../g++.dg/warn/Wdangling-reference19.C | 25 +++++++++++
> > > > > .../g++.dg/warn/Wdangling-reference20.C | 42 +++++++++++++++++++
> > > > > 5 files changed, 123 insertions(+)
> > > > > create mode 100644 gcc/testsuite/g++.dg/warn/Wdangling-reference18.C
> > > > > create mode 100644 gcc/testsuite/g++.dg/warn/Wdangling-reference19.C
> > > > > create mode 100644 gcc/testsuite/g++.dg/warn/Wdangling-reference20.C
> > > > >
> > > > > diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
> > > > > index 9de0d77c423..afd3e1ff024 100644
> > > > > --- a/gcc/cp/call.cc
> > > > > +++ b/gcc/cp/call.cc
> > > > > @@ -14082,6 +14082,24 @@ reference_like_class_p (tree ctype)
> > > > > return true;
> > > > > }
> > > > > + /* Avoid warning if CTYPE looks like std::span: it's a class template,
> > > > > + has a T* member, and a trivial destructor. For example,
> > > > > +
> > > > > + template<typename T>
> > > > > + struct Span {
> > > > > + T* data_;
> > > > > + std::size len_;
> > > > > + };
> > > > > +
> > > > > + is considered std::span-like. */
> > > > > + if (NON_UNION_CLASS_TYPE_P (ctype)
> > > > > + && CLASSTYPE_TEMPLATE_INSTANTIATION (ctype)
> > > > > + && TYPE_HAS_TRIVIAL_DESTRUCTOR (ctype))
> > > > > + for (tree field = next_aggregate_field (TYPE_FIELDS (ctype));
> > > > > + field; field = next_aggregate_field (DECL_CHAIN (field)))
> > > > > + if (TYPE_PTR_P (TREE_TYPE (field)))
> > > > > + return true;
> > > > > +
> > > > > /* Some classes, such as std::tuple, have the reference member in its
> > > > > (non-direct) base class. */
> > > > > if (dfs_walk_once (TYPE_BINFO (ctype), class_has_reference_member_p_r,
> > > > > diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> > > > > index 6ec56493e59..e0ff18a86f5 100644
> > > > > --- a/gcc/doc/invoke.texi
> > > > > +++ b/gcc/doc/invoke.texi
> > > > > @@ -3916,6 +3916,20 @@ 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 does not warn for @code{std::span}-like classes. We consider
> > > > > +classes of the form:
> > > > > +
> > > > > +@smallexample
> > > > > +template<typename T>
> > > > > +struct Span @{
> > > > > + T* data_;
> > > > > + std::size len_;
> > > > > +@};
> > > > > +@end smallexample
> > > > > +
> > > > > +as @code{std::span}-like; that is, the class is a non-union class template
> > > > > +that has a pointer data member and a trivial destructor.
> > > > > +
> > > > > This warning is enabled by @option{-Wall}.
> > > > > @opindex Wdelete-non-virtual-dtor
> > > > > diff --git a/gcc/testsuite/g++.dg/warn/Wdangling-reference18.C b/gcc/testsuite/g++.dg/warn/Wdangling-reference18.C
> > > > > new file mode 100644
> > > > > index 00000000000..e088c177769
> > > > > --- /dev/null
> > > > > +++ b/gcc/testsuite/g++.dg/warn/Wdangling-reference18.C
> > > > > @@ -0,0 +1,24 @@
> > > > > +// PR c++/110358
> > > > > +// { dg-do compile { target c++11 } }
> > > > > +// { dg-options "-Wdangling-reference" }
> > > > > +// Don't warn for std::span-like classes.
> > > > > +
> > > > > +template <typename T>
> > > > > +struct Span {
> > > > > + T* data_;
> > > > > + int len_;
> > > > > +
> > > > > + [[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 reference" }
> > > > > + int const& b = get().back(); // { dg-bogus "dangling reference" }
> > > > > + int const& c = get()[0]; // { dg-bogus "dangling reference" }
> > > > > +
> > > > > + return a + b + c;
> > > > > +}
> > > > > diff --git a/gcc/testsuite/g++.dg/warn/Wdangling-reference19.C b/gcc/testsuite/g++.dg/warn/Wdangling-reference19.C
> > > > > new file mode 100644
> > > > > index 00000000000..053467d822f
> > > > > --- /dev/null
> > > > > +++ b/gcc/testsuite/g++.dg/warn/Wdangling-reference19.C
> > > > > @@ -0,0 +1,25 @@
> > > > > +// PR c++/110358
> > > > > +// { dg-do compile { target c++11 } }
> > > > > +// { dg-options "-Wdangling-reference" }
> > > > > +// Like Wdangling-reference18.C but not actually a span-like class.
> > > > > +
> > > > > +template <typename T>
> > > > > +struct Span {
> > > > > + T* data_;
> > > > > + int len_;
> > > > > + ~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-warning "dangling reference" }
> > > > > + int const& b = get().back(); // { dg-warning "dangling reference" }
> > > > > + int const& c = get()[0]; // { dg-warning "dangling reference" }
> > > > > +
> > > > > + return a + b + c;
> > > > > +}
> > > > > diff --git a/gcc/testsuite/g++.dg/warn/Wdangling-reference20.C b/gcc/testsuite/g++.dg/warn/Wdangling-reference20.C
> > > > > new file mode 100644
> > > > > index 00000000000..463c7380283
> > > > > --- /dev/null
> > > > > +++ b/gcc/testsuite/g++.dg/warn/Wdangling-reference20.C
> > > > > @@ -0,0 +1,42 @@
> > > > > +// PR c++/109640
> > > > > +// { dg-do compile { target c++20 } }
> > > > > +// { dg-options "-Wdangling-reference" }
> > > > > +// Don't warn for std::span-like classes.
> > > > > +
> > > > > +#include <iterator>
> > > > > +#include <span>
> > > > > +
> > > > > +template <typename T>
> > > > > +struct MySpan
> > > > > +{
> > > > > + MySpan(T* data, std::size_t size) :
> > > > > + data_(data),
> > > > > + size_(size)
> > > > > + {}
> > > > > +
> > > > > + T& operator[](std::size_t idx) { return data_[idx]; }
> > > > > +
> > > > > +private:
> > > > > + T* data_;
> > > > > + std::size_t size_;
> > > > > +};
> > > > > +
> > > > > +template <typename T, std::size_t n>
> > > > > +MySpan<T const> make_my_span(T const(&x)[n])
> > > > > +{
> > > > > + return MySpan(std::begin(x), n);
> > > > > +}
> > > > > +
> > > > > +template <typename T, std::size_t n>
> > > > > +std::span<T const> make_span(T const(&x)[n])
> > > > > +{
> > > > > + return std::span(std::begin(x), n);
> > > > > +}
> > > > > +
> > > > > +int main()
> > > > > +{
> > > > > + int x[10]{};
> > > > > + int const& y{make_my_span(x)[0]};
> > > > > + int const& y2{make_span(x)[0]};
> > > >
> > > > Let's also test that we do warn if the argument to make_*span is a
> > > > temporary. OK with that change, assuming it works as expected.
> > >
> > > We do warn then, I've added
> > > using T = int[10];
> > > [[maybe_unused]] int const& y3{make_my_span(T{})[0]};
> > > [[maybe_unused]] int const& y4{make_span(T{})[0]};
> > > and get two warnings. I'll push with that adjusted; thanks.
> >
> > It looks like this patch breaks bootstrap on aarch64-linux-gnu, I see
> > the following building aarch64-early-ra.cc in stage2:
> >
> > /work/builds/bstrap/./prev-gcc/xg++ -B/work/builds/bstrap/./prev-gcc/ -B/work/builds/bstrap/aarch64-unknown-linux-gnu/bin/ -nostdinc++ -B/work/builds/bstrap/prev-aarch64-unknown-linux-gnu/libstdc++-v3/src/.libs -B/work/builds/bstrap/prev-aarch64-unknown-linux-gnu/libstdc++-v3/libsupc++/.libs -I/work/builds/bstrap/prev-aarch64-unknown-linux-gnu/libstdc++-v3/include/aarch64-unknown-linux-gnu -I/work/builds/bstrap/prev-aarch64-unknown-linux-gnu/libstdc++-v3/include -I/home/alecop01/toolchain/src/gcc/libstdc++-v3/libsupc++ -L/work/builds/bstrap/prev-aarch64-unknown-linux-gnu/libstdc++-v3/src/.libs -L/work/builds/bstrap/prev-aarch64-unknown-linux-gnu/libstdc++-v3/libsupc++/.libs -fno-PIE -c -g -O2 -fno-checking -gtoggle -DIN_GCC -fno-exceptions -fno-rtti -fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual -Wmissing-format-attribute -Wconditionally-supported -Woverloaded-virtual -pedantic -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -Werror -fno-common -DHAVE_CONFIG_H -fno-PIE -I. -I. -I/home/alecop01/toolchain/src/gcc/gcc -I/home/alecop01/toolchain/src/gcc/gcc/. -I/home/alecop01/toolchain/src/gcc/gcc/../include -I/home/alecop01/toolchain/src/gcc/gcc/../libcpp/include -I/home/alecop01/toolchain/src/gcc/gcc/../libcody -I/home/alecop01/toolchain/src/gcc/gcc/../libdecnumber -I/home/alecop01/toolchain/src/gcc/gcc/../libdecnumber/bid -I../libdecnumber -I/home/alecop01/toolchain/src/gcc/gcc/../libbacktrace -I. -I. -I/home/alecop01/toolchain/src/gcc/gcc -I/home/alecop01/toolchain/src/gcc/gcc/. -I/home/alecop01/toolchain/src/gcc/gcc/../include -I/home/alecop01/toolchain/src/gcc/gcc/../libcpp/include -I/home/alecop01/toolchain/src/gcc/gcc/../libcody -I/home/alecop01/toolchain/src/gcc/gcc/../libdecnumber -I/home/alecop01/toolchain/src/gcc/gcc/../libdecnumber/bid -I../libdecnumber -I/home/alecop01/toolchain/src/gcc/gcc/../libbacktrace \
> > /home/alecop01/toolchain/src/gcc/gcc/config/aarch64/aarch64-early-ra.cc
> > /home/alecop01/toolchain/src/gcc/gcc/config/aarch64/aarch64-early-ra.cc: In member function ‘void {anonymous}::early_ra::record_constraints(rtx_insn*)’:
> > /home/alecop01/toolchain/src/gcc/gcc/config/aarch64/aarch64-early-ra.cc:1858:66: error: possibly dangling reference to a temporary [-Werror=dangling-reference]
> > 1858 | for (auto &allocno : get_allocno_subgroup (op).allocnos ())
> > | ^
> > /home/alecop01/toolchain/src/gcc/gcc/config/aarch64/aarch64-early-ra.cc:1858:65: note: the temporary was destroyed at the end of the full expression ‘(({anonymous}::early_ra*)this)->{anonymous}::early_ra::get_allocno_subgroup(op).{anonymous}::early_ra::allocno_subgroup::allocnos()’
> > 1858 | for (auto &allocno : get_allocno_subgroup (op).allocnos ())
> > | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~
> > cc1plus: all warnings being treated as errors
> > make[3]: *** [/home/alecop01/toolchain/src/gcc/gcc/config/aarch64/t-aarch64:200: aarch64-early-ra.o] Error 1
> > make[3]: Leaving directory '/work/builds/bstrap/gcc'
> > make[2]: *** [Makefile:5096: all-stage2-gcc] Error 2
> > make[2]: Leaving directory '/work/builds/bstrap'
> > make[1]: *** [Makefile:25405: stage2-bubble] Error 2
> > make[1]: Leaving directory '/work/builds/bstrap'
> > make: *** [Makefile:1100: all] Error 2
>
> Very sorry about that.
>
> > Is the patch expected to introduce new warnings?
>
> No, on the contrary, it was supposed to strictly reduce the # of warnings.
>
> > I'll try to reduce a testcase from this where we don't see the warning
> > before and we see the warning afterwards.
>
> That would be great. I think the fix may be just removing one line.
Here is a reduced testcase as promised:
```
template <typename T> struct array_slice {
using iterator = T *;
iterator begin();
iterator end();
iterator m_base;
};
char recog_data_2;
int record_constraints_op;
struct early_ra {
using operand_mask = int;
struct allocno_info {
int is_earlyclobbered;
};
struct allocno_subgroup {
array_slice<allocno_info> allocnos();
};
allocno_subgroup get_allocno_subgroup(int);
void record_constraints();
};
void early_ra::record_constraints() {
operand_mask earlyclobber_operands, matched_operands, unmatched_operands,
matches_operands, op_mask = operand_mask();
auto record_operand = [&](int, int) {
operand_mask overlaps;
matches_operands |= overlaps;
};
for (int opno; recog_data_2; ++opno) {
operand_mask op_mask = earlyclobber_operands |= op_mask;
if (0)
record_operand(1, 0);
}
if (op_mask || (matched_operands & unmatched_operands && 0))
for (auto &allocno : get_allocno_subgroup(record_constraints_op).allocnos())
allocno.is_earlyclobbered = true;
}
```
Thanks,
Alex
>
> Marek
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] c++: avoid -Wdangling-reference for std::span-like classes [PR110358]
2024-02-01 14:32 ` Alex Coplan
@ 2024-02-01 18:05 ` Marek Polacek
0 siblings, 0 replies; 11+ messages in thread
From: Marek Polacek @ 2024-02-01 18:05 UTC (permalink / raw)
To: Alex Coplan; +Cc: Jason Merrill, GCC Patches
On Thu, Feb 01, 2024 at 02:32:33PM +0000, Alex Coplan wrote:
> On 31/01/2024 15:53, Marek Polacek wrote:
> > On Wed, Jan 31, 2024 at 07:44:41PM +0000, Alex Coplan wrote:
> > > Hi Marek,
> > >
> > > On 30/01/2024 13:15, Marek Polacek wrote:
> > > > On Thu, Jan 25, 2024 at 10:13:10PM -0500, Jason Merrill wrote:
> > > > > On 1/25/24 20:36, Marek Polacek wrote:
> > > > > > Better version:
> > > > > >
> > > > > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> > > > > >
> > > > > > -- >8 --
> > > > > > Real-world experience shows that -Wdangling-reference triggers for
> > > > > > user-defined std::span-like classes a lot. We can easily avoid that
> > > > > > by considering classes like
> > > > > >
> > > > > > template<typename T>
> > > > > > struct Span {
> > > > > > T* data_;
> > > > > > std::size len_;
> > > > > > };
> > > > > >
> > > > > > to be std::span-like, and not warning for them. Unlike the previous
> > > > > > patch, this one considers a non-union class template that has a pointer
> > > > > > data member and a trivial destructor as std::span-like.
> > > > > >
> > > > > > PR c++/110358
> > > > > > PR c++/109640
> > > > > >
> > > > > > gcc/cp/ChangeLog:
> > > > > >
> > > > > > * call.cc (reference_like_class_p): Don't warn for std::span-like
> > > > > > classes.
> > > > > >
> > > > > > gcc/ChangeLog:
> > > > > >
> > > > > > * doc/invoke.texi: Update -Wdangling-reference description.
> > > > > >
> > > > > > gcc/testsuite/ChangeLog:
> > > > > >
> > > > > > * g++.dg/warn/Wdangling-reference18.C: New test.
> > > > > > * g++.dg/warn/Wdangling-reference19.C: New test.
> > > > > > * g++.dg/warn/Wdangling-reference20.C: New test.
> > > > > > ---
> > > > > > gcc/cp/call.cc | 18 ++++++++
> > > > > > gcc/doc/invoke.texi | 14 +++++++
> > > > > > .../g++.dg/warn/Wdangling-reference18.C | 24 +++++++++++
> > > > > > .../g++.dg/warn/Wdangling-reference19.C | 25 +++++++++++
> > > > > > .../g++.dg/warn/Wdangling-reference20.C | 42 +++++++++++++++++++
> > > > > > 5 files changed, 123 insertions(+)
> > > > > > create mode 100644 gcc/testsuite/g++.dg/warn/Wdangling-reference18.C
> > > > > > create mode 100644 gcc/testsuite/g++.dg/warn/Wdangling-reference19.C
> > > > > > create mode 100644 gcc/testsuite/g++.dg/warn/Wdangling-reference20.C
> > > > > >
> > > > > > diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
> > > > > > index 9de0d77c423..afd3e1ff024 100644
> > > > > > --- a/gcc/cp/call.cc
> > > > > > +++ b/gcc/cp/call.cc
> > > > > > @@ -14082,6 +14082,24 @@ reference_like_class_p (tree ctype)
> > > > > > return true;
> > > > > > }
> > > > > > + /* Avoid warning if CTYPE looks like std::span: it's a class template,
> > > > > > + has a T* member, and a trivial destructor. For example,
> > > > > > +
> > > > > > + template<typename T>
> > > > > > + struct Span {
> > > > > > + T* data_;
> > > > > > + std::size len_;
> > > > > > + };
> > > > > > +
> > > > > > + is considered std::span-like. */
> > > > > > + if (NON_UNION_CLASS_TYPE_P (ctype)
> > > > > > + && CLASSTYPE_TEMPLATE_INSTANTIATION (ctype)
> > > > > > + && TYPE_HAS_TRIVIAL_DESTRUCTOR (ctype))
> > > > > > + for (tree field = next_aggregate_field (TYPE_FIELDS (ctype));
> > > > > > + field; field = next_aggregate_field (DECL_CHAIN (field)))
> > > > > > + if (TYPE_PTR_P (TREE_TYPE (field)))
> > > > > > + return true;
> > > > > > +
> > > > > > /* Some classes, such as std::tuple, have the reference member in its
> > > > > > (non-direct) base class. */
> > > > > > if (dfs_walk_once (TYPE_BINFO (ctype), class_has_reference_member_p_r,
> > > > > > diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> > > > > > index 6ec56493e59..e0ff18a86f5 100644
> > > > > > --- a/gcc/doc/invoke.texi
> > > > > > +++ b/gcc/doc/invoke.texi
> > > > > > @@ -3916,6 +3916,20 @@ 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 does not warn for @code{std::span}-like classes. We consider
> > > > > > +classes of the form:
> > > > > > +
> > > > > > +@smallexample
> > > > > > +template<typename T>
> > > > > > +struct Span @{
> > > > > > + T* data_;
> > > > > > + std::size len_;
> > > > > > +@};
> > > > > > +@end smallexample
> > > > > > +
> > > > > > +as @code{std::span}-like; that is, the class is a non-union class template
> > > > > > +that has a pointer data member and a trivial destructor.
> > > > > > +
> > > > > > This warning is enabled by @option{-Wall}.
> > > > > > @opindex Wdelete-non-virtual-dtor
> > > > > > diff --git a/gcc/testsuite/g++.dg/warn/Wdangling-reference18.C b/gcc/testsuite/g++.dg/warn/Wdangling-reference18.C
> > > > > > new file mode 100644
> > > > > > index 00000000000..e088c177769
> > > > > > --- /dev/null
> > > > > > +++ b/gcc/testsuite/g++.dg/warn/Wdangling-reference18.C
> > > > > > @@ -0,0 +1,24 @@
> > > > > > +// PR c++/110358
> > > > > > +// { dg-do compile { target c++11 } }
> > > > > > +// { dg-options "-Wdangling-reference" }
> > > > > > +// Don't warn for std::span-like classes.
> > > > > > +
> > > > > > +template <typename T>
> > > > > > +struct Span {
> > > > > > + T* data_;
> > > > > > + int len_;
> > > > > > +
> > > > > > + [[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 reference" }
> > > > > > + int const& b = get().back(); // { dg-bogus "dangling reference" }
> > > > > > + int const& c = get()[0]; // { dg-bogus "dangling reference" }
> > > > > > +
> > > > > > + return a + b + c;
> > > > > > +}
> > > > > > diff --git a/gcc/testsuite/g++.dg/warn/Wdangling-reference19.C b/gcc/testsuite/g++.dg/warn/Wdangling-reference19.C
> > > > > > new file mode 100644
> > > > > > index 00000000000..053467d822f
> > > > > > --- /dev/null
> > > > > > +++ b/gcc/testsuite/g++.dg/warn/Wdangling-reference19.C
> > > > > > @@ -0,0 +1,25 @@
> > > > > > +// PR c++/110358
> > > > > > +// { dg-do compile { target c++11 } }
> > > > > > +// { dg-options "-Wdangling-reference" }
> > > > > > +// Like Wdangling-reference18.C but not actually a span-like class.
> > > > > > +
> > > > > > +template <typename T>
> > > > > > +struct Span {
> > > > > > + T* data_;
> > > > > > + int len_;
> > > > > > + ~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-warning "dangling reference" }
> > > > > > + int const& b = get().back(); // { dg-warning "dangling reference" }
> > > > > > + int const& c = get()[0]; // { dg-warning "dangling reference" }
> > > > > > +
> > > > > > + return a + b + c;
> > > > > > +}
> > > > > > diff --git a/gcc/testsuite/g++.dg/warn/Wdangling-reference20.C b/gcc/testsuite/g++.dg/warn/Wdangling-reference20.C
> > > > > > new file mode 100644
> > > > > > index 00000000000..463c7380283
> > > > > > --- /dev/null
> > > > > > +++ b/gcc/testsuite/g++.dg/warn/Wdangling-reference20.C
> > > > > > @@ -0,0 +1,42 @@
> > > > > > +// PR c++/109640
> > > > > > +// { dg-do compile { target c++20 } }
> > > > > > +// { dg-options "-Wdangling-reference" }
> > > > > > +// Don't warn for std::span-like classes.
> > > > > > +
> > > > > > +#include <iterator>
> > > > > > +#include <span>
> > > > > > +
> > > > > > +template <typename T>
> > > > > > +struct MySpan
> > > > > > +{
> > > > > > + MySpan(T* data, std::size_t size) :
> > > > > > + data_(data),
> > > > > > + size_(size)
> > > > > > + {}
> > > > > > +
> > > > > > + T& operator[](std::size_t idx) { return data_[idx]; }
> > > > > > +
> > > > > > +private:
> > > > > > + T* data_;
> > > > > > + std::size_t size_;
> > > > > > +};
> > > > > > +
> > > > > > +template <typename T, std::size_t n>
> > > > > > +MySpan<T const> make_my_span(T const(&x)[n])
> > > > > > +{
> > > > > > + return MySpan(std::begin(x), n);
> > > > > > +}
> > > > > > +
> > > > > > +template <typename T, std::size_t n>
> > > > > > +std::span<T const> make_span(T const(&x)[n])
> > > > > > +{
> > > > > > + return std::span(std::begin(x), n);
> > > > > > +}
> > > > > > +
> > > > > > +int main()
> > > > > > +{
> > > > > > + int x[10]{};
> > > > > > + int const& y{make_my_span(x)[0]};
> > > > > > + int const& y2{make_span(x)[0]};
> > > > >
> > > > > Let's also test that we do warn if the argument to make_*span is a
> > > > > temporary. OK with that change, assuming it works as expected.
> > > >
> > > > We do warn then, I've added
> > > > using T = int[10];
> > > > [[maybe_unused]] int const& y3{make_my_span(T{})[0]};
> > > > [[maybe_unused]] int const& y4{make_span(T{})[0]};
> > > > and get two warnings. I'll push with that adjusted; thanks.
> > >
> > > It looks like this patch breaks bootstrap on aarch64-linux-gnu, I see
> > > the following building aarch64-early-ra.cc in stage2:
> > >
> > > /work/builds/bstrap/./prev-gcc/xg++ -B/work/builds/bstrap/./prev-gcc/ -B/work/builds/bstrap/aarch64-unknown-linux-gnu/bin/ -nostdinc++ -B/work/builds/bstrap/prev-aarch64-unknown-linux-gnu/libstdc++-v3/src/.libs -B/work/builds/bstrap/prev-aarch64-unknown-linux-gnu/libstdc++-v3/libsupc++/.libs -I/work/builds/bstrap/prev-aarch64-unknown-linux-gnu/libstdc++-v3/include/aarch64-unknown-linux-gnu -I/work/builds/bstrap/prev-aarch64-unknown-linux-gnu/libstdc++-v3/include -I/home/alecop01/toolchain/src/gcc/libstdc++-v3/libsupc++ -L/work/builds/bstrap/prev-aarch64-unknown-linux-gnu/libstdc++-v3/src/.libs -L/work/builds/bstrap/prev-aarch64-unknown-linux-gnu/libstdc++-v3/libsupc++/.libs -fno-PIE -c -g -O2 -fno-checking -gtoggle -DIN_GCC -fno-exceptions -fno-rtti -fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual -Wmissing-format-attribute -Wconditionally-supported -Woverloaded-virtual -pedantic -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -Werror -fno-common -DHAVE_CONFIG_H -fno-PIE -I. -I. -I/home/alecop01/toolchain/src/gcc/gcc -I/home/alecop01/toolchain/src/gcc/gcc/. -I/home/alecop01/toolchain/src/gcc/gcc/../include -I/home/alecop01/toolchain/src/gcc/gcc/../libcpp/include -I/home/alecop01/toolchain/src/gcc/gcc/../libcody -I/home/alecop01/toolchain/src/gcc/gcc/../libdecnumber -I/home/alecop01/toolchain/src/gcc/gcc/../libdecnumber/bid -I../libdecnumber -I/home/alecop01/toolchain/src/gcc/gcc/../libbacktrace -I. -I. -I/home/alecop01/toolchain/src/gcc/gcc -I/home/alecop01/toolchain/src/gcc/gcc/. -I/home/alecop01/toolchain/src/gcc/gcc/../include -I/home/alecop01/toolchain/src/gcc/gcc/../libcpp/include -I/home/alecop01/toolchain/src/gcc/gcc/../libcody -I/home/alecop01/toolchain/src/gcc/gcc/../libdecnumber -I/home/alecop01/toolchain/src/gcc/gcc/../libdecnumber/bid -I../libdecnumber -I/home/alecop01/toolchain/src/gcc/gcc/../libbacktrace \
> > > /home/alecop01/toolchain/src/gcc/gcc/config/aarch64/aarch64-early-ra.cc
> > > /home/alecop01/toolchain/src/gcc/gcc/config/aarch64/aarch64-early-ra.cc: In member function ‘void {anonymous}::early_ra::record_constraints(rtx_insn*)’:
> > > /home/alecop01/toolchain/src/gcc/gcc/config/aarch64/aarch64-early-ra.cc:1858:66: error: possibly dangling reference to a temporary [-Werror=dangling-reference]
> > > 1858 | for (auto &allocno : get_allocno_subgroup (op).allocnos ())
> > > | ^
> > > /home/alecop01/toolchain/src/gcc/gcc/config/aarch64/aarch64-early-ra.cc:1858:65: note: the temporary was destroyed at the end of the full expression ‘(({anonymous}::early_ra*)this)->{anonymous}::early_ra::get_allocno_subgroup(op).{anonymous}::early_ra::allocno_subgroup::allocnos()’
> > > 1858 | for (auto &allocno : get_allocno_subgroup (op).allocnos ())
> > > | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~
> > > cc1plus: all warnings being treated as errors
> > > make[3]: *** [/home/alecop01/toolchain/src/gcc/gcc/config/aarch64/t-aarch64:200: aarch64-early-ra.o] Error 1
> > > make[3]: Leaving directory '/work/builds/bstrap/gcc'
> > > make[2]: *** [Makefile:5096: all-stage2-gcc] Error 2
> > > make[2]: Leaving directory '/work/builds/bstrap'
> > > make[1]: *** [Makefile:25405: stage2-bubble] Error 2
> > > make[1]: Leaving directory '/work/builds/bstrap'
> > > make: *** [Makefile:1100: all] Error 2
> >
> > Very sorry about that.
> >
> > > Is the patch expected to introduce new warnings?
> >
> > No, on the contrary, it was supposed to strictly reduce the # of warnings.
> >
> > > I'll try to reduce a testcase from this where we don't see the warning
> > > before and we see the warning afterwards.
> >
> > That would be great. I think the fix may be just removing one line.
>
> Here is a reduced testcase as promised:
>
> ```
> template <typename T> struct array_slice {
> using iterator = T *;
> iterator begin();
> iterator end();
> iterator m_base;
> };
> char recog_data_2;
> int record_constraints_op;
> struct early_ra {
> using operand_mask = int;
> struct allocno_info {
> int is_earlyclobbered;
> };
> struct allocno_subgroup {
> array_slice<allocno_info> allocnos();
> };
> allocno_subgroup get_allocno_subgroup(int);
> void record_constraints();
> };
> void early_ra::record_constraints() {
> operand_mask earlyclobber_operands, matched_operands, unmatched_operands,
> matches_operands, op_mask = operand_mask();
> auto record_operand = [&](int, int) {
> operand_mask overlaps;
> matches_operands |= overlaps;
> };
> for (int opno; recog_data_2; ++opno) {
> operand_mask op_mask = earlyclobber_operands |= op_mask;
> if (0)
> record_operand(1, 0);
> }
> if (op_mask || (matched_operands & unmatched_operands && 0))
> for (auto &allocno : get_allocno_subgroup(record_constraints_op).allocnos())
> allocno.is_earlyclobbered = true;
> }
> ```
Thanks. I've added a pointer member to allocno_subgroup so that it
reflects what we have in the codebase, and I'm testing a patch as well
as a bootstrap on aarch64 (with Jakub's fix to get past 113705).
Marek
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2024-02-01 18:05 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-01-22 23:13 [PATCH] c++: avoid -Wdangling-reference for std::span-like classes [PR110358] Marek Polacek
2024-01-26 1:36 ` [PATCH v2] " Marek Polacek
2024-01-26 3:13 ` Jason Merrill
2024-01-30 18:15 ` Marek Polacek
2024-01-31 19:44 ` Alex Coplan
2024-01-31 19:57 ` Jason Merrill
2024-01-31 20:56 ` Marek Polacek
2024-02-01 2:52 ` Jason Merrill
2024-01-31 20:53 ` Marek Polacek
2024-02-01 14:32 ` Alex Coplan
2024-02-01 18:05 ` Marek Polacek
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).