On Sat, Mar 18, 2023 at 08:35:36AM -0400, Jason Merrill wrote: > On 3/17/23 16:29, Marek Polacek wrote: > > Based on <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107532#c24>, > > 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<int&>(i)); > > Why not? tuple<int&> has an int& member, doesn't it? Do we need to look > into bases as well?
Indeed. I don't know why I didn't do it right away; it's really not that complicated: Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? -- >8 -- Based on <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107532#c24>, it seems like we should treat *any* class with a reference member as a reference wrapper. To suppress the warning in int i = 42; auto const& v = std::get<0>(std::tuple<int&>(i)); we have to look into base classes as well. For std::tuple, this means that we have to check the _Head_base subobject, which is a non-direct base class of std::tuple. So I've employed a DFS walk. PR c++/107532 gcc/cp/ChangeLog: * call.cc (class_has_reference_member_p): New. (class_has_reference_member_p_r): New. (reference_like_class_p): Don't look for a specific constructor. Use a DFS walk with class_has_reference_member_p_r. gcc/testsuite/ChangeLog: * g++.dg/warn/Wdangling-reference11.C: New test. * g++.dg/warn/Wdangling-reference12.C: New test. --- gcc/cp/call.cc | 63 +++++++++++-------- .../g++.dg/warn/Wdangling-reference11.C | 23 +++++++ .../g++.dg/warn/Wdangling-reference12.C | 12 ++++ 3 files changed, 72 insertions(+), 26 deletions(-) create mode 100644 gcc/testsuite/g++.dg/warn/Wdangling-reference11.C create mode 100644 gcc/testsuite/g++.dg/warn/Wdangling-reference12.C diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc index c52a09b9be2..429170e43ea 100644 --- a/gcc/cp/call.cc +++ b/gcc/cp/call.cc @@ -13783,8 +13783,31 @@ 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 +class_has_reference_member_p (tree t) +{ + for (tree fields = TYPE_FIELDS (t); + fields; + fields = DECL_CHAIN (fields)) + if (TREE_CODE (fields) == FIELD_DECL + && !DECL_ARTIFICIAL (fields) + && TYPE_REF_P (TREE_TYPE (fields))) + return true; + return false; +} + +/* A wrapper for the above suitable as a callback for dfs_walk_once. */ + +static tree +class_has_reference_member_p_r (tree binfo, void *) +{ + return (class_has_reference_member_p (BINFO_TYPE (binfo)) + ? integer_one_node : NULL_TREE); +} static bool reference_like_class_p (tree ctype) @@ -13800,31 +13823,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"))); - } - 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 (name + && (id_equal (name, "reference_wrapper") + || id_equal (name, "span") + || id_equal (name, "ref_view"))) + 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, + nullptr, nullptr)) + 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 00000000000..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<int&&>(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; +} diff --git a/gcc/testsuite/g++.dg/warn/Wdangling-reference12.C b/gcc/testsuite/g++.dg/warn/Wdangling-reference12.C new file mode 100644 index 00000000000..85e01f01a50 --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Wdangling-reference12.C @@ -0,0 +1,12 @@ +// PR c++/107532 +// { dg-do compile { target c++11 } } +// { dg-options "-Wdangling-reference" } + +#include <tuple> + +int main() +{ + int i = 42; + auto const& v = std::get<0>(std::tuple<int&>(i)); // { dg-bogus "dangling reference" } + (void) v; +} base-commit: a226590fefb35ed66adf73d85cefe49048a78ab8 -- 2.39.2
