public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [Patch] Fortran: Fix -Wno-missing-include-dirs handling [PR55534]
@ 2021-09-17 13:02 Tobias Burnus
  2021-09-17 18:45 ` Tobias Burnus
  0 siblings, 1 reply; 9+ messages in thread
From: Tobias Burnus @ 2021-09-17 13:02 UTC (permalink / raw)
  To: gcc-patches, fortran

[-- Attachment #1: Type: text/plain, Size: 2891 bytes --]

Short version:
* -Wno-missing-include-dirs  had no effect as the warning was always on
* For CPP-only options like -idirafter, no warning was shown.

This patch tries to address both, i.e. cpp's include-dir diagnostics are
shown as well – and silencing the diagnostic works as well.

OK for mainline?

Tobias

PS:  BACKGROUND and LONG DESCRIPTION

C/C++ by default have disabled the -Wmissing-include-dirs warning.
Fortran by default has that warning enabled.

The value is actually stored at two places (cf. c-family/c.opt):
   Wmissing-include-dirs
   ... CPP(warn_missing_include_dirs) Var(cpp_warn_missing_include_dirs) Init(0)

For CPP, that value always needs to initialized – and it is used
in gcc/incpath.c as
               cpp_options *opts = cpp_get_options (pfile);
               if (opts->warn_missing_include_dirs && cur->user_supplied_p)
                 cpp_warning (pfile, CPP_W_MISSING_INCLUDE_DIRS, "%s: %s",

Additionally, there is cpp_warn_missing_include_dirs which is used by
Fortran – and which consists of
   global_options.x_cpp_warn_missing_include_dirs
   global_options_set._cpp_warn_missing_include_dirs

The flag processing happens as described in
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55534#c11
in short:
   - First gfc_init_options is called
   - Then for reach command-line option gfc_handle_option
   - Finally gfc_post_options

Currently:
- gfc_init_options: Sets cpp_warn_missing_include_dirs
   (unconditionally as unset)
- gfc_handle_option: Always warns about the missing include dir
- before gfc_post_options: set_option is called, which sets
   cpp_warn_missing_include_dirs – but that's too late.

Additionally, as mentioned above – pfile's warn_missing_include_dirs
is never properly set.

  * * *

This patch fixes those issues:
* Now -Wno-missing-include-dirs does silence the warnings
* CPP now also properly does warn.

Example (initial version):
$ gfortran-trunk ../empty.f90 -c -cpp -idirafter /fdaf/ -I bar/ -Wmissing-include-dirs
f951: Warning: Nonexistent include directory ‘bar//’ [-Wmissing-include-dirs]
<built-in>: Warning: /fdaf/: No such file or directory
<built-in>: Warning: bar/: No such file or directory

In order to avoid the double output for -I, I disabled the Fortran output if
CPP is enabled. Additionally, I had to use the cpp_reason_option_codes to
print the flag in brackets.
Fixed/final output is:

<built-in>: Warning: /fdaf/: No such file or directory [-Wmissing-include-dirs]
<built-in>: Warning: bar/: No such file or directory [-Wmissing-include-dirs]

-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955

[-- Attachment #2: missing-incl.diff --]
[-- Type: text/x-patch, Size: 11469 bytes --]

Fortran: Fix -Wno-missing-include-dirs handling [PR55534]

gcc/fortran/ChangeLog:

	PR fortran/55534
	* cpp.c: Define GCC_C_COMMON_C for #include "options.h" to make
	cpp_reason_option_codes available.
	(gfc_cpp_register_include_paths): Make static, set pfile's
	warn_missing_include_dirs and move before caller.
	(gfc_cpp_init_cb): New, cb code moved from ...
	(gfc_cpp_init_0): ... here.
	(gfc_cpp_post_options): Call gfc_cpp_init_cb.
	(cb_cpp_diagnostic_cpp_option): New. As implemented in c-family
	to match CppReason flags to -W... names.
	(cb_cpp_diagnostic): Use it to replace single special case.
	* cpp.h (gfc_cpp_register_include_paths): Remove as now static.
	* gfortran.h (gfc_check_include_dirs): New prototype.
	* options.c (gfc_init_options): Don't set -Wmissing-include-dirs.
	(gfc_post_options): Set it here after commandline processing.
	* scanner.c (gfc_do_check_include_dirs, gfc_check_include_dirs):
	New. Diagnostic moved from ...
	(add_path_to_list): ... here, which came before cmdline processing.
	* scanner.h (struct gfc_directorylist): Reorder for alignment issues,
	add new 'bool warn'.

 gcc/fortran/cpp.c      | 106 ++++++++++++++++++++++++++++++-------------------
 gcc/fortran/cpp.h      |   2 -
 gcc/fortran/gfortran.h |   1 +
 gcc/fortran/options.c  |  14 +++----
 gcc/fortran/scanner.c  |  58 +++++++++++++++++++--------
 gcc/fortran/scanner.h  |   2 +-
 6 files changed, 116 insertions(+), 67 deletions(-)

diff --git a/gcc/fortran/cpp.c b/gcc/fortran/cpp.c
index 83c4517acdb..3ff895455e9 100644
--- a/gcc/fortran/cpp.c
+++ b/gcc/fortran/cpp.c
@@ -19,11 +19,15 @@ along with GCC; see the file COPYING3.  If not see
 #include "config.h"
 #include "system.h"
 #include "coretypes.h"
+
+#define GCC_C_COMMON_C
+#include "options.h"  /* For cpp_reason_option_codes. */
+#undef GCC_C_COMMON_C
+
 #include "target.h"
 #include "gfortran.h"
 #include "diagnostic.h"
 
-
 #include "toplev.h"
 
 #include "../../libcpp/internal.h"
@@ -240,6 +244,18 @@ gfc_cpp_temporary_file (void)
   return gfc_cpp_option.temporary_filename;
 }
 
+static void
+gfc_cpp_register_include_paths (void)
+{
+  int cxx_stdinc = 0;
+  cpp_get_options (cpp_in)->warn_missing_include_dirs
+    = global_options.x_cpp_warn_missing_include_dirs;
+  register_include_chains (cpp_in, gfc_cpp_option.sysroot,
+			   gfc_cpp_option.prefix, gfc_cpp_option.multilib,
+			   gfc_cpp_option.standard_include_paths, cxx_stdinc,
+			   gfc_cpp_option.verbose);
+}
+
 void
 gfc_cpp_init_options (unsigned int decoded_options_count,
 		      struct cl_decoded_option *decoded_options ATTRIBUTE_UNUSED)
@@ -435,6 +451,37 @@ gfc_cpp_handle_option (size_t scode, const char *arg, int value ATTRIBUTE_UNUSED
   return result;
 }
 
+/* This function needs to be called before gfc_cpp_register_include_paths
+   as the latter may diagnose missing include directories.  */
+static void
+gfc_cpp_init_cb (void)
+{
+  struct cpp_callbacks *cb;
+
+  cb = cpp_get_callbacks (cpp_in);
+  cb->file_change = cb_file_change;
+  cb->line_change = cb_line_change;
+  cb->ident = cb_ident;
+  cb->def_pragma = cb_def_pragma;
+  cb->diagnostic = cb_cpp_diagnostic;
+
+  if (gfc_cpp_option.dump_includes)
+    cb->include = cb_include;
+
+  if ((gfc_cpp_option.dump_macros == 'D')
+      || (gfc_cpp_option.dump_macros == 'N'))
+    {
+      cb->define = cb_define;
+      cb->undef  = cb_undef;
+    }
+
+  if (gfc_cpp_option.dump_macros == 'U')
+    {
+      cb->before_define = dump_queued_macros;
+      cb->used_define = cb_used_define;
+      cb->used_undef = cb_used_undef;
+    }
+}
 
 void
 gfc_cpp_post_options (void)
@@ -498,6 +545,7 @@ gfc_cpp_post_options (void)
      way libcpp will do it, namely, with no charset conversion but with
      skipping of a UTF-8 BOM if present.  */
   diagnostic_initialize_input_context (global_dc, nullptr, true);
+  gfc_cpp_init_cb ();
 
   gfc_cpp_register_include_paths ();
 }
@@ -506,32 +554,6 @@ gfc_cpp_post_options (void)
 void
 gfc_cpp_init_0 (void)
 {
-  struct cpp_callbacks *cb;
-
-  cb = cpp_get_callbacks (cpp_in);
-  cb->file_change = cb_file_change;
-  cb->line_change = cb_line_change;
-  cb->ident = cb_ident;
-  cb->def_pragma = cb_def_pragma;
-  cb->diagnostic = cb_cpp_diagnostic;
-
-  if (gfc_cpp_option.dump_includes)
-    cb->include = cb_include;
-
-  if ((gfc_cpp_option.dump_macros == 'D')
-      || (gfc_cpp_option.dump_macros == 'N'))
-    {
-      cb->define = cb_define;
-      cb->undef  = cb_undef;
-    }
-
-  if (gfc_cpp_option.dump_macros == 'U')
-    {
-      cb->before_define = dump_queued_macros;
-      cb->used_define = cb_used_define;
-      cb->used_undef = cb_used_undef;
-    }
-
   /* Initialize the print structure.  Setting print.src_line to -1 here is
      a trick to guarantee that the first token of the file will cause
      a linemarker to be output by maybe_print_line.  */
@@ -723,17 +745,6 @@ gfc_cpp_add_include_path_after (char *path, bool user_supplied)
   add_path (path, INC_AFTER, cxx_aware, user_supplied);
 }
 
-void
-gfc_cpp_register_include_paths (void)
-{
-  int cxx_stdinc = 0;
-  register_include_chains (cpp_in, gfc_cpp_option.sysroot,
-			   gfc_cpp_option.prefix, gfc_cpp_option.multilib,
-			   gfc_cpp_option.standard_include_paths, cxx_stdinc,
-			   gfc_cpp_option.verbose);
-}
-
-
 
 static void scan_translation_unit_trad (cpp_reader *);
 static void account_for_newlines (const unsigned char *, size_t);
@@ -1043,6 +1054,21 @@ cb_used_define (cpp_reader *pfile, location_t line ATTRIBUTE_UNUSED,
   cpp_define_queue = q;
 }
 
+/* Return the gcc option code associated with the reason for a cpp
+   message, or 0 if none.  */
+
+static int
+cb_cpp_diagnostic_cpp_option (enum cpp_warning_reason reason)
+{
+  const struct cpp_reason_option_codes_t *entry;
+
+  for (entry = cpp_reason_option_codes; entry->reason != CPP_W_NONE; entry++)
+    if (entry->reason == reason)
+      return entry->option_code;
+  return 0;
+}
+
+
 /* Callback from cpp_error for PFILE to print diagnostics from the
    preprocessor.  The diagnostic is of type LEVEL, with REASON set
    to the reason code if LEVEL is represents a warning, at location
@@ -1089,8 +1115,8 @@ cb_cpp_diagnostic (cpp_reader *pfile ATTRIBUTE_UNUSED,
     }
   diagnostic_set_info_translated (&diagnostic, msg, ap,
 				  richloc, dlevel);
-  if (reason == CPP_W_WARNING_DIRECTIVE)
-    diagnostic_override_option_index (&diagnostic, OPT_Wcpp);
+  diagnostic_override_option_index (&diagnostic,
+				    cb_cpp_diagnostic_cpp_option (reason));
   ret = diagnostic_report_diagnostic (global_dc, &diagnostic);
   if (level == CPP_DL_WARNING_SYSHDR)
     global_dc->dc_warn_system_headers = save_warn_system_headers;
diff --git a/gcc/fortran/cpp.h b/gcc/fortran/cpp.h
index f642c7745f7..5cb7e5a9c34 100644
--- a/gcc/fortran/cpp.h
+++ b/gcc/fortran/cpp.h
@@ -50,6 +50,4 @@ void gfc_cpp_done (void);
 void gfc_cpp_add_include_path (char *path, bool user_supplied);
 void gfc_cpp_add_include_path_after (char *path, bool user_supplied);
 
-void gfc_cpp_register_include_paths (void);
-
 #endif /* GFC_CPP_H */
diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h
index fdf556eef3d..b02c434295c 100644
--- a/gcc/fortran/gfortran.h
+++ b/gcc/fortran/gfortran.h
@@ -3031,6 +3031,7 @@ void gfc_scanner_init_1 (void);
 void gfc_add_include_path (const char *, bool, bool, bool);
 void gfc_add_intrinsic_modules_path (const char *);
 void gfc_release_include_path (void);
+void gfc_check_include_dirs (void);
 FILE *gfc_open_included_file (const char *, bool, bool);
 
 int gfc_at_end (void);
diff --git a/gcc/fortran/options.c b/gcc/fortran/options.c
index 847e20e8829..a1ac760dd96 100644
--- a/gcc/fortran/options.c
+++ b/gcc/fortran/options.c
@@ -159,14 +159,7 @@ gfc_init_options (unsigned int decoded_options_count,
 			   | GFC_FPE_UNDERFLOW;
   gfc_option.rtcheck = 0;
 
-  /* ??? Wmissing-include-dirs is disabled by default in C/C++ but
-     enabled by default in Fortran.  Ideally, we should express this
-     in .opt, but that is not supported yet.  */
-  SET_OPTION_IF_UNSET (&global_options, &global_options_set,
-		       cpp_warn_missing_include_dirs, 1);
-
   set_dec_flags (0);
-
   set_default_std_flags ();
 
   /* Initialize cpp-related options.  */
@@ -260,6 +253,13 @@ gfc_post_options (const char **pfilename)
   char *source_path;
   int i;
 
+  /* This needs to be after the commandline has been processed.
+     In Fortran, the options is by default enabled, in C/C++
+     by default disabled.  */
+  SET_OPTION_IF_UNSET (&global_options, &global_options_set,
+		       cpp_warn_missing_include_dirs, 1);
+  gfc_check_include_dirs ();
+
   /* Finalize DEC flags.  */
   post_dec_flags (flag_dec);
 
diff --git a/gcc/fortran/scanner.c b/gcc/fortran/scanner.c
index 39db0994b62..93446c90b0e 100644
--- a/gcc/fortran/scanner.c
+++ b/gcc/fortran/scanner.c
@@ -298,6 +298,46 @@ gfc_scanner_done_1 (void)
     }
 }
 
+/* In order that -W(no-)missing-include-dirs works, the diagnostic can only be
+   run after processing the commandline.  */
+static void
+gfc_do_check_include_dirs (gfc_directorylist **list)
+{
+  struct stat st;
+  gfc_directorylist *prev, *q, *n;
+  prev = NULL;
+  n = *list;
+  while (n)
+    {
+      q = n; n = n->next;
+      if (stat (q->path, &st))
+	{
+	  if (errno != ENOENT)
+	    gfc_warning_now (0, "Include directory %qs: %s", q->path,
+			     xstrerror(errno));
+	  else if (q->warn && !gfc_cpp_enabled ())
+	    gfc_warning_now (OPT_Wmissing_include_dirs,
+			     "Nonexistent include directory %qs", q->path);
+	}
+      else if (!S_ISDIR (st.st_mode))
+	gfc_fatal_error ("%qs is not a directory", q->path);
+      else
+	continue;
+      if (prev == NULL)
+	*list = n;
+      else
+	prev->next = n;
+      free (q->path);
+      free (q);
+    }
+}
+
+void
+gfc_check_include_dirs ()
+{
+  gfc_do_check_include_dirs (&include_dirs);
+  gfc_do_check_include_dirs (&intrinsic_modules_dirs);
+}
 
 /* Adds path to the list pointed to by list.  */
 
@@ -308,7 +348,6 @@ add_path_to_list (gfc_directorylist **list, const char *path,
   gfc_directorylist *dir;
   const char *p;
   char *q;
-  struct stat st;
   size_t len;
   int i;
   
@@ -326,22 +365,6 @@ add_path_to_list (gfc_directorylist **list, const char *path,
   while (i >=0 && IS_DIR_SEPARATOR (q[i]))
     q[i--] = '\0';
 
-  if (stat (q, &st))
-    {
-      if (errno != ENOENT)
-	gfc_warning_now (0, "Include directory %qs: %s", path,
-			 xstrerror(errno));
-      else if (warn)
-	gfc_warning_now (OPT_Wmissing_include_dirs,
-			 "Nonexistent include directory %qs", path);
-      return;
-    }
-  else if (!S_ISDIR (st.st_mode))
-    {
-      gfc_fatal_error ("%qs is not a directory", path);
-      return;
-    }
-
   if (head || *list == NULL)
     {
       dir = XCNEW (gfc_directorylist);
@@ -362,6 +385,7 @@ add_path_to_list (gfc_directorylist **list, const char *path,
   if (head)
     *list = dir;
   dir->use_for_modules = use_for_modules;
+  dir->warn = warn;
   dir->path = XCNEWVEC (char, strlen (p) + 2);
   strcpy (dir->path, p);
   strcat (dir->path, "/");	/* make '/' last character */
diff --git a/gcc/fortran/scanner.h b/gcc/fortran/scanner.h
index 0bad63f4ffb..8782fe6ccc5 100644
--- a/gcc/fortran/scanner.h
+++ b/gcc/fortran/scanner.h
@@ -23,8 +23,8 @@ along with GCC; see the file COPYING3.  If not see
 typedef struct gfc_directorylist
 {
   char *path;
-  bool use_for_modules;
   struct gfc_directorylist *next;
+  bool use_for_modules, warn;
 }
 gfc_directorylist;
 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Patch] Fortran: Fix -Wno-missing-include-dirs handling [PR55534]
  2021-09-17 13:02 [Patch] Fortran: Fix -Wno-missing-include-dirs handling [PR55534] Tobias Burnus
@ 2021-09-17 18:45 ` Tobias Burnus
  2021-09-20 20:33   ` Tobias Burnus
  0 siblings, 1 reply; 9+ messages in thread
From: Tobias Burnus @ 2021-09-17 18:45 UTC (permalink / raw)
  To: gcc-patches, fortran

[-- Attachment #1: Type: text/plain, Size: 4212 bytes --]

I seemingly messed up a bit in previous patch – corrected version attached.

OK?

Tobias

PS: Due to now enabling the missing-include-dir warning also for cpp,the following
warning show up during build. This seems to be specific to libgfortran building,
libgomp works and real-world code also does not seem to be affected:
<built-in>: Error: <instdir>/x86_64-pc-linux-gnu/include: No such file or directory [-Werror=missing-include-dirs]
<built-in>: Error: <instdir>/x86_64-pc-linux-gnu/sys-include: No such file or directory [-Werror=missing-include-dirs]
<built-in>: Error: finclude: No such file or directory [-Werror=missing-include-dirs]

The latter is due to the driver adding '-fintrinsic-modules-path finclude'
when calling f951. I think the rest is a side effect of running with -B
and other build trickery.

The warnings do not trigger when compiling the Fortran file in libgomp nor for
a quick real-world case (which uses gfortran in a normal way not with -B etc.).
Thus, I think it should be fine.
Alternatively, we could think of reducing the noisiness. Thoughts?

PPS: Besides this, the following still applies:

On 17.09.21 15:02, Tobias Burnus wrote:
> Short version:
> * -Wno-missing-include-dirs  had no effect as the warning was always on
> * For CPP-only options like -idirafter, no warning was shown.
>
> This patch tries to address both, i.e. cpp's include-dir diagnostics are
> shown as well – and silencing the diagnostic works as well.
>
> OK for mainline?
>
> Tobias
>
> PS:  BACKGROUND and LONG DESCRIPTION
>
> C/C++ by default have disabled the -Wmissing-include-dirs warning.
> Fortran by default has that warning enabled.
>
> The value is actually stored at two places (cf. c-family/c.opt):
>   Wmissing-include-dirs
>   ... CPP(warn_missing_include_dirs)
> Var(cpp_warn_missing_include_dirs) Init(0)
>
> For CPP, that value always needs to initialized – and it is used
> in gcc/incpath.c as
>               cpp_options *opts = cpp_get_options (pfile);
>               if (opts->warn_missing_include_dirs &&
> cur->user_supplied_p)
>                 cpp_warning (pfile, CPP_W_MISSING_INCLUDE_DIRS, "%s: %s",
>
> Additionally, there is cpp_warn_missing_include_dirs which is used by
> Fortran – and which consists of
>   global_options.x_cpp_warn_missing_include_dirs
>   global_options_set._cpp_warn_missing_include_dirs
>
> The flag processing happens as described in
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55534#c11
> in short:
>   - First gfc_init_options is called
>   - Then for reach command-line option gfc_handle_option
>   - Finally gfc_post_options
>
> Currently:
> - gfc_init_options: Sets cpp_warn_missing_include_dirs
>   (unconditionally as unset)
> - gfc_handle_option: Always warns about the missing include dir
> - before gfc_post_options: set_option is called, which sets
>   cpp_warn_missing_include_dirs – but that's too late.
>
> Additionally, as mentioned above – pfile's warn_missing_include_dirs
> is never properly set.
>
>  * * *
>
> This patch fixes those issues:
> * Now -Wno-missing-include-dirs does silence the warnings
> * CPP now also properly does warn.
>
> Example (initial version):
> $ gfortran-trunk ../empty.f90 -c -cpp -idirafter /fdaf/ -I bar/
> -Wmissing-include-dirs
> f951: Warning: Nonexistent include directory ‘bar//’
> [-Wmissing-include-dirs]
> <built-in>: Warning: /fdaf/: No such file or directory
> <built-in>: Warning: bar/: No such file or directory
>
> In order to avoid the double output for -I, I disabled the Fortran
> output if
> CPP is enabled. Additionally, I had to use the cpp_reason_option_codes to
> print the flag in brackets.
> Fixed/final output is:
>
> <built-in>: Warning: /fdaf/: No such file or directory
> [-Wmissing-include-dirs]
> <built-in>: Warning: bar/: No such file or directory
> [-Wmissing-include-dirs]
>
-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955

[-- Attachment #2: missing-incl-v2.diff --]
[-- Type: text/x-patch, Size: 13017 bytes --]

Fortran: Fix -Wno-missing-include-dirs handling [PR55534]

gcc/fortran/ChangeLog:

	PR fortran/55534
	* cpp.c: Define GCC_C_COMMON_C for #include "options.h" to make
	cpp_reason_option_codes available.
	(gfc_cpp_register_include_paths): Make static, set pfile's
	warn_missing_include_dirs and move before caller.
	(gfc_cpp_init_cb): New, cb code moved from ...
	(gfc_cpp_init_0): ... here.
	(gfc_cpp_post_options): Call gfc_cpp_init_cb.
	(cb_cpp_diagnostic_cpp_option): New. As implemented in c-family
	to match CppReason flags to -W... names.
	(cb_cpp_diagnostic): Use it to replace single special case.
	* cpp.h (gfc_cpp_register_include_paths): Remove as now static.
	* gfortran.h (gfc_check_include_dirs): New prototype.
	* options.c (gfc_init_options): Don't set -Wmissing-include-dirs.
	(gfc_post_options): Set it here after commandline processing.
	* scanner.c (gfc_do_check_include_dirs, gfc_check_include_dirs):
	New. Diagnostic moved from ...
	(add_path_to_list): ... here, which came before cmdline processing.
	* scanner.h (struct gfc_directorylist): Reorder for alignment issues,
	add new 'bool warn'.

libgfortran/ChangeLog:
	PR fortran/55534
	* configure.ac (AM_FCFLAGS): Add -Wno-missing-include-dirs.
	* configure: Regenerate.


 gcc/fortran/cpp.c        | 106 +++++++++++++++++++++++++++++------------------
 gcc/fortran/cpp.h        |   2 -
 gcc/fortran/gfortran.h   |   1 +
 gcc/fortran/options.c    |  14 +++----
 gcc/fortran/scanner.c    |  61 +++++++++++++++++++--------
 gcc/fortran/scanner.h    |   2 +-
 libgfortran/configure    |   2 +-
 libgfortran/configure.ac |   2 +-
 8 files changed, 121 insertions(+), 69 deletions(-)

diff --git a/gcc/fortran/cpp.c b/gcc/fortran/cpp.c
index 83c4517acdb..3ff895455e9 100644
--- a/gcc/fortran/cpp.c
+++ b/gcc/fortran/cpp.c
@@ -19,11 +19,15 @@ along with GCC; see the file COPYING3.  If not see
 #include "config.h"
 #include "system.h"
 #include "coretypes.h"
+
+#define GCC_C_COMMON_C
+#include "options.h"  /* For cpp_reason_option_codes. */
+#undef GCC_C_COMMON_C
+
 #include "target.h"
 #include "gfortran.h"
 #include "diagnostic.h"
 
-
 #include "toplev.h"
 
 #include "../../libcpp/internal.h"
@@ -240,6 +244,18 @@ gfc_cpp_temporary_file (void)
   return gfc_cpp_option.temporary_filename;
 }
 
+static void
+gfc_cpp_register_include_paths (void)
+{
+  int cxx_stdinc = 0;
+  cpp_get_options (cpp_in)->warn_missing_include_dirs
+    = global_options.x_cpp_warn_missing_include_dirs;
+  register_include_chains (cpp_in, gfc_cpp_option.sysroot,
+			   gfc_cpp_option.prefix, gfc_cpp_option.multilib,
+			   gfc_cpp_option.standard_include_paths, cxx_stdinc,
+			   gfc_cpp_option.verbose);
+}
+
 void
 gfc_cpp_init_options (unsigned int decoded_options_count,
 		      struct cl_decoded_option *decoded_options ATTRIBUTE_UNUSED)
@@ -435,6 +451,37 @@ gfc_cpp_handle_option (size_t scode, const char *arg, int value ATTRIBUTE_UNUSED
   return result;
 }
 
+/* This function needs to be called before gfc_cpp_register_include_paths
+   as the latter may diagnose missing include directories.  */
+static void
+gfc_cpp_init_cb (void)
+{
+  struct cpp_callbacks *cb;
+
+  cb = cpp_get_callbacks (cpp_in);
+  cb->file_change = cb_file_change;
+  cb->line_change = cb_line_change;
+  cb->ident = cb_ident;
+  cb->def_pragma = cb_def_pragma;
+  cb->diagnostic = cb_cpp_diagnostic;
+
+  if (gfc_cpp_option.dump_includes)
+    cb->include = cb_include;
+
+  if ((gfc_cpp_option.dump_macros == 'D')
+      || (gfc_cpp_option.dump_macros == 'N'))
+    {
+      cb->define = cb_define;
+      cb->undef  = cb_undef;
+    }
+
+  if (gfc_cpp_option.dump_macros == 'U')
+    {
+      cb->before_define = dump_queued_macros;
+      cb->used_define = cb_used_define;
+      cb->used_undef = cb_used_undef;
+    }
+}
 
 void
 gfc_cpp_post_options (void)
@@ -498,6 +545,7 @@ gfc_cpp_post_options (void)
      way libcpp will do it, namely, with no charset conversion but with
      skipping of a UTF-8 BOM if present.  */
   diagnostic_initialize_input_context (global_dc, nullptr, true);
+  gfc_cpp_init_cb ();
 
   gfc_cpp_register_include_paths ();
 }
@@ -506,32 +554,6 @@ gfc_cpp_post_options (void)
 void
 gfc_cpp_init_0 (void)
 {
-  struct cpp_callbacks *cb;
-
-  cb = cpp_get_callbacks (cpp_in);
-  cb->file_change = cb_file_change;
-  cb->line_change = cb_line_change;
-  cb->ident = cb_ident;
-  cb->def_pragma = cb_def_pragma;
-  cb->diagnostic = cb_cpp_diagnostic;
-
-  if (gfc_cpp_option.dump_includes)
-    cb->include = cb_include;
-
-  if ((gfc_cpp_option.dump_macros == 'D')
-      || (gfc_cpp_option.dump_macros == 'N'))
-    {
-      cb->define = cb_define;
-      cb->undef  = cb_undef;
-    }
-
-  if (gfc_cpp_option.dump_macros == 'U')
-    {
-      cb->before_define = dump_queued_macros;
-      cb->used_define = cb_used_define;
-      cb->used_undef = cb_used_undef;
-    }
-
   /* Initialize the print structure.  Setting print.src_line to -1 here is
      a trick to guarantee that the first token of the file will cause
      a linemarker to be output by maybe_print_line.  */
@@ -723,17 +745,6 @@ gfc_cpp_add_include_path_after (char *path, bool user_supplied)
   add_path (path, INC_AFTER, cxx_aware, user_supplied);
 }
 
-void
-gfc_cpp_register_include_paths (void)
-{
-  int cxx_stdinc = 0;
-  register_include_chains (cpp_in, gfc_cpp_option.sysroot,
-			   gfc_cpp_option.prefix, gfc_cpp_option.multilib,
-			   gfc_cpp_option.standard_include_paths, cxx_stdinc,
-			   gfc_cpp_option.verbose);
-}
-
-
 
 static void scan_translation_unit_trad (cpp_reader *);
 static void account_for_newlines (const unsigned char *, size_t);
@@ -1043,6 +1054,21 @@ cb_used_define (cpp_reader *pfile, location_t line ATTRIBUTE_UNUSED,
   cpp_define_queue = q;
 }
 
+/* Return the gcc option code associated with the reason for a cpp
+   message, or 0 if none.  */
+
+static int
+cb_cpp_diagnostic_cpp_option (enum cpp_warning_reason reason)
+{
+  const struct cpp_reason_option_codes_t *entry;
+
+  for (entry = cpp_reason_option_codes; entry->reason != CPP_W_NONE; entry++)
+    if (entry->reason == reason)
+      return entry->option_code;
+  return 0;
+}
+
+
 /* Callback from cpp_error for PFILE to print diagnostics from the
    preprocessor.  The diagnostic is of type LEVEL, with REASON set
    to the reason code if LEVEL is represents a warning, at location
@@ -1089,8 +1115,8 @@ cb_cpp_diagnostic (cpp_reader *pfile ATTRIBUTE_UNUSED,
     }
   diagnostic_set_info_translated (&diagnostic, msg, ap,
 				  richloc, dlevel);
-  if (reason == CPP_W_WARNING_DIRECTIVE)
-    diagnostic_override_option_index (&diagnostic, OPT_Wcpp);
+  diagnostic_override_option_index (&diagnostic,
+				    cb_cpp_diagnostic_cpp_option (reason));
   ret = diagnostic_report_diagnostic (global_dc, &diagnostic);
   if (level == CPP_DL_WARNING_SYSHDR)
     global_dc->dc_warn_system_headers = save_warn_system_headers;
diff --git a/gcc/fortran/cpp.h b/gcc/fortran/cpp.h
index f642c7745f7..5cb7e5a9c34 100644
--- a/gcc/fortran/cpp.h
+++ b/gcc/fortran/cpp.h
@@ -50,6 +50,4 @@ void gfc_cpp_done (void);
 void gfc_cpp_add_include_path (char *path, bool user_supplied);
 void gfc_cpp_add_include_path_after (char *path, bool user_supplied);
 
-void gfc_cpp_register_include_paths (void);
-
 #endif /* GFC_CPP_H */
diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h
index fdf556eef3d..b02c434295c 100644
--- a/gcc/fortran/gfortran.h
+++ b/gcc/fortran/gfortran.h
@@ -3031,6 +3031,7 @@ void gfc_scanner_init_1 (void);
 void gfc_add_include_path (const char *, bool, bool, bool);
 void gfc_add_intrinsic_modules_path (const char *);
 void gfc_release_include_path (void);
+void gfc_check_include_dirs (void);
 FILE *gfc_open_included_file (const char *, bool, bool);
 
 int gfc_at_end (void);
diff --git a/gcc/fortran/options.c b/gcc/fortran/options.c
index 847e20e8829..a1ac760dd96 100644
--- a/gcc/fortran/options.c
+++ b/gcc/fortran/options.c
@@ -159,14 +159,7 @@ gfc_init_options (unsigned int decoded_options_count,
 			   | GFC_FPE_UNDERFLOW;
   gfc_option.rtcheck = 0;
 
-  /* ??? Wmissing-include-dirs is disabled by default in C/C++ but
-     enabled by default in Fortran.  Ideally, we should express this
-     in .opt, but that is not supported yet.  */
-  SET_OPTION_IF_UNSET (&global_options, &global_options_set,
-		       cpp_warn_missing_include_dirs, 1);
-
   set_dec_flags (0);
-
   set_default_std_flags ();
 
   /* Initialize cpp-related options.  */
@@ -260,6 +253,13 @@ gfc_post_options (const char **pfilename)
   char *source_path;
   int i;
 
+  /* This needs to be after the commandline has been processed.
+     In Fortran, the options is by default enabled, in C/C++
+     by default disabled.  */
+  SET_OPTION_IF_UNSET (&global_options, &global_options_set,
+		       cpp_warn_missing_include_dirs, 1);
+  gfc_check_include_dirs ();
+
   /* Finalize DEC flags.  */
   post_dec_flags (flag_dec);
 
diff --git a/gcc/fortran/scanner.c b/gcc/fortran/scanner.c
index 39db0994b62..7a76ec7162e 100644
--- a/gcc/fortran/scanner.c
+++ b/gcc/fortran/scanner.c
@@ -298,6 +298,49 @@ gfc_scanner_done_1 (void)
     }
 }
 
+/* In order that -W(no-)missing-include-dirs works, the diagnostic can only be
+   run after processing the commandline.  */
+static void
+gfc_do_check_include_dirs (gfc_directorylist **list)
+{
+  struct stat st;
+  gfc_directorylist *prev, *q, *n;
+  prev = NULL;
+  n = *list;
+  while (n)
+    {
+      q = n; n = n->next;
+      if (stat (q->path, &st))
+	{
+	  if (errno != ENOENT)
+	    gfc_warning_now (0, "Include directory %qs: %s", q->path,
+			     xstrerror(errno));
+	  else if (q->warn && !gfc_cpp_enabled ())
+	    gfc_warning_now (OPT_Wmissing_include_dirs,
+			     "Nonexistent include directory %qs", q->path);
+	}
+      else if (!S_ISDIR (st.st_mode))
+	gfc_fatal_error ("%qs is not a directory", q->path);
+      else
+	{
+	  prev = q;
+	  continue;
+	}
+      if (prev == NULL)
+	*list = n;
+      else
+	prev->next = n;
+      free (q->path);
+      free (q);
+    }
+}
+
+void
+gfc_check_include_dirs ()
+{
+  gfc_do_check_include_dirs (&include_dirs);
+  gfc_do_check_include_dirs (&intrinsic_modules_dirs);
+}
 
 /* Adds path to the list pointed to by list.  */
 
@@ -308,7 +351,6 @@ add_path_to_list (gfc_directorylist **list, const char *path,
   gfc_directorylist *dir;
   const char *p;
   char *q;
-  struct stat st;
   size_t len;
   int i;
   
@@ -326,22 +368,6 @@ add_path_to_list (gfc_directorylist **list, const char *path,
   while (i >=0 && IS_DIR_SEPARATOR (q[i]))
     q[i--] = '\0';
 
-  if (stat (q, &st))
-    {
-      if (errno != ENOENT)
-	gfc_warning_now (0, "Include directory %qs: %s", path,
-			 xstrerror(errno));
-      else if (warn)
-	gfc_warning_now (OPT_Wmissing_include_dirs,
-			 "Nonexistent include directory %qs", path);
-      return;
-    }
-  else if (!S_ISDIR (st.st_mode))
-    {
-      gfc_fatal_error ("%qs is not a directory", path);
-      return;
-    }
-
   if (head || *list == NULL)
     {
       dir = XCNEW (gfc_directorylist);
@@ -362,6 +388,7 @@ add_path_to_list (gfc_directorylist **list, const char *path,
   if (head)
     *list = dir;
   dir->use_for_modules = use_for_modules;
+  dir->warn = warn;
   dir->path = XCNEWVEC (char, strlen (p) + 2);
   strcpy (dir->path, p);
   strcat (dir->path, "/");	/* make '/' last character */
diff --git a/gcc/fortran/scanner.h b/gcc/fortran/scanner.h
index 0bad63f4ffb..8782fe6ccc5 100644
--- a/gcc/fortran/scanner.h
+++ b/gcc/fortran/scanner.h
@@ -23,8 +23,8 @@ along with GCC; see the file COPYING3.  If not see
 typedef struct gfc_directorylist
 {
   char *path;
-  bool use_for_modules;
   struct gfc_directorylist *next;
+  bool use_for_modules, warn;
 }
 gfc_directorylist;
 
diff --git a/libgfortran/configure b/libgfortran/configure
index 4810b9b032e..350159aa4fe 100755
--- a/libgfortran/configure
+++ b/libgfortran/configure
@@ -5985,7 +5985,7 @@ fi
 
 # Add -Wall -fno-repack-arrays -fno-underscoring if we are using GCC.
 if test "x$GCC" = "xyes"; then
-  AM_FCFLAGS="-I . -Wall -Werror -fimplicit-none -fno-repack-arrays -fno-underscoring"
+  AM_FCFLAGS="-I . -Wall -Werror -fimplicit-none -fno-repack-arrays -fno-underscoring -Wno-missing-include-dirs"
   ## We like to use C11 and C99 routines when available.  This makes
   ## sure that
   ## __STDC_VERSION__ is set such that libc includes make them available.
diff --git a/libgfortran/configure.ac b/libgfortran/configure.ac
index a77509801e6..a3550b4e5d0 100644
--- a/libgfortran/configure.ac
+++ b/libgfortran/configure.ac
@@ -146,7 +146,7 @@ AM_PROG_CC_C_O
 
 # Add -Wall -fno-repack-arrays -fno-underscoring if we are using GCC.
 if test "x$GCC" = "xyes"; then
-  AM_FCFLAGS="-I . -Wall -Werror -fimplicit-none -fno-repack-arrays -fno-underscoring"
+  AM_FCFLAGS="-I . -Wall -Werror -fimplicit-none -fno-repack-arrays -fno-underscoring -Wno-missing-include-dirs"
   ## We like to use C11 and C99 routines when available.  This makes
   ## sure that
   ## __STDC_VERSION__ is set such that libc includes make them available.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Patch] Fortran: Fix -Wno-missing-include-dirs handling [PR55534]
  2021-09-17 18:45 ` Tobias Burnus
@ 2021-09-20 20:33   ` Tobias Burnus
  2021-09-20 21:33     ` Harald Anlauf
  2021-09-21 19:22     ` [Patch] Fortran: Improve -Wmissing-include-dirs warnings [PR55534] Tobias Burnus
  0 siblings, 2 replies; 9+ messages in thread
From: Tobias Burnus @ 2021-09-20 20:33 UTC (permalink / raw)
  To: gcc-patches, fortran

[-- Attachment #1: Type: text/plain, Size: 4510 bytes --]

And v3 – I realized that testcases would be useful. Thus, now with added
testcases. :-)

Tobias

On 17.09.21 20:45, Tobias Burnus wrote:
> I seemingly messed up a bit in previous patch – corrected version
> attached.
>
> OK?
>
> Tobias
>
> PS: Due to now enabling the missing-include-dir warning also for
> cpp,the following
> warning show up during build. This seems to be specific to libgfortran
> building,
> libgomp works and real-world code also does not seem to be affected:
> <built-in>: Error: <instdir>/x86_64-pc-linux-gnu/include: No such file
> or directory [-Werror=missing-include-dirs]
> <built-in>: Error: <instdir>/x86_64-pc-linux-gnu/sys-include: No such
> file or directory [-Werror=missing-include-dirs]
> <built-in>: Error: finclude: No such file or directory
> [-Werror=missing-include-dirs]
>
> The latter is due to the driver adding '-fintrinsic-modules-path
> finclude'
> when calling f951. I think the rest is a side effect of running with -B
> and other build trickery.
>
> The warnings do not trigger when compiling the Fortran file in libgomp
> nor for
> a quick real-world case (which uses gfortran in a normal way not with
> -B etc.).
> Thus, I think it should be fine.
> Alternatively, we could think of reducing the noisiness. Thoughts?
>
> PPS: Besides this, the following still applies:
>
> On 17.09.21 15:02, Tobias Burnus wrote:
>> Short version:
>> * -Wno-missing-include-dirs  had no effect as the warning was always on
>> * For CPP-only options like -idirafter, no warning was shown.
>>
>> This patch tries to address both, i.e. cpp's include-dir diagnostics are
>> shown as well – and silencing the diagnostic works as well.
>>
>> OK for mainline?
>>
>> Tobias
>>
>> PS:  BACKGROUND and LONG DESCRIPTION
>>
>> C/C++ by default have disabled the -Wmissing-include-dirs warning.
>> Fortran by default has that warning enabled.
>>
>> The value is actually stored at two places (cf. c-family/c.opt):
>>   Wmissing-include-dirs
>>   ... CPP(warn_missing_include_dirs)
>> Var(cpp_warn_missing_include_dirs) Init(0)
>>
>> For CPP, that value always needs to initialized – and it is used
>> in gcc/incpath.c as
>>               cpp_options *opts = cpp_get_options (pfile);
>>               if (opts->warn_missing_include_dirs &&
>> cur->user_supplied_p)
>>                 cpp_warning (pfile, CPP_W_MISSING_INCLUDE_DIRS, "%s:
>> %s",
>>
>> Additionally, there is cpp_warn_missing_include_dirs which is used by
>> Fortran – and which consists of
>>   global_options.x_cpp_warn_missing_include_dirs
>>   global_options_set._cpp_warn_missing_include_dirs
>>
>> The flag processing happens as described in
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55534#c11
>> in short:
>>   - First gfc_init_options is called
>>   - Then for reach command-line option gfc_handle_option
>>   - Finally gfc_post_options
>>
>> Currently:
>> - gfc_init_options: Sets cpp_warn_missing_include_dirs
>>   (unconditionally as unset)
>> - gfc_handle_option: Always warns about the missing include dir
>> - before gfc_post_options: set_option is called, which sets
>>   cpp_warn_missing_include_dirs – but that's too late.
>>
>> Additionally, as mentioned above – pfile's warn_missing_include_dirs
>> is never properly set.
>>
>>  * * *
>>
>> This patch fixes those issues:
>> * Now -Wno-missing-include-dirs does silence the warnings
>> * CPP now also properly does warn.
>>
>> Example (initial version):
>> $ gfortran-trunk ../empty.f90 -c -cpp -idirafter /fdaf/ -I bar/
>> -Wmissing-include-dirs
>> f951: Warning: Nonexistent include directory ‘bar//’
>> [-Wmissing-include-dirs]
>> <built-in>: Warning: /fdaf/: No such file or directory
>> <built-in>: Warning: bar/: No such file or directory
>>
>> In order to avoid the double output for -I, I disabled the Fortran
>> output if
>> CPP is enabled. Additionally, I had to use the
>> cpp_reason_option_codes to
>> print the flag in brackets.
>> Fixed/final output is:
>>
>> <built-in>: Warning: /fdaf/: No such file or directory
>> [-Wmissing-include-dirs]
>> <built-in>: Warning: bar/: No such file or directory
>> [-Wmissing-include-dirs]
>>
-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955

[-- Attachment #2: missing-incl-v4.diff --]
[-- Type: text/x-patch, Size: 23027 bytes --]

Fortran: Fix -Wno-missing-include-dirs handling [PR55534]

gcc/fortran/ChangeLog:

	PR fortran/55534
	* cpp.c: Define GCC_C_COMMON_C for #include "options.h" to make
	cpp_reason_option_codes available.
	(gfc_cpp_register_include_paths): Make static, set pfile's
	warn_missing_include_dirs and move before caller.
	(gfc_cpp_init_cb): New, cb code moved from ...
	(gfc_cpp_init_0): ... here.
	(gfc_cpp_post_options): Call gfc_cpp_init_cb.
	(cb_cpp_diagnostic_cpp_option): New. As implemented in c-family
	to match CppReason flags to -W... names.
	(cb_cpp_diagnostic): Use it to replace single special case.
	* cpp.h (gfc_cpp_register_include_paths): Remove as now static.
	* gfortran.h (gfc_check_include_dirs): New prototype.
	(gfc_add_include_path): Add new bool arg.
	* options.c (gfc_init_options): Don't set -Wmissing-include-dirs.
	(gfc_post_options): Set it here after commandline processing. Call
	gfc_add_include_path with defer_warn=false.
	(gfc_handle_option): Call it with defer_warn=true.
	* scanner.c (gfc_do_check_include_dir, gfc_do_check_include_dirs,
	gfc_check_include_dirs): New. Diagnostic moved from ...
	(add_path_to_list): ... here, which came before cmdline processing.
	Take additional bool defer_warn argument.
	(gfc_add_include_path): Take additional defer_warn arg.
	* scanner.h (struct gfc_directorylist): Reorder for alignment issues,
	add new 'bool warn'.

libgfortran/ChangeLog:
	PR fortran/55534
	* configure.ac (AM_FCFLAGS): Add -Wno-missing-include-dirs.
	* configure: Regenerate.

libgfortran/ChangeLog:
	PR fortran/55534
	* testsuite/libgomp.fortran/fortran.exp: Add -Wno-missing-include-dirs
	to ALWAYS_CFLAGS.
	* testsuite/libgomp.oacc-fortran/fortran.exp: Likewise.

gcc/testsuite/ChangeLog:
        * gfortran.dg/include_6.f90: Change dg-error to
	dg-warning and update pattern.
        * gfortran.dg/include_14.f90: New test.
        * gfortran.dg/include_15.f90: New test.
        * gfortran.dg/include_16.f90: New test.
        * gfortran.dg/include_17.f90: New test.
        * gfortran.dg/include_18.f90: New test.
        * gfortran.dg/include_19.f90: New test.
        * gfortran.dg/include_20.f90: New test.
        * gfortran.dg/include_21.f90: New test.

 gcc/fortran/cpp.c                                  | 106 +++++++++++++--------
 gcc/fortran/cpp.h                                  |   2 -
 gcc/fortran/gfortran.h                             |   3 +-
 gcc/fortran/options.c                              |  24 ++---
 gcc/fortran/scanner.c                              |  82 ++++++++++++----
 gcc/fortran/scanner.h                              |   2 +-
 gcc/testsuite/gfortran.dg/include_14.f90           |   5 +
 gcc/testsuite/gfortran.dg/include_15.f90           |   5 +
 gcc/testsuite/gfortran.dg/include_16.f90           |   2 +
 gcc/testsuite/gfortran.dg/include_17.f90           |   4 +
 gcc/testsuite/gfortran.dg/include_18.f90           |   3 +
 gcc/testsuite/gfortran.dg/include_19.f90           |   4 +
 gcc/testsuite/gfortran.dg/include_20.f90           |   5 +
 gcc/testsuite/gfortran.dg/include_21.f90           |  26 +++++
 gcc/testsuite/gfortran.dg/include_6.f90            |   2 +-
 libgfortran/configure                              |   2 +-
 libgfortran/configure.ac                           |   2 +-
 libgomp/testsuite/libgomp.fortran/fortran.exp      |   3 +
 libgomp/testsuite/libgomp.oacc-fortran/fortran.exp |   3 +
 19 files changed, 206 insertions(+), 79 deletions(-)

diff --git a/gcc/fortran/cpp.c b/gcc/fortran/cpp.c
index 83c4517acdb..3ff895455e9 100644
--- a/gcc/fortran/cpp.c
+++ b/gcc/fortran/cpp.c
@@ -19,11 +19,15 @@ along with GCC; see the file COPYING3.  If not see
 #include "config.h"
 #include "system.h"
 #include "coretypes.h"
+
+#define GCC_C_COMMON_C
+#include "options.h"  /* For cpp_reason_option_codes. */
+#undef GCC_C_COMMON_C
+
 #include "target.h"
 #include "gfortran.h"
 #include "diagnostic.h"
 
-
 #include "toplev.h"
 
 #include "../../libcpp/internal.h"
@@ -240,6 +244,18 @@ gfc_cpp_temporary_file (void)
   return gfc_cpp_option.temporary_filename;
 }
 
+static void
+gfc_cpp_register_include_paths (void)
+{
+  int cxx_stdinc = 0;
+  cpp_get_options (cpp_in)->warn_missing_include_dirs
+    = global_options.x_cpp_warn_missing_include_dirs;
+  register_include_chains (cpp_in, gfc_cpp_option.sysroot,
+			   gfc_cpp_option.prefix, gfc_cpp_option.multilib,
+			   gfc_cpp_option.standard_include_paths, cxx_stdinc,
+			   gfc_cpp_option.verbose);
+}
+
 void
 gfc_cpp_init_options (unsigned int decoded_options_count,
 		      struct cl_decoded_option *decoded_options ATTRIBUTE_UNUSED)
@@ -435,6 +451,37 @@ gfc_cpp_handle_option (size_t scode, const char *arg, int value ATTRIBUTE_UNUSED
   return result;
 }
 
+/* This function needs to be called before gfc_cpp_register_include_paths
+   as the latter may diagnose missing include directories.  */
+static void
+gfc_cpp_init_cb (void)
+{
+  struct cpp_callbacks *cb;
+
+  cb = cpp_get_callbacks (cpp_in);
+  cb->file_change = cb_file_change;
+  cb->line_change = cb_line_change;
+  cb->ident = cb_ident;
+  cb->def_pragma = cb_def_pragma;
+  cb->diagnostic = cb_cpp_diagnostic;
+
+  if (gfc_cpp_option.dump_includes)
+    cb->include = cb_include;
+
+  if ((gfc_cpp_option.dump_macros == 'D')
+      || (gfc_cpp_option.dump_macros == 'N'))
+    {
+      cb->define = cb_define;
+      cb->undef  = cb_undef;
+    }
+
+  if (gfc_cpp_option.dump_macros == 'U')
+    {
+      cb->before_define = dump_queued_macros;
+      cb->used_define = cb_used_define;
+      cb->used_undef = cb_used_undef;
+    }
+}
 
 void
 gfc_cpp_post_options (void)
@@ -498,6 +545,7 @@ gfc_cpp_post_options (void)
      way libcpp will do it, namely, with no charset conversion but with
      skipping of a UTF-8 BOM if present.  */
   diagnostic_initialize_input_context (global_dc, nullptr, true);
+  gfc_cpp_init_cb ();
 
   gfc_cpp_register_include_paths ();
 }
@@ -506,32 +554,6 @@ gfc_cpp_post_options (void)
 void
 gfc_cpp_init_0 (void)
 {
-  struct cpp_callbacks *cb;
-
-  cb = cpp_get_callbacks (cpp_in);
-  cb->file_change = cb_file_change;
-  cb->line_change = cb_line_change;
-  cb->ident = cb_ident;
-  cb->def_pragma = cb_def_pragma;
-  cb->diagnostic = cb_cpp_diagnostic;
-
-  if (gfc_cpp_option.dump_includes)
-    cb->include = cb_include;
-
-  if ((gfc_cpp_option.dump_macros == 'D')
-      || (gfc_cpp_option.dump_macros == 'N'))
-    {
-      cb->define = cb_define;
-      cb->undef  = cb_undef;
-    }
-
-  if (gfc_cpp_option.dump_macros == 'U')
-    {
-      cb->before_define = dump_queued_macros;
-      cb->used_define = cb_used_define;
-      cb->used_undef = cb_used_undef;
-    }
-
   /* Initialize the print structure.  Setting print.src_line to -1 here is
      a trick to guarantee that the first token of the file will cause
      a linemarker to be output by maybe_print_line.  */
@@ -723,17 +745,6 @@ gfc_cpp_add_include_path_after (char *path, bool user_supplied)
   add_path (path, INC_AFTER, cxx_aware, user_supplied);
 }
 
-void
-gfc_cpp_register_include_paths (void)
-{
-  int cxx_stdinc = 0;
-  register_include_chains (cpp_in, gfc_cpp_option.sysroot,
-			   gfc_cpp_option.prefix, gfc_cpp_option.multilib,
-			   gfc_cpp_option.standard_include_paths, cxx_stdinc,
-			   gfc_cpp_option.verbose);
-}
-
-
 
 static void scan_translation_unit_trad (cpp_reader *);
 static void account_for_newlines (const unsigned char *, size_t);
@@ -1043,6 +1054,21 @@ cb_used_define (cpp_reader *pfile, location_t line ATTRIBUTE_UNUSED,
   cpp_define_queue = q;
 }
 
+/* Return the gcc option code associated with the reason for a cpp
+   message, or 0 if none.  */
+
+static int
+cb_cpp_diagnostic_cpp_option (enum cpp_warning_reason reason)
+{
+  const struct cpp_reason_option_codes_t *entry;
+
+  for (entry = cpp_reason_option_codes; entry->reason != CPP_W_NONE; entry++)
+    if (entry->reason == reason)
+      return entry->option_code;
+  return 0;
+}
+
+
 /* Callback from cpp_error for PFILE to print diagnostics from the
    preprocessor.  The diagnostic is of type LEVEL, with REASON set
    to the reason code if LEVEL is represents a warning, at location
@@ -1089,8 +1115,8 @@ cb_cpp_diagnostic (cpp_reader *pfile ATTRIBUTE_UNUSED,
     }
   diagnostic_set_info_translated (&diagnostic, msg, ap,
 				  richloc, dlevel);
-  if (reason == CPP_W_WARNING_DIRECTIVE)
-    diagnostic_override_option_index (&diagnostic, OPT_Wcpp);
+  diagnostic_override_option_index (&diagnostic,
+				    cb_cpp_diagnostic_cpp_option (reason));
   ret = diagnostic_report_diagnostic (global_dc, &diagnostic);
   if (level == CPP_DL_WARNING_SYSHDR)
     global_dc->dc_warn_system_headers = save_warn_system_headers;
diff --git a/gcc/fortran/cpp.h b/gcc/fortran/cpp.h
index f642c7745f7..5cb7e5a9c34 100644
--- a/gcc/fortran/cpp.h
+++ b/gcc/fortran/cpp.h
@@ -50,6 +50,4 @@ void gfc_cpp_done (void);
 void gfc_cpp_add_include_path (char *path, bool user_supplied);
 void gfc_cpp_add_include_path_after (char *path, bool user_supplied);
 
-void gfc_cpp_register_include_paths (void);
-
 #endif /* GFC_CPP_H */
diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h
index 8b91225d659..3c7a8434d07 100644
--- a/gcc/fortran/gfortran.h
+++ b/gcc/fortran/gfortran.h
@@ -3029,9 +3029,10 @@ match gfc_get_pdt_instance (gfc_actual_arglist *, gfc_symbol **,
 void gfc_scanner_done_1 (void);
 void gfc_scanner_init_1 (void);
 
-void gfc_add_include_path (const char *, bool, bool, bool);
+void gfc_add_include_path (const char *, bool, bool, bool, bool);
 void gfc_add_intrinsic_modules_path (const char *);
 void gfc_release_include_path (void);
+void gfc_check_include_dirs (void);
 FILE *gfc_open_included_file (const char *, bool, bool);
 
 int gfc_at_end (void);
diff --git a/gcc/fortran/options.c b/gcc/fortran/options.c
index 847e20e8829..d789397515a 100644
--- a/gcc/fortran/options.c
+++ b/gcc/fortran/options.c
@@ -159,14 +159,7 @@ gfc_init_options (unsigned int decoded_options_count,
 			   | GFC_FPE_UNDERFLOW;
   gfc_option.rtcheck = 0;
 
-  /* ??? Wmissing-include-dirs is disabled by default in C/C++ but
-     enabled by default in Fortran.  Ideally, we should express this
-     in .opt, but that is not supported yet.  */
-  SET_OPTION_IF_UNSET (&global_options, &global_options_set,
-		       cpp_warn_missing_include_dirs, 1);
-
   set_dec_flags (0);
-
   set_default_std_flags ();
 
   /* Initialize cpp-related options.  */
@@ -260,6 +253,13 @@ gfc_post_options (const char **pfilename)
   char *source_path;
   int i;
 
+  /* This needs to be after the commandline has been processed.
+     In Fortran, the options is by default enabled, in C/C++
+     by default disabled.  */
+  SET_OPTION_IF_UNSET (&global_options, &global_options_set,
+		       cpp_warn_missing_include_dirs, 1);
+  gfc_check_include_dirs ();
+
   /* Finalize DEC flags.  */
   post_dec_flags (flag_dec);
 
@@ -339,10 +339,10 @@ gfc_post_options (const char **pfilename)
       source_path = (char *) alloca (i + 1);
       memcpy (source_path, canon_source_file, i);
       source_path[i] = 0;
-      gfc_add_include_path (source_path, true, true, true);
+      gfc_add_include_path (source_path, true, true, true, false);
     }
   else
-    gfc_add_include_path (".", true, true, true);
+    gfc_add_include_path (".", true, true, true, false);
 
   if (canon_source_file != gfc_source_file)
     free (CONST_CAST (char *, canon_source_file));
@@ -511,7 +511,7 @@ gfc_handle_module_path_options (const char *arg)
   gfc_option.module_dir = XCNEWVEC (char, strlen (arg) + 2);
   strcpy (gfc_option.module_dir, arg);
 
-  gfc_add_include_path (gfc_option.module_dir, true, false, true);
+  gfc_add_include_path (gfc_option.module_dir, true, false, true, true);
 
   strcat (gfc_option.module_dir, "/");
 }
@@ -690,7 +690,7 @@ gfc_handle_option (size_t scode, const char *arg, HOST_WIDE_INT value,
 	 with intrinsic modules.  Do no warn because during testing
 	 without an installed compiler, we would get lots of bogus
 	 warnings for a missing include directory.  */
-      gfc_add_include_path (arg, false, false, false);
+      gfc_add_include_path (arg, false, false, false, true);
 
       gfc_add_intrinsic_modules_path (arg);
       break;
@@ -737,7 +737,7 @@ gfc_handle_option (size_t scode, const char *arg, HOST_WIDE_INT value,
       break;
 
     case OPT_I:
-      gfc_add_include_path (arg, true, false, true);
+      gfc_add_include_path (arg, true, false, true, true);
       break;
 
     case OPT_J:
diff --git a/gcc/fortran/scanner.c b/gcc/fortran/scanner.c
index 39db0994b62..6fe74bd5b8d 100644
--- a/gcc/fortran/scanner.c
+++ b/gcc/fortran/scanner.c
@@ -298,17 +298,69 @@ gfc_scanner_done_1 (void)
     }
 }
 
+static bool
+gfc_do_check_include_dir (const char *path, bool warn)
+{
+  struct stat st;
+  if (stat (path, &st))
+    {
+      if (errno != ENOENT)
+	gfc_warning_now (0, "Include directory %qs: %s",
+			 path, xstrerror(errno));
+      else if (warn && !gfc_cpp_enabled ())
+	  gfc_warning_now (OPT_Wmissing_include_dirs,
+			     "Nonexistent include directory %qs", path);
+      return false;
+    }
+  else if (!S_ISDIR (st.st_mode))
+    {
+      gfc_fatal_error ("%qs is not a directory", path);
+      return false;
+    }
+  return true;
+}
+
+/* In order that -W(no-)missing-include-dirs works, the diagnostic can only be
+   run after processing the commandline.  */
+static void
+gfc_do_check_include_dirs (gfc_directorylist **list)
+{
+  gfc_directorylist *prev, *q, *n;
+  prev = NULL;
+  n = *list;
+  while (n)
+    {
+      q = n; n = n->next;
+      if (gfc_do_check_include_dir (q->path, q->warn))
+	{
+	  prev = q;
+	  continue;
+	}
+      if (prev == NULL)
+	*list = n;
+      else
+	prev->next = n;
+      free (q->path);
+      free (q);
+    }
+}
+
+void
+gfc_check_include_dirs ()
+{
+  gfc_do_check_include_dirs (&include_dirs);
+  gfc_do_check_include_dirs (&intrinsic_modules_dirs);
+}
 
 /* Adds path to the list pointed to by list.  */
 
 static void
 add_path_to_list (gfc_directorylist **list, const char *path,
-		  bool use_for_modules, bool head, bool warn)
+		  bool use_for_modules, bool head, bool warn, bool defer_warn)
 {
   gfc_directorylist *dir;
   const char *p;
   char *q;
-  struct stat st;
   size_t len;
   int i;
   
@@ -326,21 +378,8 @@ add_path_to_list (gfc_directorylist **list, const char *path,
   while (i >=0 && IS_DIR_SEPARATOR (q[i]))
     q[i--] = '\0';
 
-  if (stat (q, &st))
-    {
-      if (errno != ENOENT)
-	gfc_warning_now (0, "Include directory %qs: %s", path,
-			 xstrerror(errno));
-      else if (warn)
-	gfc_warning_now (OPT_Wmissing_include_dirs,
-			 "Nonexistent include directory %qs", path);
-      return;
-    }
-  else if (!S_ISDIR (st.st_mode))
-    {
-      gfc_fatal_error ("%qs is not a directory", path);
-      return;
-    }
+  if (!defer_warn && !gfc_do_check_include_dir (q, warn))
+    return;
 
   if (head || *list == NULL)
     {
@@ -362,17 +401,20 @@ add_path_to_list (gfc_directorylist **list, const char *path,
   if (head)
     *list = dir;
   dir->use_for_modules = use_for_modules;
+  dir->warn = warn;
   dir->path = XCNEWVEC (char, strlen (p) + 2);
   strcpy (dir->path, p);
   strcat (dir->path, "/");	/* make '/' last character */
 }
 
+/* defer_warn is set to true while parsing the commandline.  */
 
 void
 gfc_add_include_path (const char *path, bool use_for_modules, bool file_dir,
-		      bool warn)
+		      bool warn, bool defer_warn)
 {
-  add_path_to_list (&include_dirs, path, use_for_modules, file_dir, warn);
+  add_path_to_list (&include_dirs, path, use_for_modules, file_dir, warn,
+		    defer_warn);
 
   /* For '#include "..."' these directories are automatically searched.  */
   if (!file_dir)
@@ -383,7 +425,7 @@ gfc_add_include_path (const char *path, bool use_for_modules, bool file_dir,
 void
 gfc_add_intrinsic_modules_path (const char *path)
 {
-  add_path_to_list (&intrinsic_modules_dirs, path, true, false, false);
+  add_path_to_list (&intrinsic_modules_dirs, path, true, false, false, false);
 }
 
 
diff --git a/gcc/fortran/scanner.h b/gcc/fortran/scanner.h
index 0bad63f4ffb..8782fe6ccc5 100644
--- a/gcc/fortran/scanner.h
+++ b/gcc/fortran/scanner.h
@@ -23,8 +23,8 @@ along with GCC; see the file COPYING3.  If not see
 typedef struct gfc_directorylist
 {
   char *path;
-  bool use_for_modules;
   struct gfc_directorylist *next;
+  bool use_for_modules, warn;
 }
 gfc_directorylist;
 
diff --git a/gcc/testsuite/gfortran.dg/include_14.f90 b/gcc/testsuite/gfortran.dg/include_14.f90
new file mode 100644
index 00000000000..b306b2c813b
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/include_14.f90
@@ -0,0 +1,5 @@
+! { dg-additional-options "-cpp -idirafter /fdaf/ -I bar" }
+end
+
+! { dg-warning "/fdaf/: No such file or directory" "" { target *-*-* } 0 }
+! { dg-warning "bar: No such file or directory" "" { target *-*-* } 0 }
diff --git a/gcc/testsuite/gfortran.dg/include_15.f90 b/gcc/testsuite/gfortran.dg/include_15.f90
new file mode 100644
index 00000000000..4944282f931
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/include_15.f90
@@ -0,0 +1,5 @@
+! { dg-additional-options "-cpp -idirafter /fdaf/ -I bar -Wmissing-include-dirs" }
+end
+
+! { dg-warning "/fdaf/: No such file or directory" "" { target *-*-* } 0 }
+! { dg-warning "bar: No such file or directory" "" { target *-*-* } 0 }
diff --git a/gcc/testsuite/gfortran.dg/include_16.f90 b/gcc/testsuite/gfortran.dg/include_16.f90
new file mode 100644
index 00000000000..45794f28e73
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/include_16.f90
@@ -0,0 +1,2 @@
+! { dg-additional-options "-cpp -idirafter /fdaf/ -I bar -Wno-missing-include-dirs" }
+end
diff --git a/gcc/testsuite/gfortran.dg/include_17.f90 b/gcc/testsuite/gfortran.dg/include_17.f90
new file mode 100644
index 00000000000..0ed5c86d323
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/include_17.f90
@@ -0,0 +1,4 @@
+! { dg-do compile }
+! { dg-options "-I foo-bar -Wno-missing-include-dirs" }
+end 
+
diff --git a/gcc/testsuite/gfortran.dg/include_18.f90 b/gcc/testsuite/gfortran.dg/include_18.f90
new file mode 100644
index 00000000000..ca69df382fe
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/include_18.f90
@@ -0,0 +1,3 @@
+! { dg-do compile }
+! { dg-options "-I nothere -Wno-missing-include-dirs" }
+end 
diff --git a/gcc/testsuite/gfortran.dg/include_19.f90 b/gcc/testsuite/gfortran.dg/include_19.f90
new file mode 100644
index 00000000000..2a068174b7e
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/include_19.f90
@@ -0,0 +1,4 @@
+! { dg-do compile }
+! { dg-options "-J foobar/foo -Wno-missing-include-dirs" }
+program main
+end program main
diff --git a/gcc/testsuite/gfortran.dg/include_20.f90 b/gcc/testsuite/gfortran.dg/include_20.f90
new file mode 100644
index 00000000000..4f8fdc6b786
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/include_20.f90
@@ -0,0 +1,5 @@
+! { dg-do compile }
+! { dg-options "-J foobar/foo" }
+program main
+end program main
+! { dg-warning "Nonexistent include directory" "" { target *-*-* } 0 }
diff --git a/gcc/testsuite/gfortran.dg/include_21.f90 b/gcc/testsuite/gfortran.dg/include_21.f90
new file mode 100644
index 00000000000..40bc5986f7b
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/include_21.f90
@@ -0,0 +1,26 @@
+# 1 "../../../trunk/libgfortran/generated/_abs_c4.F90"
+# 1 "C:\\msys\\1.0.10\\home\\FX\\ibin\\i586-pc-mingw32\\libgfortran//"
+# 1 "<built-in>"
+# 1 "<command-line>"
+# 1 "../../../trunk/libgfortran/generated/_abs_c4.F90"
+!   Comment here
+
+# 1 "./config.h" 1
+
+# 37 "../../../trunk/libgfortran/generated/_abs_c4.F90" 2
+
+# 1 "./kinds.inc" 1
+# 38 "../../../trunk/libgfortran/generated/_abs_c4.F90" 2
+
+# 1 "./c99_protos.inc" 1
+# 39 "../../../trunk/libgfortran/generated/_abs_c4.F90" 2
+
+elemental function abs_c4 (parm)
+   complex (kind=4), intent (in) :: parm
+   real (kind=4) :: abs_c4
+
+   abs_c4 = abs (parm)
+end function
+
+! { dg-do compile }
+! { dg-options "-fpreprocessed -g3 -Wno-missing-include-dirs" }
diff --git a/gcc/testsuite/gfortran.dg/include_6.f90 b/gcc/testsuite/gfortran.dg/include_6.f90
index f5bb085d321..3e3be1b7bd5 100644
--- a/gcc/testsuite/gfortran.dg/include_6.f90
+++ b/gcc/testsuite/gfortran.dg/include_6.f90
@@ -1,6 +1,6 @@
 ! { dg-do compile }
 ! { dg-options "-I gfortran.log" }
-! { dg-error "is not a directory" "" { target *-*-* } 0 }
+! { dg-warning "Include directory 'gfortran.log/': Not a directory" "" { target *-*-* } 0 }
 ! { dg-prune-output "compilation terminated." }
 end 
 
diff --git a/libgfortran/configure b/libgfortran/configure
index 4810b9b032e..350159aa4fe 100755
--- a/libgfortran/configure
+++ b/libgfortran/configure
@@ -5985,7 +5985,7 @@ fi
 
 # Add -Wall -fno-repack-arrays -fno-underscoring if we are using GCC.
 if test "x$GCC" = "xyes"; then
-  AM_FCFLAGS="-I . -Wall -Werror -fimplicit-none -fno-repack-arrays -fno-underscoring"
+  AM_FCFLAGS="-I . -Wall -Werror -fimplicit-none -fno-repack-arrays -fno-underscoring -Wno-missing-include-dirs"
   ## We like to use C11 and C99 routines when available.  This makes
   ## sure that
   ## __STDC_VERSION__ is set such that libc includes make them available.
diff --git a/libgfortran/configure.ac b/libgfortran/configure.ac
index a77509801e6..a3550b4e5d0 100644
--- a/libgfortran/configure.ac
+++ b/libgfortran/configure.ac
@@ -146,7 +146,7 @@ AM_PROG_CC_C_O
 
 # Add -Wall -fno-repack-arrays -fno-underscoring if we are using GCC.
 if test "x$GCC" = "xyes"; then
-  AM_FCFLAGS="-I . -Wall -Werror -fimplicit-none -fno-repack-arrays -fno-underscoring"
+  AM_FCFLAGS="-I . -Wall -Werror -fimplicit-none -fno-repack-arrays -fno-underscoring -Wno-missing-include-dirs"
   ## We like to use C11 and C99 routines when available.  This makes
   ## sure that
   ## __STDC_VERSION__ is set such that libc includes make them available.
diff --git a/libgomp/testsuite/libgomp.fortran/fortran.exp b/libgomp/testsuite/libgomp.fortran/fortran.exp
index eb701311b6a..912dd2a743e 100644
--- a/libgomp/testsuite/libgomp.fortran/fortran.exp
+++ b/libgomp/testsuite/libgomp.fortran/fortran.exp
@@ -20,6 +20,9 @@ dg-init
 
 # Turn on OpenMP.
 lappend ALWAYS_CFLAGS "additional_flags=-fopenmp"
+# Silence warnings due to explicitly passed but nonexisting
+# -isystem <instdir>/target>/{sys-,}include (gfortran warns by default)
+lappend ALWAYS_CFLAGS "additional_flags=-Wno-missing-include-dirs"
 
 if { $blddir != "" } {
     set lang_source_re {^.*\.[fF](|90|95|03|08)$}
diff --git a/libgomp/testsuite/libgomp.oacc-fortran/fortran.exp b/libgomp/testsuite/libgomp.oacc-fortran/fortran.exp
index 7365b320668..22e91eca256 100644
--- a/libgomp/testsuite/libgomp.oacc-fortran/fortran.exp
+++ b/libgomp/testsuite/libgomp.oacc-fortran/fortran.exp
@@ -22,6 +22,9 @@ dg-init
 
 # Turn on OpenACC.
 lappend ALWAYS_CFLAGS "additional_flags=-fopenacc"
+# Silence warnings due to explicitly passed but nonexisting
+# -isystem <instdir>/target>/{sys-,}include (gfortran warns by default)
+lappend ALWAYS_CFLAGS "additional_flags=-Wno-missing-include-dirs"
 
 if { $blddir != "" } {
     set lang_source_re {^.*\.[fF](|90|95|03|08)$}

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Patch] Fortran: Fix -Wno-missing-include-dirs handling [PR55534]
  2021-09-20 20:33   ` Tobias Burnus
@ 2021-09-20 21:33     ` Harald Anlauf
  2021-09-21 19:22     ` [Patch] Fortran: Improve -Wmissing-include-dirs warnings [PR55534] Tobias Burnus
  1 sibling, 0 replies; 9+ messages in thread
From: Harald Anlauf @ 2021-09-20 21:33 UTC (permalink / raw)
  To: gcc-patches; +Cc: fortran

Hi Tobias,

Am 20.09.21 um 22:33 schrieb Tobias Burnus:
> And v3 – I realized that testcases would be useful. Thus, now with added
> testcases. :-)

I was about to recommend a testcase I prepared when your revised patch
arrived.  Will not bother you with it, as you seem to provide really
good coverage.

LGTM.

Thanks for the patch!

Harald

> Tobias
> 
> On 17.09.21 20:45, Tobias Burnus wrote:
>> I seemingly messed up a bit in previous patch – corrected version
>> attached.
>>
>> OK?
>>
>> Tobias
>>
>> PS: Due to now enabling the missing-include-dir warning also for
>> cpp,the following
>> warning show up during build. This seems to be specific to libgfortran
>> building,
>> libgomp works and real-world code also does not seem to be affected:
>> <built-in>: Error: <instdir>/x86_64-pc-linux-gnu/include: No such file
>> or directory [-Werror=missing-include-dirs]
>> <built-in>: Error: <instdir>/x86_64-pc-linux-gnu/sys-include: No such
>> file or directory [-Werror=missing-include-dirs]
>> <built-in>: Error: finclude: No such file or directory
>> [-Werror=missing-include-dirs]
>>
>> The latter is due to the driver adding '-fintrinsic-modules-path
>> finclude'
>> when calling f951. I think the rest is a side effect of running with -B
>> and other build trickery.
>>
>> The warnings do not trigger when compiling the Fortran file in libgomp
>> nor for
>> a quick real-world case (which uses gfortran in a normal way not with
>> -B etc.).
>> Thus, I think it should be fine.
>> Alternatively, we could think of reducing the noisiness. Thoughts?
>>
>> PPS: Besides this, the following still applies:
>>
>> On 17.09.21 15:02, Tobias Burnus wrote:
>>> Short version:
>>> * -Wno-missing-include-dirs  had no effect as the warning was always on
>>> * For CPP-only options like -idirafter, no warning was shown.
>>>
>>> This patch tries to address both, i.e. cpp's include-dir diagnostics are
>>> shown as well – and silencing the diagnostic works as well.
>>>
>>> OK for mainline?
>>>
>>> Tobias
>>>
>>> PS:  BACKGROUND and LONG DESCRIPTION
>>>
>>> C/C++ by default have disabled the -Wmissing-include-dirs warning.
>>> Fortran by default has that warning enabled.
>>>
>>> The value is actually stored at two places (cf. c-family/c.opt):
>>>   Wmissing-include-dirs
>>>   ... CPP(warn_missing_include_dirs)
>>> Var(cpp_warn_missing_include_dirs) Init(0)
>>>
>>> For CPP, that value always needs to initialized – and it is used
>>> in gcc/incpath.c as
>>>               cpp_options *opts = cpp_get_options (pfile);
>>>               if (opts->warn_missing_include_dirs &&
>>> cur->user_supplied_p)
>>>                 cpp_warning (pfile, CPP_W_MISSING_INCLUDE_DIRS, "%s:
>>> %s",
>>>
>>> Additionally, there is cpp_warn_missing_include_dirs which is used by
>>> Fortran – and which consists of
>>>   global_options.x_cpp_warn_missing_include_dirs
>>>   global_options_set._cpp_warn_missing_include_dirs
>>>
>>> The flag processing happens as described in
>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55534#c11
>>> in short:
>>>   - First gfc_init_options is called
>>>   - Then for reach command-line option gfc_handle_option
>>>   - Finally gfc_post_options
>>>
>>> Currently:
>>> - gfc_init_options: Sets cpp_warn_missing_include_dirs
>>>   (unconditionally as unset)
>>> - gfc_handle_option: Always warns about the missing include dir
>>> - before gfc_post_options: set_option is called, which sets
>>>   cpp_warn_missing_include_dirs – but that's too late.
>>>
>>> Additionally, as mentioned above – pfile's warn_missing_include_dirs
>>> is never properly set.
>>>
>>>  * * *
>>>
>>> This patch fixes those issues:
>>> * Now -Wno-missing-include-dirs does silence the warnings
>>> * CPP now also properly does warn.
>>>
>>> Example (initial version):
>>> $ gfortran-trunk ../empty.f90 -c -cpp -idirafter /fdaf/ -I bar/
>>> -Wmissing-include-dirs
>>> f951: Warning: Nonexistent include directory ‘bar//’
>>> [-Wmissing-include-dirs]
>>> <built-in>: Warning: /fdaf/: No such file or directory
>>> <built-in>: Warning: bar/: No such file or directory
>>>
>>> In order to avoid the double output for -I, I disabled the Fortran
>>> output if
>>> CPP is enabled. Additionally, I had to use the
>>> cpp_reason_option_codes to
>>> print the flag in brackets.
>>> Fixed/final output is:
>>>
>>> <built-in>: Warning: /fdaf/: No such file or directory
>>> [-Wmissing-include-dirs]
>>> <built-in>: Warning: bar/: No such file or directory
>>> [-Wmissing-include-dirs]
>>>
> -----------------
> Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 
> 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: 
> Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; 
> Registergericht München, HRB 106955



^ permalink raw reply	[flat|nested] 9+ messages in thread

* [Patch] Fortran: Improve -Wmissing-include-dirs warnings [PR55534]
@ 2021-09-21 19:22     ` Tobias Burnus
  2021-09-22  9:13       ` Thomas Schwinge
  2021-09-22 18:29       ` Harald Anlauf
  0 siblings, 2 replies; 9+ messages in thread
From: Tobias Burnus @ 2021-09-21 19:22 UTC (permalink / raw)
  To: gcc-patches, fortran

[-- Attachment #1: Type: text/plain, Size: 2063 bytes --]

While the previous patch fixed -Wno-missing-include-dirs and sorted
out some inconsistencies with libcpp warnings, it had two issues:

* Some superfluous warnings were printed, e.g. for
     gfortran nonexisting/file.f90
   there was a warning about include path "nonexisting" not existing
   and twice the error that the "nonexisting/file.f90" could not be
   read.

* At least as invoked when build GCC or when running the GCC testsuite,
   the passed -B -isystem etc. arguments lead to proper but pointless
   diagnostic about 'finclude' or other directories not being found,
   causing excess-error FAILS and -Werror build fails.

While the latter could be fixed by adding -Wno-missing-include-dirs,
it still felt like the wrong approach.

While the testsuite does run for me, others reported that they do
see missing-include-dirs warnings. Instead of adding a bunch of
-Wno-missing-include-dirs to the test config, I now only warn for
-I and -J by default (similar to previous state) and only do a full
warnings when the user requested passes the -Wmissing-include-dirs
explicitly. The Fortran behavior is now also properly documented
in the manual.

In order to handle the silencing of the diagnostic and to avoid
double output via the Fortran code and libcpp (or rather: gcc/incpath.c),
I had to add some not that clean and obvious diagnostic flags.
I hope they still make sense and are somewhat readable.

OK? Comments?

Tobias

PS: There is also some inconsistency whether fprintf stderr and
gfc_error is used. All calls in load_file could be fatal errors
as all exist with an error - and similar issues get different
error messages for no good reason. I have not tried to solve
this issue – but can if deemed reasonable as follow-up patch.

-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955

[-- Attachment #2: missing-include-cleanup.diff --]
[-- Type: text/x-patch, Size: 18787 bytes --]

Fortran: Improve -Wmissing-include-dirs warnings [PR55534]

It turned out that enabling the -Wmissing-include-dirs for libcpp did output
too many warnings – at least as run with -B and similar options during the
GCC build and warning for internal include dirs like finclude, unlikely of
relevance to for a real-world user.
This patch now only warns for -I and -J by default but permits to get the
full warnings including libcpp ones with -Wmissing-include-dirs. It
additionally documents this in the manual.

With that change, the -Wno-missing-include-dirs could be removed
from libgfortran's configure and libgomp's testsuite always cflags.
This reverts those bits of the previous
commit r12-3722-g417ea5c02cef7f000e66d1af22b066c2c1cda047

Additionally, it turned out that all call to load_file called exit
explicitly - except for the main file via gfc_init -> gfc_new_file. The
latter also output a file not existing fatal error, such that two errors
where printed. Now exit is called in line with the other users of
load_file.

Finally, when compileing with "nonexisting/file.f90", first a warning that
"nonexisting" does not exist as include path was printed before the file
not found error was printed. Now the directory in which the physical file
is located is added silently, relying on the file-not-found diagnostic for
those.

	PR fortran/55534
gcc/ChangeLog:

	* doc/invoke.texi (-Wno-missing-include-dirs.): Document Fortran
	behavior.

gcc/fortran/ChangeLog:

	* cpp.c (gfc_cpp_register_include_paths, gfc_cpp_post_options):
	Add new bool verbose_missing_dir_warn argument.
	* cpp.h (gfc_cpp_post_options): Update prototype.
	* f95-lang.c (gfc_init): Remove duplicated file-not found diag.
	* gfortran.h (gfc_check_include_dirs): Takes bool
	verbose_missing_dir_warn arg.
	(gfc_new_file): Returns now void.
	* options.c (gfc_post_options): Update to warn for -I and -J,
	only, by default but for all when user requested.
	* scanner.c (gfc_do_check_include_dir):
	(gfc_do_check_include_dirs, gfc_check_include_dirs): Take bool
	verbose warn arg and update to avoid printing the same message
	twice or never.
	(load_file): Fix indent.
	(gfc_new_file): Return void and exit when load_file failed
	as all other load_file users do.

libgfortran/ChangeLog:

	* configure.ac (AM_FCFLAGS): Revert r12-3722 by removing
	-Wno-missing-include-dirs.
	* configure: Regenerate.
    
libgomp/ChangeLog:

	* testsuite/libgomp.fortran/fortran.exp (ALWAYS_CFLAGS): Revert
	r12-3722 by removing -Wno-missing-include-dirs.
	* testsuite/libgomp.oacc-fortran/fortran.exp (ALWAYS_CFLAGS): Likewise.

gcc/testsuite/ChangeLog:

	* gfortran.dg/include_14.f90: Add -J testcase and update dg-output.
	* gfortran.dg/include_15.f90: Likewise.
	* gfortran.dg/include_16.f90: Likewise.
	* gfortran.dg/include_17.f90: Likewise.
	* gfortran.dg/include_18.f90: Likewise.
	* gfortran.dg/include_19.f90: Likewise.

 gcc/doc/invoke.texi                                |  6 +++--
 gcc/fortran/cpp.c                                  |  9 ++++----
 gcc/fortran/cpp.h                                  |  2 +-
 gcc/fortran/f95-lang.c                             |  4 ++--
 gcc/fortran/gfortran.h                             |  4 ++--
 gcc/fortran/options.c                              | 19 +++++++++++----
 gcc/fortran/scanner.c                              | 27 ++++++++++++++--------
 gcc/testsuite/gfortran.dg/include_14.f90           |  7 +++---
 gcc/testsuite/gfortran.dg/include_15.f90           |  7 +++---
 gcc/testsuite/gfortran.dg/include_16.f90           |  2 +-
 gcc/testsuite/gfortran.dg/include_17.f90           |  4 +++-
 gcc/testsuite/gfortran.dg/include_18.f90           |  4 +++-
 gcc/testsuite/gfortran.dg/include_19.f90           |  2 +-
 libgfortran/configure                              |  2 +-
 libgfortran/configure.ac                           |  2 +-
 libgomp/testsuite/libgomp.fortran/fortran.exp      |  3 ---
 libgomp/testsuite/libgomp.oacc-fortran/fortran.exp |  3 ---
 17 files changed, 63 insertions(+), 44 deletions(-)

diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 4acb94181d2..ba98eab68a5 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -6459,10 +6459,12 @@ int b[2][2] = @{ @{ 0, 1 @}, @{ 2, 3 @} @};
 
 This warning is enabled by @option{-Wall}.
 
-@item -Wmissing-include-dirs @r{(C, C++, Objective-C and Objective-C++ only)}
+@item -Wmissing-include-dirs @r{(C, C++, Objective-C, Objective-C++ and Fortran only)}
 @opindex Wmissing-include-dirs
 @opindex Wno-missing-include-dirs
-Warn if a user-supplied include directory does not exist.
+Warn if a user-supplied include directory does not exist. This opions is disabled
+by default for C, C++, Objective-C and Objective-C++. For Fortran, it is partially
+enabled by default by warning for -I and -J, only.
 
 @item -Wno-missing-profile
 @opindex Wmissing-profile
diff --git a/gcc/fortran/cpp.c b/gcc/fortran/cpp.c
index 3ff895455e9..e86386c8b17 100644
--- a/gcc/fortran/cpp.c
+++ b/gcc/fortran/cpp.c
@@ -245,11 +245,12 @@ gfc_cpp_temporary_file (void)
 }
 
 static void
-gfc_cpp_register_include_paths (void)
+gfc_cpp_register_include_paths (bool verbose_missing_dir_warn)
 {
   int cxx_stdinc = 0;
   cpp_get_options (cpp_in)->warn_missing_include_dirs
-    = global_options.x_cpp_warn_missing_include_dirs;
+    = (global_options.x_cpp_warn_missing_include_dirs
+       && verbose_missing_dir_warn);
   register_include_chains (cpp_in, gfc_cpp_option.sysroot,
 			   gfc_cpp_option.prefix, gfc_cpp_option.multilib,
 			   gfc_cpp_option.standard_include_paths, cxx_stdinc,
@@ -484,7 +485,7 @@ gfc_cpp_init_cb (void)
 }
 
 void
-gfc_cpp_post_options (void)
+gfc_cpp_post_options (bool verbose_missing_dir_warn)
 {
   /* Any preprocessing-related option without '-cpp' is considered
      an error.  */
@@ -547,7 +548,7 @@ gfc_cpp_post_options (void)
   diagnostic_initialize_input_context (global_dc, nullptr, true);
   gfc_cpp_init_cb ();
 
-  gfc_cpp_register_include_paths ();
+  gfc_cpp_register_include_paths (verbose_missing_dir_warn);
 }
 
 
diff --git a/gcc/fortran/cpp.h b/gcc/fortran/cpp.h
index 5cb7e5a9c34..44644a2a333 100644
--- a/gcc/fortran/cpp.h
+++ b/gcc/fortran/cpp.h
@@ -41,7 +41,7 @@ void gfc_cpp_init_options (unsigned int decoded_options_count,
 
 int gfc_cpp_handle_option(size_t scode, const char *arg, int value);
 
-void gfc_cpp_post_options (void);
+void gfc_cpp_post_options (bool);
 
 bool gfc_cpp_preprocess (const char *source_file);
 
diff --git a/gcc/fortran/f95-lang.c b/gcc/fortran/f95-lang.c
index 026228da09f..58dcaf01d75 100644
--- a/gcc/fortran/f95-lang.c
+++ b/gcc/fortran/f95-lang.c
@@ -259,8 +259,8 @@ gfc_init (void)
 
   gfc_init_1 ();
 
-  if (!gfc_new_file ())
-    fatal_error (input_location, "cannot open input file: %s", gfc_source_file);
+  /* Calls exit in case of a fail. */
+  gfc_new_file ();
 
   if (flag_preprocess_only)
     return false;
diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h
index 3c7a8434d07..7ef835b211a 100644
--- a/gcc/fortran/gfortran.h
+++ b/gcc/fortran/gfortran.h
@@ -3032,7 +3032,7 @@ void gfc_scanner_init_1 (void);
 void gfc_add_include_path (const char *, bool, bool, bool, bool);
 void gfc_add_intrinsic_modules_path (const char *);
 void gfc_release_include_path (void);
-void gfc_check_include_dirs (void);
+void gfc_check_include_dirs (bool);
 FILE *gfc_open_included_file (const char *, bool, bool);
 
 int gfc_at_end (void);
@@ -3064,7 +3064,7 @@ gfc_char_t gfc_peek_char (void);
 char gfc_peek_ascii_char (void);
 void gfc_error_recovery (void);
 void gfc_gobble_whitespace (void);
-bool gfc_new_file (void);
+void gfc_new_file (void);
 const char * gfc_read_orig_filename (const char *, const char **);
 
 extern gfc_source_form gfc_current_form;
diff --git a/gcc/fortran/options.c b/gcc/fortran/options.c
index d789397515a..016b70443ae 100644
--- a/gcc/fortran/options.c
+++ b/gcc/fortran/options.c
@@ -251,14 +251,20 @@ gfc_post_options (const char **pfilename)
 {
   const char *filename = *pfilename, *canon_source_file = NULL;
   char *source_path;
+  bool verbose_missing_dir_warn;
   int i;
 
   /* This needs to be after the commandline has been processed.
      In Fortran, the options is by default enabled, in C/C++
-     by default disabled.  */
+     by default disabled.
+     If not enabled explicitly by the user, only warn for -I
+     and -J, otherwise warn for all include paths.  */
+  verbose_missing_dir_warn
+    = (global_options_set.x_cpp_warn_missing_include_dirs
+       && global_options.x_cpp_warn_missing_include_dirs);
   SET_OPTION_IF_UNSET (&global_options, &global_options_set,
 		       cpp_warn_missing_include_dirs, 1);
-  gfc_check_include_dirs ();
+  gfc_check_include_dirs (verbose_missing_dir_warn);
 
   /* Finalize DEC flags.  */
   post_dec_flags (flag_dec);
@@ -339,10 +345,13 @@ gfc_post_options (const char **pfilename)
       source_path = (char *) alloca (i + 1);
       memcpy (source_path, canon_source_file, i);
       source_path[i] = 0;
-      gfc_add_include_path (source_path, true, true, true, false);
+      /* Only warn if the directory is different from the input file as
+	 if that one is not found, already an error is shown.  */
+      bool warn = gfc_option.flag_preprocessed && gfc_source_file != filename;
+      gfc_add_include_path (source_path, true, true, warn, false);
     }
   else
-    gfc_add_include_path (".", true, true, true, false);
+    gfc_add_include_path (".", true, true, false, false);
 
   if (canon_source_file != gfc_source_file)
     free (CONST_CAST (char *, canon_source_file));
@@ -490,7 +499,7 @@ gfc_post_options (const char **pfilename)
     gfc_fatal_error ("Maximum subrecord length cannot exceed %d",
 		     MAX_SUBRECORD_LENGTH);
 
-  gfc_cpp_post_options ();
+  gfc_cpp_post_options (verbose_missing_dir_warn);
 
   if (gfc_option.allow_std & GFC_STD_F2008)
     lang_hooks.name = "GNU Fortran2008";
diff --git a/gcc/fortran/scanner.c b/gcc/fortran/scanner.c
index 6fe74bd5b8d..52124bd5d36 100644
--- a/gcc/fortran/scanner.c
+++ b/gcc/fortran/scanner.c
@@ -307,9 +307,9 @@ gfc_do_check_include_dir (const char *path, bool warn)
       if (errno != ENOENT)
 	gfc_warning_now (0, "Include directory %qs: %s",
 			 path, xstrerror(errno));
-      else if (warn && !gfc_cpp_enabled ())
+      else if (warn)
 	  gfc_warning_now (OPT_Wmissing_include_dirs,
-			     "Nonexistent include directory %qs", path);
+			   "Nonexistent include directory %qs", path);
       return false;
     }
   else if (!S_ISDIR (st.st_mode))
@@ -323,7 +323,7 @@ gfc_do_check_include_dir (const char *path, bool warn)
 /* In order that -W(no-)missing-include-dirs works, the diagnostic can only be
    run after processing the commandline.  */
 static void
-gfc_do_check_include_dirs (gfc_directorylist **list)
+gfc_do_check_include_dirs (gfc_directorylist **list, bool do_warn)
 {
   gfc_directorylist *prev, *q, *n;
   prev = NULL;
@@ -331,7 +331,7 @@ gfc_do_check_include_dirs (gfc_directorylist **list)
   while (n)
     {
       q = n; n = n->next;
-      if (gfc_do_check_include_dir (q->path, q->warn))
+      if (gfc_do_check_include_dir (q->path, q->warn && do_warn))
 	{
 	  prev = q;
 	  continue;
@@ -346,10 +346,16 @@ gfc_do_check_include_dirs (gfc_directorylist **list)
 }
 
 void
-gfc_check_include_dirs ()
+gfc_check_include_dirs (bool verbose_missing_dir_warn)
 {
-  gfc_do_check_include_dirs (&include_dirs);
-  gfc_do_check_include_dirs (&intrinsic_modules_dirs);
+  /* This is a bit convoluted: If gfc_cpp_enabled () and
+     verbose_missing_dir_warn, the warning is shown by libcpp. Otherwise,
+     it is shown here, still conditional on OPT_Wmissing_include_dirs.  */
+  bool warn = !gfc_cpp_enabled () || !verbose_missing_dir_warn;
+  gfc_do_check_include_dirs (&include_dirs, warn);
+  gfc_do_check_include_dirs (&intrinsic_modules_dirs, verbose_missing_dir_warn);
+  if (gfc_option.module_dir && gfc_cpp_enabled ())
+    gfc_do_check_include_dirs (&include_dirs, true);
 }
 
 /* Adds path to the list pointed to by list.  */
@@ -2771,7 +2777,7 @@ load_file (const char *realfilename, const char *displayedname, bool initial)
    it tries to determine the source form from the filename, defaulting
    to free form.  */
 
-bool
+void
 gfc_new_file (void)
 {
   bool result;
@@ -2789,6 +2795,9 @@ gfc_new_file (void)
   else
     result = load_file (gfc_source_file, NULL, true);
 
+  if (!result)
+    exit (FATAL_EXIT_CODE);
+
   gfc_current_locus.lb = line_head;
   gfc_current_locus.nextc = (line_head == NULL) ? NULL : line_head->line;
 
@@ -2799,8 +2808,6 @@ gfc_new_file (void)
 
   exit (SUCCESS_EXIT_CODE);
 #endif
-
-  return result;
 }
 
 static char *
diff --git a/gcc/testsuite/gfortran.dg/include_14.f90 b/gcc/testsuite/gfortran.dg/include_14.f90
index b306b2c813b..8110e49bf43 100644
--- a/gcc/testsuite/gfortran.dg/include_14.f90
+++ b/gcc/testsuite/gfortran.dg/include_14.f90
@@ -1,5 +1,6 @@
-! { dg-additional-options "-cpp -idirafter /fdaf/ -I bar" }
+! { dg-additional-options "-cpp -idirafter /fdaf/ -I bar -J foo/bar" }
 end
+! default: warn for -I and -J but ignore other options.
+! { dg-warning "Nonexistent include directory 'bar/'" "" { target *-*-* } 0 }
+! { dg-warning "Nonexistent include directory 'foo/bar/'" "" { target *-*-* } 0 }
 
-! { dg-warning "/fdaf/: No such file or directory" "" { target *-*-* } 0 }
-! { dg-warning "bar: No such file or directory" "" { target *-*-* } 0 }
diff --git a/gcc/testsuite/gfortran.dg/include_15.f90 b/gcc/testsuite/gfortran.dg/include_15.f90
index 4944282f931..068dcef5826 100644
--- a/gcc/testsuite/gfortran.dg/include_15.f90
+++ b/gcc/testsuite/gfortran.dg/include_15.f90
@@ -1,5 +1,6 @@
-! { dg-additional-options "-cpp -idirafter /fdaf/ -I bar -Wmissing-include-dirs" }
+! { dg-additional-options "-cpp -idirafter /fdaf/ -I bar -J foo/bar -Wmissing-include-dirs" }
 end
 
-! { dg-warning "/fdaf/: No such file or directory" "" { target *-*-* } 0 }
-! { dg-warning "bar: No such file or directory" "" { target *-*-* } 0 }
+! { dg-warning " /fdaf/: No such file or directory" "" { target *-*-* } 0 }
+! { dg-warning " bar: No such file or directory" "" { target *-*-* } 0 }
+! { dg-warning " foo/bar: No such file or directory" "" { target *-*-* } 0 }
diff --git a/gcc/testsuite/gfortran.dg/include_16.f90 b/gcc/testsuite/gfortran.dg/include_16.f90
index 45794f28e73..65e4c7efdad 100644
--- a/gcc/testsuite/gfortran.dg/include_16.f90
+++ b/gcc/testsuite/gfortran.dg/include_16.f90
@@ -1,2 +1,2 @@
-! { dg-additional-options "-cpp -idirafter /fdaf/ -I bar -Wno-missing-include-dirs" }
+! { dg-additional-options "-cpp -idirafter /fdaf/ -I bar -J foo/bar -Wno-missing-include-dirs" }
 end
diff --git a/gcc/testsuite/gfortran.dg/include_17.f90 b/gcc/testsuite/gfortran.dg/include_17.f90
index 0ed5c86d323..06677590be3 100644
--- a/gcc/testsuite/gfortran.dg/include_17.f90
+++ b/gcc/testsuite/gfortran.dg/include_17.f90
@@ -1,4 +1,6 @@
 ! { dg-do compile }
-! { dg-options "-I foo-bar -Wno-missing-include-dirs" }
+! { dg-options "-I foo-bar -J foo/bar" }
 end 
+! { dg-warning "Nonexistent include directory 'foo-bar/'" "" { target *-*-* } 0 }
+! { dg-warning "Nonexistent include directory 'foo/bar/'" "" { target *-*-* } 0 }
 
diff --git a/gcc/testsuite/gfortran.dg/include_18.f90 b/gcc/testsuite/gfortran.dg/include_18.f90
index ca69df382fe..b74a585bf1b 100644
--- a/gcc/testsuite/gfortran.dg/include_18.f90
+++ b/gcc/testsuite/gfortran.dg/include_18.f90
@@ -1,3 +1,5 @@
 ! { dg-do compile }
-! { dg-options "-I nothere -Wno-missing-include-dirs" }
+! { dg-options "-I nothere -J neither/here -Wmissing-include-dirs" }
 end 
+! { dg-warning "Nonexistent include directory 'nothere/'" "" { target *-*-* } 0 }
+! { dg-warning "Nonexistent include directory 'neither/here/'" "" { target *-*-* } 0 }
diff --git a/gcc/testsuite/gfortran.dg/include_19.f90 b/gcc/testsuite/gfortran.dg/include_19.f90
index 2a068174b7e..74049295281 100644
--- a/gcc/testsuite/gfortran.dg/include_19.f90
+++ b/gcc/testsuite/gfortran.dg/include_19.f90
@@ -1,4 +1,4 @@
 ! { dg-do compile }
-! { dg-options "-J foobar/foo -Wno-missing-include-dirs" }
+! { dg-options "-I nothere -J foobar/foo -Wno-missing-include-dirs" }
 program main
 end program main
diff --git a/libgfortran/configure b/libgfortran/configure
index 350159aa4fe..4810b9b032e 100755
--- a/libgfortran/configure
+++ b/libgfortran/configure
@@ -5985,7 +5985,7 @@ fi
 
 # Add -Wall -fno-repack-arrays -fno-underscoring if we are using GCC.
 if test "x$GCC" = "xyes"; then
-  AM_FCFLAGS="-I . -Wall -Werror -fimplicit-none -fno-repack-arrays -fno-underscoring -Wno-missing-include-dirs"
+  AM_FCFLAGS="-I . -Wall -Werror -fimplicit-none -fno-repack-arrays -fno-underscoring"
   ## We like to use C11 and C99 routines when available.  This makes
   ## sure that
   ## __STDC_VERSION__ is set such that libc includes make them available.
diff --git a/libgfortran/configure.ac b/libgfortran/configure.ac
index a3550b4e5d0..a77509801e6 100644
--- a/libgfortran/configure.ac
+++ b/libgfortran/configure.ac
@@ -146,7 +146,7 @@ AM_PROG_CC_C_O
 
 # Add -Wall -fno-repack-arrays -fno-underscoring if we are using GCC.
 if test "x$GCC" = "xyes"; then
-  AM_FCFLAGS="-I . -Wall -Werror -fimplicit-none -fno-repack-arrays -fno-underscoring -Wno-missing-include-dirs"
+  AM_FCFLAGS="-I . -Wall -Werror -fimplicit-none -fno-repack-arrays -fno-underscoring"
   ## We like to use C11 and C99 routines when available.  This makes
   ## sure that
   ## __STDC_VERSION__ is set such that libc includes make them available.
diff --git a/libgomp/testsuite/libgomp.fortran/fortran.exp b/libgomp/testsuite/libgomp.fortran/fortran.exp
index 912dd2a743e..eb701311b6a 100644
--- a/libgomp/testsuite/libgomp.fortran/fortran.exp
+++ b/libgomp/testsuite/libgomp.fortran/fortran.exp
@@ -20,9 +20,6 @@ dg-init
 
 # Turn on OpenMP.
 lappend ALWAYS_CFLAGS "additional_flags=-fopenmp"
-# Silence warnings due to explicitly passed but nonexisting
-# -isystem <instdir>/target>/{sys-,}include (gfortran warns by default)
-lappend ALWAYS_CFLAGS "additional_flags=-Wno-missing-include-dirs"
 
 if { $blddir != "" } {
     set lang_source_re {^.*\.[fF](|90|95|03|08)$}
diff --git a/libgomp/testsuite/libgomp.oacc-fortran/fortran.exp b/libgomp/testsuite/libgomp.oacc-fortran/fortran.exp
index 22e91eca256..7365b320668 100644
--- a/libgomp/testsuite/libgomp.oacc-fortran/fortran.exp
+++ b/libgomp/testsuite/libgomp.oacc-fortran/fortran.exp
@@ -22,9 +22,6 @@ dg-init
 
 # Turn on OpenACC.
 lappend ALWAYS_CFLAGS "additional_flags=-fopenacc"
-# Silence warnings due to explicitly passed but nonexisting
-# -isystem <instdir>/target>/{sys-,}include (gfortran warns by default)
-lappend ALWAYS_CFLAGS "additional_flags=-Wno-missing-include-dirs"
 
 if { $blddir != "" } {
     set lang_source_re {^.*\.[fF](|90|95|03|08)$}

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Patch] Fortran: Improve -Wmissing-include-dirs warnings [PR55534]
  2021-09-21 19:22     ` [Patch] Fortran: Improve -Wmissing-include-dirs warnings [PR55534] Tobias Burnus
@ 2021-09-22  9:13       ` Thomas Schwinge
  2021-09-22 18:29       ` Harald Anlauf
  1 sibling, 0 replies; 9+ messages in thread
From: Thomas Schwinge @ 2021-09-22  9:13 UTC (permalink / raw)
  To: Tobias Burnus; +Cc: gcc-patches, fortran

Hi Tobias!

On 2021-09-21T21:22:44+0200, Tobias Burnus <tobias@codesourcery.com> wrote:
> While the previous patch fixed -Wno-missing-include-dirs and sorted
> out some inconsistencies with libcpp warnings, it had two issues:
>
> * Some superfluous warnings were printed, e.g. for
>      gfortran nonexisting/file.f90
>    there was a warning about include path "nonexisting" not existing
>    and twice the error that the "nonexisting/file.f90" could not be
>    read.
>
> * At least as invoked when build GCC or when running the GCC testsuite,
>    the passed -B -isystem etc. arguments lead to proper but pointless
>    diagnostic about 'finclude' or other directories not being found,
>    causing excess-error FAILS and -Werror build fails.

ACK, I too had seen those (with '-cpp' only?), but not yet reported.

> While the latter could be fixed by adding -Wno-missing-include-dirs,
> it still felt like the wrong approach.

ACK, and also for the ones you already had added.  ;-)

> While the testsuite does run for me, others reported that they do
> see missing-include-dirs warnings. Instead of adding a bunch of
> -Wno-missing-include-dirs to the test config, I now only warn for
> -I and -J by default (similar to previous state) and only do a full
> warnings when the user requested passes the -Wmissing-include-dirs
> explicitly. The Fortran behavior is now also properly documented
> in the manual.

ACK, such an approach seems reasonable to me.  (But I can't formally
approve, of course.)


Grüße
 Thomas
-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Patch] Fortran: Improve -Wmissing-include-dirs warnings [PR55534]
  2021-09-21 19:22     ` [Patch] Fortran: Improve -Wmissing-include-dirs warnings [PR55534] Tobias Burnus
  2021-09-22  9:13       ` Thomas Schwinge
@ 2021-09-22 18:29       ` Harald Anlauf
  2021-09-22 22:06         ` Fortran: Improve file-reading error diagnostic [PR55534] (was: Re: [Patch] Fortran: Improve -Wmissing-include-dirs warnings [PR55534]) Tobias Burnus
  1 sibling, 1 reply; 9+ messages in thread
From: Harald Anlauf @ 2021-09-22 18:29 UTC (permalink / raw)
  To: gcc-patches; +Cc: fortran

Hi Tobias,

Am 21.09.21 um 21:22 schrieb Tobias Burnus:
> While the previous patch fixed -Wno-missing-include-dirs and sorted
> out some inconsistencies with libcpp warnings, it had two issues:
> 
> * Some superfluous warnings were printed, e.g. for
>      gfortran nonexisting/file.f90
>    there was a warning about include path "nonexisting" not existing
>    and twice the error that the "nonexisting/file.f90" could not be
>    read.
> 
> * At least as invoked when build GCC or when running the GCC testsuite,
>    the passed -B -isystem etc. arguments lead to proper but pointless
>    diagnostic about 'finclude' or other directories not being found,
>    causing excess-error FAILS and -Werror build fails.
> 
> While the latter could be fixed by adding -Wno-missing-include-dirs,
> it still felt like the wrong approach.

I concur.

> While the testsuite does run for me, others reported that they do
> see missing-include-dirs warnings. Instead of adding a bunch of
> -Wno-missing-include-dirs to the test config, I now only warn for
> -I and -J by default (similar to previous state) and only do a full
> warnings when the user requested passes the -Wmissing-include-dirs
> explicitly. The Fortran behavior is now also properly documented
> in the manual.

I had actually only looked at my use cases (-I, -J), which worked
as expected.

What I find a bit confusing - from the viewpoint of a user - is the
case of using the preprocessor (-cpp), as one gets e.g.

<built-in>: Warning: ./no/such/dir: No such file or directory 
[-Wmissing-include-dirs]

while without -cpp:

f951: Warning: Nonexistent include directory './no/such/dir/' 
[-Wmissing-include-dirs]

If you feel like me that the printing "<built-in>" should be
documented, feel free to do so.  I failed to find it.

(In some build setups the users do not normally see the actual
command line invoking the compiler, and they have to guess.)

> In order to handle the silencing of the diagnostic and to avoid
> double output via the Fortran code and libcpp (or rather: gcc/incpath.c),
> I had to add some not that clean and obvious diagnostic flags.
> I hope they still make sense and are somewhat readable.
> 
> OK? Comments?

I think that's good to go.  Even better if you have a user-friendly
solution to my above comment.

Thanks for the patch!

Harald

> Tobias
> 
> PS: There is also some inconsistency whether fprintf stderr and
> gfc_error is used. All calls in load_file could be fatal errors
> as all exist with an error - and similar issues get different
> error messages for no good reason. I have not tried to solve
> this issue – but can if deemed reasonable as follow-up patch.
> 
> -----------------
> Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 
> 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: 
> Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; 
> Registergericht München, HRB 106955



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Fortran: Improve file-reading error diagnostic [PR55534] (was: Re: [Patch] Fortran: Improve -Wmissing-include-dirs warnings [PR55534])
  2021-09-22 18:29       ` Harald Anlauf
@ 2021-09-22 22:06         ` Tobias Burnus
       [not found]           ` <siipui$dr9$1@ciao.gmane.io>
  0 siblings, 1 reply; 9+ messages in thread
From: Tobias Burnus @ 2021-09-22 22:06 UTC (permalink / raw)
  To: Harald Anlauf, gcc-patches, fortran

[-- Attachment #1: Type: text/plain, Size: 2023 bytes --]

Hi Harald,

On 22.09.21 20:29, Harald Anlauf via Gcc-patches wrote:
> What I find a bit confusing - from the viewpoint of a user - is the
> case of using the preprocessor (-cpp), as one gets e.g.
>
> <built-in>: Warning: ./no/such/dir: No such file or directory
> [-Wmissing-include-dirs]
>
> while without -cpp:
>
> f951: Warning: Nonexistent include directory './no/such/dir/'
> [-Wmissing-include-dirs]

C/C++ do something likewise (grep for that string).

The reason for the <built-in> is the code in cpp.c's gfc_cpp_init,
which uses:
   cpp_change_file (cpp_in, LC_RENAME, _("<built-in>"));

It might be possible to reset it by passing NULL to it, at the end
of that function but I don't know whether that causes side effects.
At least linemap_add then uses set->depth--.
It might work just fine, but I do not know.
(Additionally, cb_file_change or print_line needs to be updated
to handle to_file == NULL.)

Feel free to experiment there. Otherwise, I leave it as is.

  * * *

However, this patch now improves the diagnostic printed by
load_file – and uses directly an fatal error instead of
a usual error and then propagating the error through.

Errors are now also properly colored.

Note:
* -fpre-included= is not easily testable. It works when calling
   the compiler itself (f951) but the driver (gfortran) overrides
   it here with:
    -fpre-include=/usr/include/finclude/math-vector-fortran.h
   which exits.

* I did not include the test "include_22.f90" with:
     include "include_22.f90"  ! { dg-error "File 'include_22.f90' is being included recursively" }
   as the error message seemingly confused DejaGNU and causes it
   to enter an endless loop.

OK for mainline?

Tobias

-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955

[-- Attachment #2: file-not-found-diag.diff --]
[-- Type: text/x-patch, Size: 6743 bytes --]

Fortran: Improve file-reading error diagnostic [PR55534]

	PR fortran/55534

gcc/fortran/ChangeLog:

	* scanner.c (load_file): Return void, call (gfc_)fatal_error for
	all errors.
	(include_line, include_stmt, gfc_new_file): Remove exit call
	for failed load_file run.

gcc/testsuite/ChangeLog:

	* gfortran.dg/include_9.f90: Add dg-prune-output.
	* gfortran.dg/include_23.f90: New test.
	* gfortran.dg/include_24.f90: New test.

 gcc/fortran/scanner.c                    | 66 ++++++++++++--------------------
 gcc/testsuite/gfortran.dg/include_23.f90 |  4 ++
 gcc/testsuite/gfortran.dg/include_24.f90 |  4 ++
 gcc/testsuite/gfortran.dg/include_9.f90  |  1 +
 4 files changed, 33 insertions(+), 42 deletions(-)

diff --git a/gcc/fortran/scanner.c b/gcc/fortran/scanner.c
index 52124bd5d36..5a450692ba3 100644
--- a/gcc/fortran/scanner.c
+++ b/gcc/fortran/scanner.c
@@ -47,6 +47,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "toplev.h"	/* For set_src_pwd.  */
 #include "debug.h"
 #include "options.h"
+#include "diagnostic-core.h"  /* For fatal_error. */
 #include "cpp.h"
 #include "scanner.h"
 
@@ -2230,7 +2231,7 @@ preprocessor_line (gfc_char_t *c)
 }
 
 
-static bool load_file (const char *, const char *, bool);
+static void load_file (const char *, const char *, bool);
 
 /* include_line()-- Checks a line buffer to see if it is an include
    line.  If so, we call load_file() recursively to load the included
@@ -2396,9 +2397,7 @@ include_line (gfc_char_t *line)
 		   read by anything else.  */
 
   filename = gfc_widechar_to_char (begin, -1);
-  if (!load_file (filename, NULL, false))
-    exit (FATAL_EXIT_CODE);
-
+  load_file (filename, NULL, false);
   free (filename);
   return 1;
 }
@@ -2505,9 +2504,7 @@ include_stmt (gfc_linebuf *b)
       filename[i] = (unsigned char) c;
     }
   filename[length] = '\0';
-  if (!load_file (filename, NULL, false))
-    exit (FATAL_EXIT_CODE);
-
+  load_file (filename, NULL, false);
   free (filename);
 
 do_ret:
@@ -2525,9 +2522,11 @@ do_ret:
   return ret;
 }
 
+
+
 /* Load a file into memory by calling load_line until the file ends.  */
 
-static bool
+static void
 load_file (const char *realfilename, const char *displayedname, bool initial)
 {
   gfc_char_t *line;
@@ -2549,13 +2548,8 @@ load_file (const char *realfilename, const char *displayedname, bool initial)
 
   for (f = current_file; f; f = f->up)
     if (filename_cmp (filename, f->filename) == 0)
-      {
-	fprintf (stderr, "%s:%d: Error: File '%s' is being included "
-		 "recursively\n", current_file->filename, current_file->line,
-		 filename);
-	return false;
-      }
-
+      fatal_error (linemap_line_start (line_table, current_file->line, 0),
+		   "File %qs is being included recursively", filename);
   if (initial)
     {
       if (gfc_src_file)
@@ -2567,10 +2561,7 @@ load_file (const char *realfilename, const char *displayedname, bool initial)
 	input = gfc_open_file (realfilename);
 
       if (input == NULL)
-	{
-	  gfc_error_now ("Cannot open file %qs", filename);
-	  return false;
-	}
+	gfc_fatal_error ("Cannot open file %qs", filename);
     }
   else
     {
@@ -2579,22 +2570,20 @@ load_file (const char *realfilename, const char *displayedname, bool initial)
 	{
 	  /* For -fpre-include file, current_file is NULL.  */
 	  if (current_file)
-	    fprintf (stderr, "%s:%d: Error: Can't open included file '%s'\n",
-		     current_file->filename, current_file->line, filename);
+	    fatal_error (linemap_line_start (line_table, current_file->line, 0),
+			 "Cannot open included file %qs", filename);
 	  else
-	    fprintf (stderr, "Error: Can't open pre-included file '%s'\n",
-		     filename);
-
-	  return false;
+	    gfc_fatal_error ("Cannot open pre-included file %qs", filename);
 	}
       stat_result = stat (realfilename, &st);
-      if (stat_result == 0 && !S_ISREG(st.st_mode))
+      if (stat_result == 0 && !S_ISREG (st.st_mode))
 	{
-	  fprintf (stderr, "%s:%d: Error: Included path '%s'"
-		   " is not a regular file\n",
-		   current_file->filename, current_file->line, filename);
 	  fclose (input);
-	  return false;
+	  if (current_file)
+	    fatal_error (linemap_line_start (line_table, current_file->line, 0),
+			 "Included file %qs is not a regular file", filename);
+	  else
+	    gfc_fatal_error ("Included file %qs is not a regular file", filename);
 	}
     }
 
@@ -2768,7 +2757,6 @@ load_file (const char *realfilename, const char *displayedname, bool initial)
     add_file_change (NULL, current_file->inclusion_line + 1);
   current_file = current_file->up;
   linemap_add (line_table, LC_LEAVE, 0, NULL, 0);
-  return true;
 }
 
 
@@ -2780,23 +2768,17 @@ load_file (const char *realfilename, const char *displayedname, bool initial)
 void
 gfc_new_file (void)
 {
-  bool result;
-
-  if (flag_pre_include != NULL
-      && !load_file (flag_pre_include, NULL, false))
-    exit (FATAL_EXIT_CODE);
+  if (flag_pre_include != NULL)
+    load_file (flag_pre_include, NULL, false);
 
   if (gfc_cpp_enabled ())
     {
-      result = gfc_cpp_preprocess (gfc_source_file);
+      gfc_cpp_preprocess (gfc_source_file);
       if (!gfc_cpp_preprocess_only ())
-        result = load_file (gfc_cpp_temporary_file (), gfc_source_file, true);
+	load_file (gfc_cpp_temporary_file (), gfc_source_file, true);
     }
   else
-    result = load_file (gfc_source_file, NULL, true);
-
-  if (!result)
-    exit (FATAL_EXIT_CODE);
+    load_file (gfc_source_file, NULL, true);
 
   gfc_current_locus.lb = line_head;
   gfc_current_locus.nextc = (line_head == NULL) ? NULL : line_head->line;
diff --git a/gcc/testsuite/gfortran.dg/include_23.f90 b/gcc/testsuite/gfortran.dg/include_23.f90
new file mode 100644
index 00000000000..421ddda87bc
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/include_23.f90
@@ -0,0 +1,4 @@
+implicit none
+include "nonexisting/file.f90"  ! { dg-error "Cannot open included file 'nonexisting/file.f90'" }
+end
+! { dg-prune-output "compilation terminated." }
diff --git a/gcc/testsuite/gfortran.dg/include_24.f90 b/gcc/testsuite/gfortran.dg/include_24.f90
new file mode 100644
index 00000000000..1fe9eb57625
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/include_24.f90
@@ -0,0 +1,4 @@
+implicit none
+include "."  ! { dg-error "Included file '.' is not a regular file" }
+end
+! { dg-prune-output "compilation terminated." }
diff --git a/gcc/testsuite/gfortran.dg/include_9.f90 b/gcc/testsuite/gfortran.dg/include_9.f90
index c4ef50f6e50..6b0648b3ee5 100644
--- a/gcc/testsuite/gfortran.dg/include_9.f90
+++ b/gcc/testsuite/gfortran.dg/include_9.f90
@@ -4,3 +4,4 @@
       program main
       end program
 ! { dg-error "is not a regular file" " " { target *-*-* } 3 }
+! { dg-prune-output "compilation terminated." }

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Fortran: Improve file-reading error diagnostic [PR55534] (was: Re: [Patch] Fortran: Improve -Wmissing-include-dirs warnings [PR55534])
       [not found]           ` <siipui$dr9$1@ciao.gmane.io>
@ 2021-09-24  8:41             ` Tobias Burnus
  0 siblings, 0 replies; 9+ messages in thread
From: Tobias Burnus @ 2021-09-24  8:41 UTC (permalink / raw)
  To: Harald Anlauf, fortran; +Cc: gcc-patches

On 23.09.21 23:01, Harald Anlauf via Fortran wrote:

> compiled with -cpp gives:
>
> pr55534-play.f90:4:2:
>
>     4 |   type t
>       |  1~~~~~~
> Fatal Error: no/such/file.inc: No such file or directory
> compilation terminated.
>
> If you have an easy solution for that one,

David has an easy but hackish solution, cf. https://gcc.gnu.org/PR100904

That's a GCC 7 regression, which also affects C/C++ but only when
compiling with -traditional-cpp, which gfortran does by default but
gcc/g++ don't.

Tobias

-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2021-09-24  8:42 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-17 13:02 [Patch] Fortran: Fix -Wno-missing-include-dirs handling [PR55534] Tobias Burnus
2021-09-17 18:45 ` Tobias Burnus
2021-09-20 20:33   ` Tobias Burnus
2021-09-20 21:33     ` Harald Anlauf
2021-09-21 19:22     ` [Patch] Fortran: Improve -Wmissing-include-dirs warnings [PR55534] Tobias Burnus
2021-09-22  9:13       ` Thomas Schwinge
2021-09-22 18:29       ` Harald Anlauf
2021-09-22 22:06         ` Fortran: Improve file-reading error diagnostic [PR55534] (was: Re: [Patch] Fortran: Improve -Wmissing-include-dirs warnings [PR55534]) Tobias Burnus
     [not found]           ` <siipui$dr9$1@ciao.gmane.io>
2021-09-24  8:41             ` Tobias Burnus

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).