public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
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>;


      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).