Re: [PATCH v2 1/2] c++: Standardise errors for module_may_redeclare

2024-04-25 Thread Jason Merrill 

On 4/19/24 09:18, Nathaniel Shead wrote:

On Mon, Apr 15, 2024 at 02:49:35PM +1000, Nathaniel Shead wrote:

I took another look at this patch and have split it into two, one (this
one) to standardise the error messages used and prepare
'module_may_redeclare' for use with temploid friends, and another
followup patch to actually handle them correctly.

Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?

-- >8 --

Currently different places calling 'module_may_redeclare' all emit very
similar but slightly different error messages, and handle different
kinds of declarations differently.  This patch makes the function
perform its own error messages so that they're all in one place, and
prepares it for use with temploid friends (PR c++/114275).

gcc/cp/ChangeLog:

* cp-tree.h (module_may_redeclare): Add default parameter.
* decl.cc (duplicate_decls): Don't emit errors for failed
module_may_redeclare.
(xref_tag): Likewise.
(start_enum): Likewise.
* semantics.cc (begin_class_definition): Likewise.
* module.cc (module_may_redeclare): Clean up logic. Emit error
messages on failure.

gcc/testsuite/ChangeLog:

* g++.dg/modules/enum-12.C: Update error message.
* g++.dg/modules/friend-5_b.C: Likewise.
* g++.dg/modules/shadow-1_b.C: Likewise.

Signed-off-by: Nathaniel Shead 
---
  gcc/cp/cp-tree.h  |   2 +-
  gcc/cp/decl.cc|  28 +
  gcc/cp/module.cc  | 120 ++
  gcc/cp/semantics.cc   |   6 +-
  gcc/testsuite/g++.dg/modules/enum-12.C|   2 +-
  gcc/testsuite/g++.dg/modules/friend-5_b.C |   2 +-
  gcc/testsuite/g++.dg/modules/shadow-1_b.C |   5 +-
  7 files changed, 89 insertions(+), 76 deletions(-)

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 1dbb577a38d..faa7a0052a5 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -7401,7 +7401,7 @@ inline bool module_exporting_p ()
  
  extern module_state *get_module (tree name, module_state *parent = NULL,

 bool partition = false);
-extern bool module_may_redeclare (tree decl);
+extern bool module_may_redeclare (tree olddecl, tree newdecl = NULL);
  
  extern bool module_global_init_needed ();

  extern bool module_determine_import_inits ();
diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
index 65ab64885ff..aa66da4829d 100644
--- a/gcc/cp/decl.cc
+++ b/gcc/cp/decl.cc
@@ -2279,18 +2279,8 @@ duplicate_decls (tree newdecl, tree olddecl, bool 
hiding, bool was_hidden)
&& TREE_CODE (olddecl) != NAMESPACE_DECL
&& !hiding)
  {
-  if (!module_may_redeclare (olddecl))
-   {
- if (DECL_ARTIFICIAL (olddecl))
-   error ("declaration %qD conflicts with builtin", newdecl);
- else
-   {
- error ("declaration %qD conflicts with import", newdecl);
- inform (olddecl_loc, "import declared %q#D here", olddecl);
-   }
-
- return error_mark_node;
-   }
+  if (!module_may_redeclare (olddecl, newdecl))
+   return error_mark_node;
  
tree not_tmpl = STRIP_TEMPLATE (olddecl);

if (DECL_LANG_SPECIFIC (not_tmpl)
@@ -16620,12 +16610,7 @@ xref_tag (enum tag_types tag_code, tree name,
{
  tree decl = TYPE_NAME (t);
  if (!module_may_redeclare (decl))
-   {
- auto_diagnostic_group d;
- error ("cannot declare %qD in a different module", decl);
- inform (DECL_SOURCE_LOCATION (decl), "previously declared here");
- return error_mark_node;
-   }
+   return error_mark_node;
  
  	  tree not_tmpl = STRIP_TEMPLATE (decl);

  if (DECL_LANG_SPECIFIC (not_tmpl)
@@ -16973,12 +16958,7 @@ start_enum (tree name, tree enumtype, tree 
underlying_type,
{
  tree decl = TYPE_NAME (enumtype);
  if (!module_may_redeclare (decl))
-   {
- auto_diagnostic_group d;
- error ("cannot declare %qD in different module", decl);
- inform (DECL_SOURCE_LOCATION (decl), "previously declared here");
- enumtype = error_mark_node;
-   }
+   enumtype = error_mark_node;
  else
set_instantiating_module (decl);
}
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index 001430a4a8f..e2d2910ae48 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -18992,11 +18992,15 @@ get_importing_module (tree decl, bool flexible)
return module->mod;
  }
  
-/* Is it permissible to redeclare DECL.  */

+/* Is it permissible to redeclare OLDDECL with NEWDECL.
+
+   If NEWDECL is NULL, assumes that OLDDECL will be redeclared using
+   the current scope's module and attachment.  */
  
  bool

-module_may_redeclare (tree decl)
+module_may_redeclare (tree olddecl, tree newdecl)
  {
+  tree decl = olddecl;
for (;;)
  {
tree ctx =

Re: [PATCH v2 1/2] c++: Standardise errors for module_may_redeclare

2024-04-19 Thread Nathaniel Shead 
On Mon, Apr 15, 2024 at 02:49:35PM +1000, Nathaniel Shead wrote:
> I took another look at this patch and have split it into two, one (this
> one) to standardise the error messages used and prepare
> 'module_may_redeclare' for use with temploid friends, and another
> followup patch to actually handle them correctly.
> 
> Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?
> 
> -- >8 --
> 
> Currently different places calling 'module_may_redeclare' all emit very
> similar but slightly different error messages, and handle different
> kinds of declarations differently.  This patch makes the function
> perform its own error messages so that they're all in one place, and
> prepares it for use with temploid friends (PR c++/114275).
> 
> gcc/cp/ChangeLog:
> 
>   * cp-tree.h (module_may_redeclare): Add default parameter.
>   * decl.cc (duplicate_decls): Don't emit errors for failed
>   module_may_redeclare.
>   (xref_tag): Likewise.
>   (start_enum): Likewise.
>   * semantics.cc (begin_class_definition): Likewise.
>   * module.cc (module_may_redeclare): Clean up logic. Emit error
>   messages on failure.
> 
> gcc/testsuite/ChangeLog:
> 
>   * g++.dg/modules/enum-12.C: Update error message.
>   * g++.dg/modules/friend-5_b.C: Likewise.
>   * g++.dg/modules/shadow-1_b.C: Likewise.
> 
> Signed-off-by: Nathaniel Shead 
> ---
>  gcc/cp/cp-tree.h  |   2 +-
>  gcc/cp/decl.cc|  28 +
>  gcc/cp/module.cc  | 120 ++
>  gcc/cp/semantics.cc   |   6 +-
>  gcc/testsuite/g++.dg/modules/enum-12.C|   2 +-
>  gcc/testsuite/g++.dg/modules/friend-5_b.C |   2 +-
>  gcc/testsuite/g++.dg/modules/shadow-1_b.C |   5 +-
>  7 files changed, 89 insertions(+), 76 deletions(-)
> 
> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> index 1dbb577a38d..faa7a0052a5 100644
> --- a/gcc/cp/cp-tree.h
> +++ b/gcc/cp/cp-tree.h
> @@ -7401,7 +7401,7 @@ inline bool module_exporting_p ()
>  
>  extern module_state *get_module (tree name, module_state *parent = NULL,
>bool partition = false);
> -extern bool module_may_redeclare (tree decl);
> +extern bool module_may_redeclare (tree olddecl, tree newdecl = NULL);
>  
>  extern bool module_global_init_needed ();
>  extern bool module_determine_import_inits ();
> diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
> index 65ab64885ff..aa66da4829d 100644
> --- a/gcc/cp/decl.cc
> +++ b/gcc/cp/decl.cc
> @@ -2279,18 +2279,8 @@ duplicate_decls (tree newdecl, tree olddecl, bool 
> hiding, bool was_hidden)
>&& TREE_CODE (olddecl) != NAMESPACE_DECL
>&& !hiding)
>  {
> -  if (!module_may_redeclare (olddecl))
> - {
> -   if (DECL_ARTIFICIAL (olddecl))
> - error ("declaration %qD conflicts with builtin", newdecl);
> -   else
> - {
> -   error ("declaration %qD conflicts with import", newdecl);
> -   inform (olddecl_loc, "import declared %q#D here", olddecl);
> - }
> -
> -   return error_mark_node;
> - }
> +  if (!module_may_redeclare (olddecl, newdecl))
> + return error_mark_node;
>  
>tree not_tmpl = STRIP_TEMPLATE (olddecl);
>if (DECL_LANG_SPECIFIC (not_tmpl)
> @@ -16620,12 +16610,7 @@ xref_tag (enum tag_types tag_code, tree name,
>   {
> tree decl = TYPE_NAME (t);
> if (!module_may_redeclare (decl))
> - {
> -   auto_diagnostic_group d;
> -   error ("cannot declare %qD in a different module", decl);
> -   inform (DECL_SOURCE_LOCATION (decl), "previously declared here");
> -   return error_mark_node;
> - }
> + return error_mark_node;
>  
> tree not_tmpl = STRIP_TEMPLATE (decl);
> if (DECL_LANG_SPECIFIC (not_tmpl)
> @@ -16973,12 +16958,7 @@ start_enum (tree name, tree enumtype, tree 
> underlying_type,
>   {
> tree decl = TYPE_NAME (enumtype);
> if (!module_may_redeclare (decl))
> - {
> -   auto_diagnostic_group d;
> -   error ("cannot declare %qD in different module", decl);
> -   inform (DECL_SOURCE_LOCATION (decl), "previously declared here");
> -   enumtype = error_mark_node;
> - }
> + enumtype = error_mark_node;
> else
>   set_instantiating_module (decl);
>   }
> diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> index 001430a4a8f..e2d2910ae48 100644
> --- a/gcc/cp/module.cc
> +++ b/gcc/cp/module.cc
> @@ -18992,11 +18992,15 @@ get_importing_module (tree decl, bool flexible)
>return module->mod;
>  }
>  
> -/* Is it permissible to redeclare DECL.  */
> +/* Is it permissible to redeclare OLDDECL with NEWDECL.
> +
> +   If NEWDECL is NULL, assumes that OLDDECL will be redeclared using
> +   the current scope's module and attachment.  */
>  
>  bool
> -module_may_redeclare (tree decl)
> +module_may_redeclare (tree olddecl,

Re: [PATCH v2 1/2] c++: Standardise errors for module_may_redeclare

2024-04-17 Thread Patrick Palka 
On Mon, 15 Apr 2024, Nathaniel Shead wrote:

> I took another look at this patch and have split it into two, one (this
> one) to standardise the error messages used and prepare
> 'module_may_redeclare' for use with temploid friends, and another
> followup patch to actually handle them correctly.
> 
> Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?

LGTM

> 
> -- >8 --
> 
> Currently different places calling 'module_may_redeclare' all emit very
> similar but slightly different error messages, and handle different
> kinds of declarations differently.  This patch makes the function
> perform its own error messages so that they're all in one place, and
> prepares it for use with temploid friends (PR c++/114275).
> 
> gcc/cp/ChangeLog:
> 
>   * cp-tree.h (module_may_redeclare): Add default parameter.
>   * decl.cc (duplicate_decls): Don't emit errors for failed
>   module_may_redeclare.
>   (xref_tag): Likewise.
>   (start_enum): Likewise.
>   * semantics.cc (begin_class_definition): Likewise.
>   * module.cc (module_may_redeclare): Clean up logic. Emit error
>   messages on failure.
> 
> gcc/testsuite/ChangeLog:
> 
>   * g++.dg/modules/enum-12.C: Update error message.
>   * g++.dg/modules/friend-5_b.C: Likewise.
>   * g++.dg/modules/shadow-1_b.C: Likewise.
> 
> Signed-off-by: Nathaniel Shead 
> ---
>  gcc/cp/cp-tree.h  |   2 +-
>  gcc/cp/decl.cc|  28 +
>  gcc/cp/module.cc  | 120 ++
>  gcc/cp/semantics.cc   |   6 +-
>  gcc/testsuite/g++.dg/modules/enum-12.C|   2 +-
>  gcc/testsuite/g++.dg/modules/friend-5_b.C |   2 +-
>  gcc/testsuite/g++.dg/modules/shadow-1_b.C |   5 +-
>  7 files changed, 89 insertions(+), 76 deletions(-)
> 
> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> index 1dbb577a38d..faa7a0052a5 100644
> --- a/gcc/cp/cp-tree.h
> +++ b/gcc/cp/cp-tree.h
> @@ -7401,7 +7401,7 @@ inline bool module_exporting_p ()
>  
>  extern module_state *get_module (tree name, module_state *parent = NULL,
>bool partition = false);
> -extern bool module_may_redeclare (tree decl);
> +extern bool module_may_redeclare (tree olddecl, tree newdecl = NULL);
>  
>  extern bool module_global_init_needed ();
>  extern bool module_determine_import_inits ();
> diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
> index 65ab64885ff..aa66da4829d 100644
> --- a/gcc/cp/decl.cc
> +++ b/gcc/cp/decl.cc
> @@ -2279,18 +2279,8 @@ duplicate_decls (tree newdecl, tree olddecl, bool 
> hiding, bool was_hidden)
>&& TREE_CODE (olddecl) != NAMESPACE_DECL
>&& !hiding)
>  {
> -  if (!module_may_redeclare (olddecl))
> - {
> -   if (DECL_ARTIFICIAL (olddecl))
> - error ("declaration %qD conflicts with builtin", newdecl);
> -   else
> - {
> -   error ("declaration %qD conflicts with import", newdecl);
> -   inform (olddecl_loc, "import declared %q#D here", olddecl);
> - }
> -
> -   return error_mark_node;
> - }
> +  if (!module_may_redeclare (olddecl, newdecl))
> + return error_mark_node;
>  
>tree not_tmpl = STRIP_TEMPLATE (olddecl);
>if (DECL_LANG_SPECIFIC (not_tmpl)
> @@ -16620,12 +16610,7 @@ xref_tag (enum tag_types tag_code, tree name,
>   {
> tree decl = TYPE_NAME (t);
> if (!module_may_redeclare (decl))
> - {
> -   auto_diagnostic_group d;
> -   error ("cannot declare %qD in a different module", decl);
> -   inform (DECL_SOURCE_LOCATION (decl), "previously declared here");
> -   return error_mark_node;
> - }
> + return error_mark_node;
>  
> tree not_tmpl = STRIP_TEMPLATE (decl);
> if (DECL_LANG_SPECIFIC (not_tmpl)
> @@ -16973,12 +16958,7 @@ start_enum (tree name, tree enumtype, tree 
> underlying_type,
>   {
> tree decl = TYPE_NAME (enumtype);
> if (!module_may_redeclare (decl))
> - {
> -   auto_diagnostic_group d;
> -   error ("cannot declare %qD in different module", decl);
> -   inform (DECL_SOURCE_LOCATION (decl), "previously declared here");
> -   enumtype = error_mark_node;
> - }
> + enumtype = error_mark_node;
> else
>   set_instantiating_module (decl);
>   }
> diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> index 001430a4a8f..e2d2910ae48 100644
> --- a/gcc/cp/module.cc
> +++ b/gcc/cp/module.cc
> @@ -18992,11 +18992,15 @@ get_importing_module (tree decl, bool flexible)
>return module->mod;
>  }
>  
> -/* Is it permissible to redeclare DECL.  */
> +/* Is it permissible to redeclare OLDDECL with NEWDECL.
> +
> +   If NEWDECL is NULL, assumes that OLDDECL will be redeclared using
> +   the current scope's module and attachment.  */
>  
>  bool
> -module_may_redeclare (tree decl)
> +module_may_redeclare (tree olddecl, tree newdecl)
>

[PATCH v2 1/2] c++: Standardise errors for module_may_redeclare

2024-04-14 Thread Nathaniel Shead 
I took another look at this patch and have split it into two, one (this
one) to standardise the error messages used and prepare
'module_may_redeclare' for use with temploid friends, and another
followup patch to actually handle them correctly.

Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?

-- >8 --

Currently different places calling 'module_may_redeclare' all emit very
similar but slightly different error messages, and handle different
kinds of declarations differently.  This patch makes the function
perform its own error messages so that they're all in one place, and
prepares it for use with temploid friends (PR c++/114275).

gcc/cp/ChangeLog:

* cp-tree.h (module_may_redeclare): Add default parameter.
* decl.cc (duplicate_decls): Don't emit errors for failed
module_may_redeclare.
(xref_tag): Likewise.
(start_enum): Likewise.
* semantics.cc (begin_class_definition): Likewise.
* module.cc (module_may_redeclare): Clean up logic. Emit error
messages on failure.

gcc/testsuite/ChangeLog:

* g++.dg/modules/enum-12.C: Update error message.
* g++.dg/modules/friend-5_b.C: Likewise.
* g++.dg/modules/shadow-1_b.C: Likewise.

Signed-off-by: Nathaniel Shead 
---
 gcc/cp/cp-tree.h  |   2 +-
 gcc/cp/decl.cc|  28 +
 gcc/cp/module.cc  | 120 ++
 gcc/cp/semantics.cc   |   6 +-
 gcc/testsuite/g++.dg/modules/enum-12.C|   2 +-
 gcc/testsuite/g++.dg/modules/friend-5_b.C |   2 +-
 gcc/testsuite/g++.dg/modules/shadow-1_b.C |   5 +-
 7 files changed, 89 insertions(+), 76 deletions(-)

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 1dbb577a38d..faa7a0052a5 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -7401,7 +7401,7 @@ inline bool module_exporting_p ()
 
 extern module_state *get_module (tree name, module_state *parent = NULL,
 bool partition = false);
-extern bool module_may_redeclare (tree decl);
+extern bool module_may_redeclare (tree olddecl, tree newdecl = NULL);
 
 extern bool module_global_init_needed ();
 extern bool module_determine_import_inits ();
diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
index 65ab64885ff..aa66da4829d 100644
--- a/gcc/cp/decl.cc
+++ b/gcc/cp/decl.cc
@@ -2279,18 +2279,8 @@ duplicate_decls (tree newdecl, tree olddecl, bool 
hiding, bool was_hidden)
   && TREE_CODE (olddecl) != NAMESPACE_DECL
   && !hiding)
 {
-  if (!module_may_redeclare (olddecl))
-   {
- if (DECL_ARTIFICIAL (olddecl))
-   error ("declaration %qD conflicts with builtin", newdecl);
- else
-   {
- error ("declaration %qD conflicts with import", newdecl);
- inform (olddecl_loc, "import declared %q#D here", olddecl);
-   }
-
- return error_mark_node;
-   }
+  if (!module_may_redeclare (olddecl, newdecl))
+   return error_mark_node;
 
   tree not_tmpl = STRIP_TEMPLATE (olddecl);
   if (DECL_LANG_SPECIFIC (not_tmpl)
@@ -16620,12 +16610,7 @@ xref_tag (enum tag_types tag_code, tree name,
{
  tree decl = TYPE_NAME (t);
  if (!module_may_redeclare (decl))
-   {
- auto_diagnostic_group d;
- error ("cannot declare %qD in a different module", decl);
- inform (DECL_SOURCE_LOCATION (decl), "previously declared here");
- return error_mark_node;
-   }
+   return error_mark_node;
 
  tree not_tmpl = STRIP_TEMPLATE (decl);
  if (DECL_LANG_SPECIFIC (not_tmpl)
@@ -16973,12 +16958,7 @@ start_enum (tree name, tree enumtype, tree 
underlying_type,
{
  tree decl = TYPE_NAME (enumtype);
  if (!module_may_redeclare (decl))
-   {
- auto_diagnostic_group d;
- error ("cannot declare %qD in different module", decl);
- inform (DECL_SOURCE_LOCATION (decl), "previously declared here");
- enumtype = error_mark_node;
-   }
+   enumtype = error_mark_node;
  else
set_instantiating_module (decl);
}
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index 001430a4a8f..e2d2910ae48 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -18992,11 +18992,15 @@ get_importing_module (tree decl, bool flexible)
   return module->mod;
 }
 
-/* Is it permissible to redeclare DECL.  */
+/* Is it permissible to redeclare OLDDECL with NEWDECL.
+
+   If NEWDECL is NULL, assumes that OLDDECL will be redeclared using
+   the current scope's module and attachment.  */
 
 bool
-module_may_redeclare (tree decl)
+module_may_redeclare (tree olddecl, tree newdecl)
 {
+  tree decl = olddecl;
   for (;;)
 {
   tree ctx = CP_DECL_CONTEXT (decl);
@@ -19010,58 +19014,94 @@ module_may_redeclare (tree decl)
   decl = TYPE_NAME (ctx);
 }
 
-  tree not_tmpl =