[PATCH] c++: further -Wdangling-reference refinement [PR107532]

[Marek Polacek via Gcc-patches]

Based on ,
it seems like we should treat *any* class with a reference member
as a reference wrapper.  This simplifies the code so I'm happy to
make that change.

The patch, however, does not suppress the warning in

  int i = 42;
  auto const& v = std::get<0>(std::tuple(i));

Since reference_like_class_p already checks for std::pair
maybe it could also check for std::tuple.  I don't know if we
want to make that change in GCC 13, or move -Wdangling-reference to
-Wextra for GCC 13 and perhaps move it back to -Wall in GCC 14.

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

PR c++/107532

gcc/cp/ChangeLog:

* call.cc (reference_like_class_p): Don't look for a constructor.

gcc/testsuite/ChangeLog:

* g++.dg/warn/Wdangling-reference11.C: New test.
---
 gcc/cp/call.cc| 35 +++
 .../g++.dg/warn/Wdangling-reference11.C   | 23 
 2 files changed, 35 insertions(+), 23 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/warn/Wdangling-reference11.C

diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
index c01e7b82457..00d56a157b6 100644
--- a/gcc/cp/call.cc
+++ b/gcc/cp/call.cc
@@ -13781,8 +13781,9 @@ std_pair_ref_ref_p (tree t)
 
 /* Return true if a class CTYPE is either std::reference_wrapper or
std::ref_view, or a reference wrapper class.  We consider a class
-   a reference wrapper class if it has a reference member and a
-   constructor taking the same reference type.  */
+   a reference wrapper class if it has a reference member.  We no
+   longer check that it has a constructor taking the same reference type
+   since that approach still generated too many false positives.  */
 
 static bool
 reference_like_class_p (tree ctype)
@@ -13798,31 +13799,19 @@ reference_like_class_p (tree ctype)
   if (decl_in_std_namespace_p (tdecl))
 {
   tree name = DECL_NAME (tdecl);
-  return (name
- && (id_equal (name, "reference_wrapper")
- || id_equal (name, "span")
- || id_equal (name, "ref_view")));
+  if (name
+ && (id_equal (name, "reference_wrapper")
+ || id_equal (name, "span")
+ || id_equal (name, "ref_view")))
+   return true;
 }
   for (tree fields = TYPE_FIELDS (ctype);
fields;
fields = DECL_CHAIN (fields))
-{
-  if (TREE_CODE (fields) != FIELD_DECL || DECL_ARTIFICIAL (fields))
-   continue;
-  tree type = TREE_TYPE (fields);
-  if (!TYPE_REF_P (type))
-   continue;
-  /* OK, the field is a reference member.  Do we have a constructor
-taking its type?  */
-  for (tree fn : ovl_range (CLASSTYPE_CONSTRUCTORS (ctype)))
-   {
- tree args = FUNCTION_FIRST_USER_PARMTYPE (fn);
- if (args
- && same_type_p (TREE_VALUE (args), type)
- && TREE_CHAIN (args) == void_list_node)
-   return true;
-   }
-}
+if (TREE_CODE (fields) == FIELD_DECL
+   && !DECL_ARTIFICIAL (fields)
+   && TYPE_REF_P (TREE_TYPE (fields)))
+  return true;
   return false;
 }
 
diff --git a/gcc/testsuite/g++.dg/warn/Wdangling-reference11.C 
b/gcc/testsuite/g++.dg/warn/Wdangling-reference11.C
new file mode 100644
index 000..667618e7196
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wdangling-reference11.C
@@ -0,0 +1,23 @@
+// PR c++/107532
+// { dg-do compile { target c++11 } }
+// { dg-options "-Wdangling-reference" }
+
+struct R
+{
+int& r;
+int& get() { return r; }
+int&& rget() { return static_cast(r); }
+};
+
+int main()
+{
+int i = 42;
+int& l = R{i}.get(); // { dg-bogus "dangling reference" }
+int const& cl = R{i}.get(); // { dg-bogus "dangling reference" }
+int&& r = R{i}.rget(); // { dg-bogus "dangling reference" }
+int const&& cr = R{i}.rget(); // { dg-bogus "dangling reference" }
+(void) l;
+(void) r;
+(void) cr;
+(void) cl;
+}

base-commit: ae7190e345a8d80310835cb83b3b41ef2aeb0d37
-- 
2.39.2

Re: [PATCH] c++: further -Wdangling-reference refinement [PR107532]

[Jason Merrill via Gcc-patches]

On 3/17/23 16:29, Marek Polacek wrote:

Based on ,
it seems like we should treat *any* class with a reference member
as a reference wrapper.  This simplifies the code so I'm happy to
make that change.

The patch, however, does not suppress the warning in

   int i = 42;
   auto const& v = std::get<0>(std::tuple(i));


Why not?  tuple has an int& member, doesn't it?  Do we need to 
look into bases as well?



Since reference_like_class_p already checks for std::pair
maybe it could also check for std::tuple.  I don't know if we
want to make that change in GCC 13, or move -Wdangling-reference to
-Wextra for GCC 13 and perhaps move it back to -Wall in GCC 14.

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

PR c++/107532

gcc/cp/ChangeLog:

* call.cc (reference_like_class_p): Don't look for a constructor.

gcc/testsuite/ChangeLog:

* g++.dg/warn/Wdangling-reference11.C: New test.
---
  gcc/cp/call.cc| 35 +++
  .../g++.dg/warn/Wdangling-reference11.C   | 23 
  2 files changed, 35 insertions(+), 23 deletions(-)
  create mode 100644 gcc/testsuite/g++.dg/warn/Wdangling-reference11.C

diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
index c01e7b82457..00d56a157b6 100644
--- a/gcc/cp/call.cc
+++ b/gcc/cp/call.cc
@@ -13781,8 +13781,9 @@ std_pair_ref_ref_p (tree t)
  
  /* Return true if a class CTYPE is either std::reference_wrapper or

 std::ref_view, or a reference wrapper class.  We consider a class
-   a reference wrapper class if it has a reference member and a
-   constructor taking the same reference type.  */
+   a reference wrapper class if it has a reference member.  We no
+   longer check that it has a constructor taking the same reference type
+   since that approach still generated too many false positives.  */
  
  static bool

  reference_like_class_p (tree ctype)
@@ -13798,31 +13799,19 @@ reference_like_class_p (tree ctype)
if (decl_in_std_namespace_p (tdecl))
  {
tree name = DECL_NAME (tdecl);
-  return (name
- && (id_equal (name, "reference_wrapper")
- || id_equal (name, "span")
- || id_equal (name, "ref_view")));
+  if (name
+ && (id_equal (name, "reference_wrapper")
+ || id_equal (name, "span")
+ || id_equal (name, "ref_view")))
+   return true;
  }
for (tree fields = TYPE_FIELDS (ctype);
 fields;
 fields = DECL_CHAIN (fields))
-{
-  if (TREE_CODE (fields) != FIELD_DECL || DECL_ARTIFICIAL (fields))
-   continue;
-  tree type = TREE_TYPE (fields);
-  if (!TYPE_REF_P (type))
-   continue;
-  /* OK, the field is a reference member.  Do we have a constructor
-taking its type?  */
-  for (tree fn : ovl_range (CLASSTYPE_CONSTRUCTORS (ctype)))
-   {
- tree args = FUNCTION_FIRST_USER_PARMTYPE (fn);
- if (args
- && same_type_p (TREE_VALUE (args), type)
- && TREE_CHAIN (args) == void_list_node)
-   return true;
-   }
-}
+if (TREE_CODE (fields) == FIELD_DECL
+   && !DECL_ARTIFICIAL (fields)
+   && TYPE_REF_P (TREE_TYPE (fields)))
+  return true;
return false;
  }
  
diff --git a/gcc/testsuite/g++.dg/warn/Wdangling-reference11.C b/gcc/testsuite/g++.dg/warn/Wdangling-reference11.C

new file mode 100644
index 000..667618e7196
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wdangling-reference11.C
@@ -0,0 +1,23 @@
+// PR c++/107532
+// { dg-do compile { target c++11 } }
+// { dg-options "-Wdangling-reference" }
+
+struct R
+{
+int& r;
+int& get() { return r; }
+int&& rget() { return static_cast(r); }
+};
+
+int main()
+{
+int i = 42;
+int& l = R{i}.get(); // { dg-bogus "dangling reference" }
+int const& cl = R{i}.get(); // { dg-bogus "dangling reference" }
+int&& r = R{i}.rget(); // { dg-bogus "dangling reference" }
+int const&& cr = R{i}.rget(); // { dg-bogus "dangling reference" }
+(void) l;
+(void) r;
+(void) cr;
+(void) cl;
+}

base-commit: ae7190e345a8d80310835cb83b3b41ef2aeb0d37