On Fri, May 24, 2024 at 11:24:38AM -0400, Jason Merrill wrote:
> On 5/24/24 11:20, Nathaniel Shead wrote:
> > This is just a small improvement to a diagnostic. I thought about also
> > adding an inform to suggest something like "standard library headers
> > should be included in the GMF" or somesuch, but I'm not sure that'll be
> > especially valuable and may be confusing if this particular error is
> > caused some other way.
> >
> > Bootstrapped and regtested (so far just modules.exp) on
> > x86_64-pc-linux-gnu, OK for trunk if full regtest succeeds?
> >
> > -- >8 --
> >
> > If a user mistakenly includes a standard library header within the
> > module purview, they currently get a confusing "declaration conflicts
> > with builtin" error. This patch updates the message to include "in
> > module", to help guide the user towards the likely cause.
> >
> > gcc/cp/ChangeLog:
> >
> > * module.cc (module_may_redeclare): Update error message.
> >
> > gcc/testsuite/ChangeLog:
> >
> > * g++.dg/modules/enum-12.C: Test for updated error.
> >
> > Signed-off-by: Nathaniel Shead <nathanielosh...@gmail.com>
> > ---
> > gcc/cp/module.cc | 9 ++++++++-
> > gcc/testsuite/g++.dg/modules/enum-12.C | 2 +-
> > 2 files changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> > index 6cd7d9e0b93..60cf3ebc468 100644
> > --- a/gcc/cp/module.cc
> > +++ b/gcc/cp/module.cc
> > @@ -19140,7 +19140,14 @@ module_may_redeclare (tree olddecl, tree newdecl)
> > decl = newdecl ? newdecl : olddecl;
> > location_t loc = newdecl ? DECL_SOURCE_LOCATION (newdecl) :
> > input_location;
> > if (DECL_IS_UNDECLARED_BUILTIN (olddecl))
> > - error_at (loc, "declaration %qD conflicts with builtin", decl);
> > + {
> > + if (newdecl_attached_p)
> > + error_at (loc, "declaring %qD in module %qs conflicts with builtin",
>
> Maybe "with builtin in global module"?
Yup, that's even clearer, thanks.
>
> > + decl, new_mod->get_flatname ());
> > + else
> > + error_at (loc, "declaring %qD in global module conflicts with builtin",
>
> I'm not sure mentioning the global module adds anything in this case?
>
I'd done it for consistency with the other cases in case we ever somehow
reached this code path, but happy to remove since this should always be
a problem regardless.
So maybe something like this?
-- >8 --
If a user mistakenly includes a standard library header within the
module purview, they currently get a confusing "declaration conflicts
with builtin" error. This patch updates the message to include "in
module", to help guide the user towards the likely cause.
gcc/cp/ChangeLog:
* module.cc (module_may_redeclare): Update error message.
gcc/testsuite/ChangeLog:
* g++.dg/modules/enum-12.C: Test for updated error.
Signed-off-by: Nathaniel Shead <nathanielosh...@gmail.com>
---
gcc/cp/module.cc | 8 +++++++-
gcc/testsuite/g++.dg/modules/enum-12.C | 2 +-
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index 6cd7d9e0b93..3f8f84bb9fd 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -19140,7 +19140,13 @@ module_may_redeclare (tree olddecl, tree newdecl)
decl = newdecl ? newdecl : olddecl;
location_t loc = newdecl ? DECL_SOURCE_LOCATION (newdecl) : input_location;
if (DECL_IS_UNDECLARED_BUILTIN (olddecl))
- error_at (loc, "declaration %qD conflicts with builtin", decl);
+ {
+ if (newdecl_attached_p)
+ error_at (loc, "declaring %qD in module %qs conflicts with builtin "
+ "in global module", decl, new_mod->get_flatname ());
+ else
+ error_at (loc, "declaration %qD conflicts with builtin", decl);
+ }
else if (DECL_LANG_SPECIFIC (old_inner) && DECL_MODULE_IMPORT_P (old_inner))
{
auto_diagnostic_group d;
diff --git a/gcc/testsuite/g++.dg/modules/enum-12.C
b/gcc/testsuite/g++.dg/modules/enum-12.C
index 064f220dedf..cf8f445e076 100644
--- a/gcc/testsuite/g++.dg/modules/enum-12.C
+++ b/gcc/testsuite/g++.dg/modules/enum-12.C
@@ -4,7 +4,7 @@
export module foo;
namespace std {
- enum class align_val_t : decltype(sizeof(int)) {}; // { dg-error "conflicts
with builtin" }
+ enum class align_val_t : decltype(sizeof(int)) {}; // { dg-error "in module
.foo. conflicts with builtin" }
}
// { dg-prune-output "not writing module" }
--
2.43.2
