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
