From: Jason Merrill <jason@redhat.com>
To: Marek Polacek <polacek@redhat.com>,
GCC Patches <gcc-patches@gcc.gnu.org>
Subject: Re: [PATCH] c++: further -Wdangling-reference refinement [PR107532]
Date: Sat, 18 Mar 2023 08:35:36 -0400 [thread overview]
Message-ID: <cba51119-5008-71c4-23b8-816d31c1c1a7@redhat.com> (raw)
In-Reply-To: <20230317202908.42800-1-polacek@redhat.com>
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?
> Since reference_like_class_p already checks for std::pair<const T&, const T&>
> maybe it could also check for std::tuple<T&, ...>. 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 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;
> +}
>
> base-commit: ae7190e345a8d80310835cb83b3b41ef2aeb0d37
next prev parent reply other threads:[~2023-03-18 12:35 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-17 20:29 Marek Polacek
2023-03-18 12:35 ` Jason Merrill [this message]
2023-03-20 22:06 ` [PATCH v2] " Marek Polacek
2023-03-21 14:31 ` Jason Merrill
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=cba51119-5008-71c4-23b8-816d31c1c1a7@redhat.com \
--to=jason@redhat.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=polacek@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).