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

Reply via email to