From: Jason Merrill <jason@redhat.com>
To: Patrick Palka <ppalka@redhat.com>
Cc: gcc-patches@gcc.gnu.org
Subject: Re: [PATCH] c++: bogus warning w/ deduction guide in anon ns [PR106604]
Date: Fri, 11 Aug 2023 10:00:33 -0400 [thread overview]
Message-ID: <cf462bbc-4ab3-a008-3f80-12af8938f68e@redhat.com> (raw)
In-Reply-To: <f1d43392-0b0b-4be8-50ad-fc50c606e5b6@idea>
On 8/11/23 09:54, Patrick Palka wrote:
> On Thu, 10 Aug 2023, Jason Merrill wrote:
>
>> On 8/10/23 16:40, Patrick Palka wrote:
>>> On Thu, 10 Aug 2023, Jason Merrill wrote:
>>>
>>>> On 8/10/23 12:09, Patrick Palka wrote:
>>>>> Booststrapped and regtested on x86_64-pc-linux-gnu, does this look OK
>>>>> for
>>>>> trunk and perhaps 13?
>>>>>
>>>>> -- >8 --
>>>>>
>>>>> We shouldn't issue a "declared static but never defined" warning
>>>>> for a deduction guide (declared in an anonymous namespace).
>>>>>
>>>>> PR c++/106604
>>>>>
>>>>> gcc/cp/ChangeLog:
>>>>>
>>>>> * decl.cc (wrapup_namespace_globals): Don't issue a
>>>>> -Wunused-function warning for a deduction guide.
>>>>
>>>> Maybe instead of special casing this here we could set DECL_INITIAL on
>>>> deduction guides so they look defined?
>>>
>>> That seems to work, but it requires some tweaks in duplicate_decls to keep
>>> saying "declared" instead of "defined" when diagnosing a deduction guide
>>> redeclaration. I'm not sure which approach is preferable?
>>
>> I'm not sure it matters which we say; the restriction that you can't repeat a
>> deduction guide makes it more like a definition anyway (even if [basic.def]
>> disagrees). Is the diagnostic worse apart from that word?
>
> Ah, makes sense. So we can also remove the special case for them in the
> redeclaration checking code after we give them a dummy DECL_INITIAL.
> Like so?
OK, thanks.
> Here's a before/after for the diagnostic with the below patch:
>
> Before
>
> src/gcc/testsuite/g++.dg/cpp1z/class-deduction74.C:11:1: error: deduction guide ‘S()-> S<int>’ redeclared
> 11 | S() -> S<int>; // { dg-error "redefinition" }
> | ^
> src/gcc/testsuite/g++.dg/cpp1z/class-deduction74.C:10:1: note: ‘S()-> S<int>’ previously declared here
> 10 | S() -> S<int>; // { dg-message "previously defined here|old declaration" }
> | ^
>
> After
>
> src/gcc/testsuite/g++.dg/cpp1z/class-deduction74.C:11:1: error: redefinition of ‘S()-> S<int>’
> 11 | S() -> S<int>; // { dg-error "redefinition" }
> | ^
> src/gcc/testsuite/g++.dg/cpp1z/class-deduction74.C:10:1: note: ‘S()-> S<int>’ previously defined here
> 10 | S() -> S<int>; // { dg-message "previously defined here|old declaration" }
> | ^
>
> -- >8 --
>
> Subject: [PATCH] c++: bogus warning w/ deduction guide in anon ns [PR106604]
>
> Here we're unintentionally issuing a "declared static but never defined"
> warning for a deduction guide declared in an anonymous namespace.
> This patch fixes this by giving deduction guides a dummy DECL_INITIAL,
> which suppresses the warning and also allows us to simplify redeclaration
> checking for them.
>
> Co-authored-by: Jason Merrill <jason@redhat.com>
>
> PR c++/106604
>
> gcc/cp/ChangeLog:
>
> * decl.cc (redeclaration_error_message): Remove special handling
> for deduction guides.
> (grokfndecl): Give deduction guides a dummy DECL_INITIAL.
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/cpp1z/class-deduction74.C: Expect "defined" instead
> of "declared" in diagnostics for a repeated deduction guide.
> * g++.dg/cpp1z/class-deduction116.C: New test.
> ---
> gcc/cp/decl.cc | 14 ++++++--------
> gcc/testsuite/g++.dg/cpp1z/class-deduction116.C | 8 ++++++++
> gcc/testsuite/g++.dg/cpp1z/class-deduction74.C | 14 +++++++-------
> 3 files changed, 21 insertions(+), 15 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/cpp1z/class-deduction116.C
>
> diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
> index 792ab330dd0..3ada5516c58 100644
> --- a/gcc/cp/decl.cc
> +++ b/gcc/cp/decl.cc
> @@ -3297,10 +3297,6 @@ redeclaration_error_message (tree newdecl, tree olddecl)
> }
> }
>
> - if (deduction_guide_p (olddecl)
> - && deduction_guide_p (newdecl))
> - return G_("deduction guide %q+D redeclared");
> -
> /* [class.compare.default]: A definition of a comparison operator as
> defaulted that appears in a class shall be the first declaration of
> that function. */
> @@ -3355,10 +3351,6 @@ redeclaration_error_message (tree newdecl, tree olddecl)
> }
> }
>
> - if (deduction_guide_p (olddecl)
> - && deduction_guide_p (newdecl))
> - return G_("deduction guide %q+D redeclared");
> -
> /* Core issue #226 (C++11):
>
> If a friend function template declaration specifies a
> @@ -10352,6 +10344,12 @@ grokfndecl (tree ctype,
> DECL_CXX_DESTRUCTOR_P (decl) = 1;
> DECL_NAME (decl) = dtor_identifier;
> break;
> + case sfk_deduction_guide:
> + /* Give deduction guides a definition even though they don't really
> + have one: the restriction that you can't repeat a deduction guide
> + makes them more like a definition anyway. */
> + DECL_INITIAL (decl) = void_node;
> + break;
> default:
> break;
> }
> diff --git a/gcc/testsuite/g++.dg/cpp1z/class-deduction116.C b/gcc/testsuite/g++.dg/cpp1z/class-deduction116.C
> new file mode 100644
> index 00000000000..00f6d5fef41
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp1z/class-deduction116.C
> @@ -0,0 +1,8 @@
> +// PR c++/106604
> +// { dg-do compile { target c++17 } }
> +// { dg-additional-options "-Wunused-function" }
> +
> +namespace {
> + template<class T> struct A { A(...); };
> + A(bool) -> A<bool>; // { dg-bogus "never defined" }
> +}
> diff --git a/gcc/testsuite/g++.dg/cpp1z/class-deduction74.C b/gcc/testsuite/g++.dg/cpp1z/class-deduction74.C
> index fe113819a95..7bab882da7d 100644
> --- a/gcc/testsuite/g++.dg/cpp1z/class-deduction74.C
> +++ b/gcc/testsuite/g++.dg/cpp1z/class-deduction74.C
> @@ -7,20 +7,20 @@
> template<typename> struct S { };
> template<typename> struct X { };
>
> -S() -> S<int>; // { dg-message "previously declared here|old declaration" }
> -S() -> S<int>; // { dg-error "redeclared" }
> +S() -> S<int>; // { dg-message "previously defined here|old declaration" }
> +S() -> S<int>; // { dg-error "redefinition" }
> X() -> X<int>;
> S() -> S<float>; // { dg-error "ambiguating new declaration of" }
>
> -S(bool) -> S<int>; // { dg-message "previously declared here" }
> -explicit S(bool) -> S<int>; // { dg-error "redeclared" }
> +S(bool) -> S<int>; // { dg-message "previously defined here" }
> +explicit S(bool) -> S<int>; // { dg-error "redefinition" }
>
> -explicit S(char) -> S<int>; // { dg-message "previously declared here" }
> -S(char) -> S<int>; // { dg-error "redeclared" }
> +explicit S(char) -> S<int>; // { dg-message "previously defined here" }
> +S(char) -> S<int>; // { dg-error "redefinition" }
>
> template<typename T> S(T, T) -> S<int>; // { dg-message "previously declared here" }
> template<typename T> X(T, T) -> X<int>;
> -template<typename T> S(T, T) -> S<int>; // { dg-error "redeclared" }
> +template<typename T> S(T, T) -> S<int>; // { dg-error "redefinition" }
>
> // OK: Use SFINAE.
> template<typename T> S(T) -> S<typename T::foo>;
prev parent reply other threads:[~2023-08-11 14:00 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-10 16:09 Patrick Palka
2023-08-10 17:22 ` Jason Merrill
2023-08-10 20:40 ` Patrick Palka
2023-08-10 21:11 ` Jason Merrill
2023-08-11 13:54 ` Patrick Palka
2023-08-11 14:00 ` Jason Merrill [this message]
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=cf462bbc-4ab3-a008-3f80-12af8938f68e@redhat.com \
--to=jason@redhat.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=ppalka@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).