Re: [PATCH] c++/modules: relax diagnostic about GMF contents

[Patrick Palka <ppalka@redhat.com>]

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;
-- 
2.44.0.rc1.15.g4fc51f00ef