| Message ID | 6550be76.170a0220.b3f34.4f25@mx.google.com |
|---|---|
| State | New |
| Headers | show |
| Series | c++/modules: check mismatching exports for class tags [PR98885] | expand |
On 11/12/23 07:00, Nathaniel Shead wrote: > I think the error message is still a little bit unclear but I couldn't > come up with something clearer that was similarly concise and matching > the existing style. > > (Also I noticed that the linked PR was assigned to Nathan but there > hadn't been activity for a while, and I've been looking into these kinds > of issues recently anyway so I thought I'd give it a go.) > > Bootstrapped and regtested on x86_64-pc-linux-gnu. I don't have write > access. ok > > -- >8 -- > > Checks for exporting a declaration that was previously declared as not > exported is implemented in 'duplicate_decls', but this doesn't handle > declarations of classes. This patch adds these checks and slightly > adjusts the associated error messages for clarity. > > PR c++/98885 > > gcc/cp/ChangeLog: > > * decl.cc (duplicate_decls): Adjust error message. > (xref_tag): Adjust error message. Check exporting decl that is > already declared as non-exporting. > > gcc/testsuite/ChangeLog: > > * g++.dg/modules/export-1.C: Adjust error messages. Remove > xfails for working case. Add new test case. > > Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com> > --- > gcc/cp/decl.cc | 21 ++++++++++++++++++--- > gcc/testsuite/g++.dg/modules/export-1.C | 16 +++++++++------- > 2 files changed, 27 insertions(+), 10 deletions(-) > > diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc > index 4a07c7e879b..bde9bd79d58 100644 > --- a/gcc/cp/decl.cc > +++ b/gcc/cp/decl.cc > @@ -2236,8 +2236,10 @@ duplicate_decls (tree newdecl, tree olddecl, bool hiding, bool was_hidden) > if (DECL_MODULE_EXPORT_P (STRIP_TEMPLATE (newdecl)) > && !DECL_MODULE_EXPORT_P (not_tmpl)) > { > - error ("conflicting exporting declaration %qD", newdecl); > - inform (olddecl_loc, "previous declaration %q#D here", olddecl); > + auto_diagnostic_group d; > + error ("conflicting exporting for declaration %qD", newdecl); > + inform (olddecl_loc, > + "previously declared here without exporting"); > } > } > else if (DECL_MODULE_EXPORT_P (newdecl)) > @@ -16249,11 +16251,24 @@ 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), "declared here"); > + inform (DECL_SOURCE_LOCATION (decl), "previously declared here"); > return error_mark_node; > } > > + tree not_tmpl = STRIP_TEMPLATE (decl); > + if (DECL_LANG_SPECIFIC (not_tmpl) > + && DECL_MODULE_ATTACH_P (not_tmpl) > + && !DECL_MODULE_EXPORT_P (not_tmpl) > + && module_exporting_p ()) > + { > + auto_diagnostic_group d; > + error ("conflicting exporting for declaration %qD", decl); > + inform (DECL_SOURCE_LOCATION (decl), > + "previously declared here without exporting"); > + } > + > tree maybe_tmpl = decl; > if (CLASS_TYPE_P (t) && CLASSTYPE_IS_TEMPLATE (t)) > maybe_tmpl = CLASSTYPE_TI_TEMPLATE (t); > diff --git a/gcc/testsuite/g++.dg/modules/export-1.C b/gcc/testsuite/g++.dg/modules/export-1.C > index 8ca696ebee0..3f93814d270 100644 > --- a/gcc/testsuite/g++.dg/modules/export-1.C > +++ b/gcc/testsuite/g++.dg/modules/export-1.C > @@ -4,19 +4,21 @@ export module frob; > // { dg-module-cmi !frob } > > int x (); > -export int x (); // { dg-error "conflicting exporting declaration" } > +export int x (); // { dg-error "conflicting exporting for declaration" } > > int y; > -export extern int y; // { dg-error "conflicting exporting declaration" } > +export extern int y; // { dg-error "conflicting exporting for declaration" } > > typedef int z; > -export typedef int z; // { dg-error "conflicting exporting declaration" } > +export typedef int z; // { dg-error "conflicting exporting for declaration" } > > template <typename T> int f (T); > -export template <typename T> int f (T); // { dg-error "conflicting exporting declaration" } > +export template <typename T> int f (T); // { dg-error "conflicting exporting for declaration" } > > -// doesn't go via duplicate_decls so we miss this for now > class A; > -export class A; // { dg-error "conflicting exporting declaration" "" { xfail *-*-* } } > +export class A; // { dg-error "conflicting exporting for declaration" } > > -// { dg-warning "due to errors" "" { target *-*-* } 0 } > +template <typename T> struct B; > +export template <typename T> struct B {}; // { dg-error "conflicting exporting for declaration" } > + > +// { dg-warning "due to errors" "" { target *-*-* } 0 }
diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc index 4a07c7e879b..bde9bd79d58 100644 --- a/gcc/cp/decl.cc +++ b/gcc/cp/decl.cc @@ -2236,8 +2236,10 @@ duplicate_decls (tree newdecl, tree olddecl, bool hiding, bool was_hidden) if (DECL_MODULE_EXPORT_P (STRIP_TEMPLATE (newdecl)) && !DECL_MODULE_EXPORT_P (not_tmpl)) { - error ("conflicting exporting declaration %qD", newdecl); - inform (olddecl_loc, "previous declaration %q#D here", olddecl); + auto_diagnostic_group d; + error ("conflicting exporting for declaration %qD", newdecl); + inform (olddecl_loc, + "previously declared here without exporting"); } } else if (DECL_MODULE_EXPORT_P (newdecl)) @@ -16249,11 +16251,24 @@ 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), "declared here"); + inform (DECL_SOURCE_LOCATION (decl), "previously declared here"); return error_mark_node; } + tree not_tmpl = STRIP_TEMPLATE (decl); + if (DECL_LANG_SPECIFIC (not_tmpl) + && DECL_MODULE_ATTACH_P (not_tmpl) + && !DECL_MODULE_EXPORT_P (not_tmpl) + && module_exporting_p ()) + { + auto_diagnostic_group d; + error ("conflicting exporting for declaration %qD", decl); + inform (DECL_SOURCE_LOCATION (decl), + "previously declared here without exporting"); + } + tree maybe_tmpl = decl; if (CLASS_TYPE_P (t) && CLASSTYPE_IS_TEMPLATE (t)) maybe_tmpl = CLASSTYPE_TI_TEMPLATE (t); diff --git a/gcc/testsuite/g++.dg/modules/export-1.C b/gcc/testsuite/g++.dg/modules/export-1.C index 8ca696ebee0..3f93814d270 100644 --- a/gcc/testsuite/g++.dg/modules/export-1.C +++ b/gcc/testsuite/g++.dg/modules/export-1.C @@ -4,19 +4,21 @@ export module frob; // { dg-module-cmi !frob } int x (); -export int x (); // { dg-error "conflicting exporting declaration" } +export int x (); // { dg-error "conflicting exporting for declaration" } int y; -export extern int y; // { dg-error "conflicting exporting declaration" } +export extern int y; // { dg-error "conflicting exporting for declaration" } typedef int z; -export typedef int z; // { dg-error "conflicting exporting declaration" } +export typedef int z; // { dg-error "conflicting exporting for declaration" } template <typename T> int f (T); -export template <typename T> int f (T); // { dg-error "conflicting exporting declaration" } +export template <typename T> int f (T); // { dg-error "conflicting exporting for declaration" } -// doesn't go via duplicate_decls so we miss this for now class A; -export class A; // { dg-error "conflicting exporting declaration" "" { xfail *-*-* } } +export class A; // { dg-error "conflicting exporting for declaration" } -// { dg-warning "due to errors" "" { target *-*-* } 0 } +template <typename T> struct B; +export template <typename T> struct B {}; // { dg-error "conflicting exporting for declaration" } + +// { dg-warning "due to errors" "" { target *-*-* } 0 }
I think the error message is still a little bit unclear but I couldn't come up with something clearer that was similarly concise and matching the existing style. (Also I noticed that the linked PR was assigned to Nathan but there hadn't been activity for a while, and I've been looking into these kinds of issues recently anyway so I thought I'd give it a go.) Bootstrapped and regtested on x86_64-pc-linux-gnu. I don't have write access. -- >8 -- Checks for exporting a declaration that was previously declared as not exported is implemented in 'duplicate_decls', but this doesn't handle declarations of classes. This patch adds these checks and slightly adjusts the associated error messages for clarity. PR c++/98885 gcc/cp/ChangeLog: * decl.cc (duplicate_decls): Adjust error message. (xref_tag): Adjust error message. Check exporting decl that is already declared as non-exporting. gcc/testsuite/ChangeLog: * g++.dg/modules/export-1.C: Adjust error messages. Remove xfails for working case. Add new test case. Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com> --- gcc/cp/decl.cc | 21 ++++++++++++++++++--- gcc/testsuite/g++.dg/modules/export-1.C | 16 +++++++++------- 2 files changed, 27 insertions(+), 10 deletions(-)