| Message ID | 20240215211019.340428-1-ppalka@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | c++/modules: relax diagnostic about GMF contents | expand |
On 2/15/24 16:10, Patrick Palka wrote: > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look > OK for trunk? > > -- >8 -- > > Issuing a hard error when the GMF doesn't contain preprocessing > directives is inconvenient for automated testcase reduction via cvise. > This patch relaxes this diagnostic into a pedwarn. > > gcc/cp/ChangeLog: > > * parser.cc (cp_parser_translation_unit): Relax GMF contents > error into a pedwarn. > > gcc/testsuite/ChangeLog: > > * g++.dg/modules/friend-6_a.C: Remove now unnecessary > preprocessing directives from GMF. > --- > gcc/cp/parser.cc | 6 +++--- > gcc/testsuite/g++.dg/modules/friend-6_a.C | 2 -- > 2 files changed, 3 insertions(+), 5 deletions(-) > > diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc > index 9d0914435fb..e60f0425035 100644 > --- a/gcc/cp/parser.cc > +++ b/gcc/cp/parser.cc > @@ -5253,9 +5253,9 @@ cp_parser_translation_unit (cp_parser* parser) > if (!warned) > { > warned = true; > - error_at (token->location, > - "global module fragment contents must be" > - " from preprocessor inclusion"); > + pedwarn (token->location, OPT_Wpedantic, > + "global module fragment contents must be" > + " from preprocessor inclusion"); Relaxing to pedwarn is fine, but I think it should be on by default, not just with -pedantic. So it should get a new option. Jason
On Thu, 15 Feb 2024, Jason Merrill wrote: > On 2/15/24 16:10, Patrick Palka wrote: > > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look > > OK for trunk? > > > > -- >8 -- > > > > Issuing a hard error when the GMF doesn't contain preprocessing > > directives is inconvenient for automated testcase reduction via cvise. > > This patch relaxes this diagnostic into a pedwarn. > > > > gcc/cp/ChangeLog: > > > > * parser.cc (cp_parser_translation_unit): Relax GMF contents > > error into a pedwarn. > > > > gcc/testsuite/ChangeLog: > > > > * g++.dg/modules/friend-6_a.C: Remove now unnecessary > > preprocessing directives from GMF. > > --- > > gcc/cp/parser.cc | 6 +++--- > > gcc/testsuite/g++.dg/modules/friend-6_a.C | 2 -- > > 2 files changed, 3 insertions(+), 5 deletions(-) > > > > diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc > > index 9d0914435fb..e60f0425035 100644 > > --- a/gcc/cp/parser.cc > > +++ b/gcc/cp/parser.cc > > @@ -5253,9 +5253,9 @@ cp_parser_translation_unit (cp_parser* parser) > > if (!warned) > > { > > warned = true; > > - error_at (token->location, > > - "global module fragment contents must be" > > - " from preprocessor inclusion"); > > + pedwarn (token->location, OPT_Wpedantic, > > + "global module fragment contents must be" > > + " from preprocessor inclusion"); > > Relaxing to pedwarn is fine, but I think it should be on by default, not just > with -pedantic. So it should get a new option. Ah, like so? I'm not sure about naming the option Wmodules-gmf-contents vs just Wgmf-contents, or something else... -- >8 -- Subject: [PATCH] c++/modules: relax diagnostic about GMF contents Issuing a hard error when the GMF doesn't contain preprocessing directives is inconvenient for automated testcase reduction via cvise. This patch relaxes this diagnostic into a pedwarn that can be disabled with -Wno-modules-gmf-contents. gcc/c-family/ChangeLog: * c.opt (Wmodules-gmf-contents): New warning. gcc/cp/ChangeLog: * parser.cc (cp_parser_translation_unit): Relax GMF contents error into a pedwarn. gcc/ChangeLog: * doc/invoke.texi (-Wno-modules-gmf-contents): Document. gcc/testsuite/ChangeLog: * g++.dg/modules/friend-6_a.C: Pass -Wno-modules-gmf-contents instead of -Wno-pedantic. Remove now unnecessary preprocessing directives from GMF. --- gcc/c-family/c.opt | 4 ++++ gcc/cp/parser.cc | 6 +++--- gcc/doc/invoke.texi | 7 +++++++ gcc/testsuite/g++.dg/modules/friend-6_a.C | 4 +--- 4 files changed, 15 insertions(+), 6 deletions(-) diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt index b7a4a1a68e3..6d5f3199b33 100644 --- a/gcc/c-family/c.opt +++ b/gcc/c-family/c.opt @@ -993,6 +993,10 @@ Wmissing-variable-declarations C ObjC Var(warn_missing_variable_declarations) Warning Warn about global variables without previous declarations. +Wmodules-gmf-contents +C++ ObjC++ Var(warn_modules_gmf_contents) Warning Init(1) +Warn about the global module fragment not containing only preprocessing directives. + Wmudflap C ObjC C++ ObjC++ WarnRemoved diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc index 9d0914435fb..eb709fc9e38 100644 --- a/gcc/cp/parser.cc +++ b/gcc/cp/parser.cc @@ -5253,9 +5253,9 @@ cp_parser_translation_unit (cp_parser* parser) if (!warned) { warned = true; - error_at (token->location, - "global module fragment contents must be" - " from preprocessor inclusion"); + pedwarn (token->location, OPT_Wmodules_gmf_contents, + "global module fragment contents must be" + " from preprocessor inclusion"); } } } diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index 511114cde59..e27fda88bc4 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -261,6 +261,7 @@ in the following sections. -Winvalid-constexpr -Winvalid-imported-macros -Wno-invalid-offsetof -Wno-literal-suffix -Wmismatched-new-delete -Wmismatched-tags +-Wno-modules-gmf-contents -Wmultiple-inheritance -Wnamespaces -Wnarrowing -Wnoexcept -Wnoexcept-type -Wnon-virtual-dtor -Wpessimizing-move -Wno-placement-new -Wplacement-new=@var{n} @@ -4568,6 +4569,12 @@ unresolved references due to the difference in the mangling of symbols declared with different class-keys. The option can be used either on its own or in conjunction with @option{-Wredundant-tags}. +@opindex Wno-modules-gmf-contents +@opindex Wmodules-gmf-contents +@item -Wno-modules-gmf-contents @r{(C++ and Objective-C++ only)} +Disable the diagnostic for when the global module fragment of a module +unit does not consist only of preprocessor directives. + @opindex Wmultiple-inheritance @opindex Wno-multiple-inheritance @item -Wmultiple-inheritance @r{(C++ and Objective-C++ only)} diff --git a/gcc/testsuite/g++.dg/modules/friend-6_a.C b/gcc/testsuite/g++.dg/modules/friend-6_a.C index 7493e8f262f..429fb460eea 100644 --- a/gcc/testsuite/g++.dg/modules/friend-6_a.C +++ b/gcc/testsuite/g++.dg/modules/friend-6_a.C @@ -1,11 +1,9 @@ -// { dg-additional-options "-fmodules-ts -Wno-pedantic" } +// { dg-additional-options "-fmodules-ts -Wno-modules-gmf-contents" } // { dg-module-cmi friend_6 } module; -# 1 "" 1 template <typename> struct Trans_NS___cxx11_basic_string { template <typename> friend class basic_stringbuf; }; template struct Trans_NS___cxx11_basic_string<char>; -# 6 "" 2 export module friend_6;
On Thu, 15 Feb 2024, Patrick Palka wrote: > On Thu, 15 Feb 2024, Jason Merrill wrote: > > > On 2/15/24 16:10, Patrick Palka wrote: > > > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look > > > OK for trunk? > > > > > > -- >8 -- > > > > > > Issuing a hard error when the GMF doesn't contain preprocessing > > > directives is inconvenient for automated testcase reduction via cvise. > > > This patch relaxes this diagnostic into a pedwarn. > > > > > > gcc/cp/ChangeLog: > > > > > > * parser.cc (cp_parser_translation_unit): Relax GMF contents > > > error into a pedwarn. > > > > > > gcc/testsuite/ChangeLog: > > > > > > * g++.dg/modules/friend-6_a.C: Remove now unnecessary > > > preprocessing directives from GMF. > > > --- > > > gcc/cp/parser.cc | 6 +++--- > > > gcc/testsuite/g++.dg/modules/friend-6_a.C | 2 -- > > > 2 files changed, 3 insertions(+), 5 deletions(-) > > > > > > diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc > > > index 9d0914435fb..e60f0425035 100644 > > > --- a/gcc/cp/parser.cc > > > +++ b/gcc/cp/parser.cc > > > @@ -5253,9 +5253,9 @@ cp_parser_translation_unit (cp_parser* parser) > > > if (!warned) > > > { > > > warned = true; > > > - error_at (token->location, > > > - "global module fragment contents must be" > > > - " from preprocessor inclusion"); > > > + pedwarn (token->location, OPT_Wpedantic, > > > + "global module fragment contents must be" > > > + " from preprocessor inclusion"); > > > > Relaxing to pedwarn is fine, but I think it should be on by default, not just > > with -pedantic. So it should get a new option. > > Ah, like so? I'm not sure about naming the option Wmodules-gmf-contents > vs just Wgmf-contents, or something else... Ping. > > -- >8 -- > > Subject: [PATCH] c++/modules: relax diagnostic about GMF contents > > Issuing a hard error when the GMF doesn't contain preprocessing > directives is inconvenient for automated testcase reduction via cvise. > This patch relaxes this diagnostic into a pedwarn that can be disabled > with -Wno-modules-gmf-contents. > > gcc/c-family/ChangeLog: > > * c.opt (Wmodules-gmf-contents): New warning. > > gcc/cp/ChangeLog: > > * parser.cc (cp_parser_translation_unit): Relax GMF contents > error into a pedwarn. > > gcc/ChangeLog: > > * doc/invoke.texi (-Wno-modules-gmf-contents): Document. > > gcc/testsuite/ChangeLog: > > * g++.dg/modules/friend-6_a.C: Pass -Wno-modules-gmf-contents > instead of -Wno-pedantic. Remove now unnecessary preprocessing > directives from GMF. > --- > gcc/c-family/c.opt | 4 ++++ > gcc/cp/parser.cc | 6 +++--- > gcc/doc/invoke.texi | 7 +++++++ > gcc/testsuite/g++.dg/modules/friend-6_a.C | 4 +--- > 4 files changed, 15 insertions(+), 6 deletions(-) > > diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt > index b7a4a1a68e3..6d5f3199b33 100644 > --- a/gcc/c-family/c.opt > +++ b/gcc/c-family/c.opt > @@ -993,6 +993,10 @@ Wmissing-variable-declarations > C ObjC Var(warn_missing_variable_declarations) Warning > Warn about global variables without previous declarations. > > +Wmodules-gmf-contents > +C++ ObjC++ Var(warn_modules_gmf_contents) Warning Init(1) > +Warn about the global module fragment not containing only preprocessing directives. > + > Wmudflap > C ObjC C++ ObjC++ WarnRemoved > > diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc > index 9d0914435fb..eb709fc9e38 100644 > --- a/gcc/cp/parser.cc > +++ b/gcc/cp/parser.cc > @@ -5253,9 +5253,9 @@ cp_parser_translation_unit (cp_parser* parser) > if (!warned) > { > warned = true; > - error_at (token->location, > - "global module fragment contents must be" > - " from preprocessor inclusion"); > + pedwarn (token->location, OPT_Wmodules_gmf_contents, > + "global module fragment contents must be" > + " from preprocessor inclusion"); > } > } > } > diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi > index 511114cde59..e27fda88bc4 100644 > --- a/gcc/doc/invoke.texi > +++ b/gcc/doc/invoke.texi > @@ -261,6 +261,7 @@ in the following sections. > -Winvalid-constexpr -Winvalid-imported-macros > -Wno-invalid-offsetof -Wno-literal-suffix > -Wmismatched-new-delete -Wmismatched-tags > +-Wno-modules-gmf-contents > -Wmultiple-inheritance -Wnamespaces -Wnarrowing > -Wnoexcept -Wnoexcept-type -Wnon-virtual-dtor > -Wpessimizing-move -Wno-placement-new -Wplacement-new=@var{n} > @@ -4568,6 +4569,12 @@ unresolved references due to the difference in the mangling of symbols > declared with different class-keys. The option can be used either on its > own or in conjunction with @option{-Wredundant-tags}. > > +@opindex Wno-modules-gmf-contents > +@opindex Wmodules-gmf-contents > +@item -Wno-modules-gmf-contents @r{(C++ and Objective-C++ only)} > +Disable the diagnostic for when the global module fragment of a module > +unit does not consist only of preprocessor directives. > + > @opindex Wmultiple-inheritance > @opindex Wno-multiple-inheritance > @item -Wmultiple-inheritance @r{(C++ and Objective-C++ only)} > diff --git a/gcc/testsuite/g++.dg/modules/friend-6_a.C b/gcc/testsuite/g++.dg/modules/friend-6_a.C > index 7493e8f262f..429fb460eea 100644 > --- a/gcc/testsuite/g++.dg/modules/friend-6_a.C > +++ b/gcc/testsuite/g++.dg/modules/friend-6_a.C > @@ -1,11 +1,9 @@ > -// { dg-additional-options "-fmodules-ts -Wno-pedantic" } > +// { dg-additional-options "-fmodules-ts -Wno-modules-gmf-contents" } > // { dg-module-cmi friend_6 } > > module; > -# 1 "" 1 > template <typename> struct Trans_NS___cxx11_basic_string { > template <typename> friend class basic_stringbuf; > }; > template struct Trans_NS___cxx11_basic_string<char>; > -# 6 "" 2 > export module friend_6; > -- > 2.44.0.rc1.15.g4fc51f00ef > >
On 2/15/24 16:51, Patrick Palka wrote: > On Thu, 15 Feb 2024, Jason Merrill wrote: > >> Relaxing to pedwarn is fine, but I think it should be on by default, not just >> with -pedantic. So it should get a new option. > > Ah, like so? I'm not sure about naming the option Wmodules-gmf-contents > vs just Wgmf-contents, or something else... Maybe -Wglobal-module? OK with that change. Jason
diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc index 9d0914435fb..e60f0425035 100644 --- a/gcc/cp/parser.cc +++ b/gcc/cp/parser.cc @@ -5253,9 +5253,9 @@ cp_parser_translation_unit (cp_parser* parser) if (!warned) { warned = true; - error_at (token->location, - "global module fragment contents must be" - " from preprocessor inclusion"); + pedwarn (token->location, OPT_Wpedantic, + "global module fragment contents must be" + " from preprocessor inclusion"); } } } diff --git a/gcc/testsuite/g++.dg/modules/friend-6_a.C b/gcc/testsuite/g++.dg/modules/friend-6_a.C index 7493e8f262f..c7868556a3e 100644 --- a/gcc/testsuite/g++.dg/modules/friend-6_a.C +++ b/gcc/testsuite/g++.dg/modules/friend-6_a.C @@ -2,10 +2,8 @@ // { dg-module-cmi friend_6 } module; -# 1 "" 1 template <typename> struct Trans_NS___cxx11_basic_string { template <typename> friend class basic_stringbuf; }; template struct Trans_NS___cxx11_basic_string<char>; -# 6 "" 2 export module friend_6;