Patchwork [Fortran] Fix PR 56224

login
register
mail settings
Submitter Thomas Koenig
Date Feb. 9, 2013, 1:14 p.m.
Message ID <51164BCD.9080101@netcologne.de>
Download mbox | patch
Permalink /patch/219397/
State New
Headers show

Comments

Thomas Koenig - Feb. 9, 2013, 1:14 p.m.
Am 09.02.2013 11:22, schrieb Tobias Burnus:

> Why did you put a FIXME there? What's wrong with adding the directory here?

I think module files are different enough from include files that I
would like to have them in different directories.

 > "Seems to break testing, all testcases emit
 > Warning: Nonexistent include directory "finclude"^M"

 > I think those only occurred when the compiler is not installed. Will 
 > they pop up again?

This version of the patch should fix that particular issue, and also has
no test cases.

Regression-tested.  OK?

2013-02-09  Thomas Koenig  <tkoenig@gcc.gnu.org>

         PR fortran/56224
         * gfortran.h (gfc_add_include_path):  Add boolean argument
         for warn.
         * scanner.c (gfc_add_include_path):  Pass along warn argument
         to add_path_to_list.
         * options.c (gfc_post_options):  Add true warn argument to
         gfc_add_include_path.
         (gfc_handle_module_path_options):  Likewise.
         (gfc_handle_option): Also gfc_add_include_path for intrinsic
         modules, without warning.
Steve Kargl - Feb. 9, 2013, 7:08 p.m.
On Sat, Feb 09, 2013 at 02:14:53PM +0100, Thomas Koenig wrote:
> Am 09.02.2013 11:22, schrieb Tobias Burnus:
> 
> > Why did you put a FIXME there? What's wrong with adding the directory here?
> 
> I think module files are different enough from include files that I
> would like to have them in different directories.
> 

IIR the history correctly, the name finclude was chosen to
mean 'Fortran include' to differentiate it from the traditional
location of C header files.  The 'include' in 'Fortran include'
was meant to mean any file that gfortran would needed (ie.,
could include) through an INCLUDE statement or USE statement.
finclude currently contains omp_lib.f90, omp_lib.h, omp_lib.mod,
and omp_lib_kinds.mod.   It seems to me that adding, say, a fmodule 
for omp_lib.mod and omp_lib_kinds.mod is adding needless complication
to the search paths.  Do we also move omp_lib.f90 to its own 
directory?
Tobias Burnus - Feb. 12, 2013, 10:15 a.m.
Thomas Koenig wrote:
> Am 09.02.2013 11:22, schrieb Tobias Burnus:
>
>> Why did you put a FIXME there? What's wrong with adding the directory 
>> here?
>
> I think module files are different enough from include files that I
> would like to have them in different directories.

I think they are similar enough to be in one directory - especially as 
include files are very unlikely to ever end in ".mod" and, thus, they 
can happily live alongside each other. Hence, I wouldn't put a FIXME there.

> This version of the patch should fix that particular issue, and also has
> no test cases.
> Regression-tested.  OK?

OK. Thanks for the patch after fixing the nit below.

> 2013-02-09  Thomas Koenig <tkoenig@gcc.gnu.org>
>
>         PR fortran/56224
>         * gfortran.h (gfc_add_include_path):  Add boolean argument
>         for warn.
>         * scanner.c (gfc_add_include_path):  Pass along warn argument
>         to add_path_to_list.
>         * options.c (gfc_post_options):  Add true warn argument to
>         gfc_add_include_path.
>         (gfc_handle_module_path_options):  Likewise.
>         (gfc_handle_option): Also gfc_add_include_path for intrinsic
>         modules, without warning.

> -gfc_add_include_path (const char *path, bool use_for_modules, bool file_dir)
> +gfc_add_include_path (const char *path, bool use_for_modules, bool file_dir, bool warn)

That line is too long.


Tobias
Thomas Koenig - Feb. 13, 2013, 7:25 p.m.
*ping*

I'd like to get this into 4.8 before release.

 > This version of the patch should fix that particular issue, and also has
> no test cases.
>
> Regression-tested.  OK?

I can leave out the FIXME if people object.

	Thomas
Tobias Burnus - Feb. 13, 2013, 7:38 p.m.
Thomas Koenig wrote:
> *ping*
* pong *

> I'd like to get this into 4.8 before release.

Others as well, given that it is a release-blocking P1 regression …

However, I have already approved it: 
http://gcc.gnu.org/ml/fortran/2013-02/msg00061.html

Tobias

Patch

Index: gfortran.h
===================================================================
--- gfortran.h	(Revision 195686)
+++ gfortran.h	(Arbeitskopie)
@@ -2378,7 +2378,7 @@  match gfc_match_char_spec (gfc_typespec *);
 void gfc_scanner_done_1 (void);
 void gfc_scanner_init_1 (void);
 
-void gfc_add_include_path (const char *, bool, bool);
+void gfc_add_include_path (const char *, bool, bool, bool);
 void gfc_add_intrinsic_modules_path (const char *);
 void gfc_release_include_path (void);
 FILE *gfc_open_included_file (const char *, bool, bool);
Index: scanner.c
===================================================================
--- scanner.c	(Revision 195686)
+++ scanner.c	(Arbeitskopie)
@@ -375,9 +375,9 @@  add_path_to_list (gfc_directorylist **list, const
 
 
 void
-gfc_add_include_path (const char *path, bool use_for_modules, bool file_dir)
+gfc_add_include_path (const char *path, bool use_for_modules, bool file_dir, bool warn)
 {
-  add_path_to_list (&include_dirs, path, use_for_modules, file_dir, true);
+  add_path_to_list (&include_dirs, path, use_for_modules, file_dir, warn);
 
   /* For '#include "..."' these directories are automatically searched.  */
   if (!file_dir)
Index: options.c
===================================================================
--- options.c	(Revision 195686)
+++ options.c	(Arbeitskopie)
@@ -337,10 +337,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);
+      gfc_add_include_path (source_path, true, true, true);
     }
   else
-    gfc_add_include_path (".", true, true);
+    gfc_add_include_path (".", true, true, true);
 
   if (canon_source_file != gfc_source_file)
     free (CONST_CAST (char *, canon_source_file));
@@ -498,7 +498,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);
+  gfc_add_include_path (gfc_option.module_dir, true, false, true);
 
   strcat (gfc_option.module_dir, "/");
 }
@@ -844,6 +844,11 @@  gfc_handle_option (size_t scode, const char *arg,
 
     case OPT_fintrinsic_modules_path:
     case OPT_fintrinsic_modules_path_:
+
+      /* FIXME:  This is needed because omp_lib.h is in a
+	 directory together with intrinsic modules.  */
+      gfc_add_include_path (arg, false, false, false);
+
       gfc_add_intrinsic_modules_path (arg);
       break;
 
@@ -978,7 +983,7 @@  gfc_handle_option (size_t scode, const char *arg,
       break;
 
     case OPT_I:
-      gfc_add_include_path (arg, true, false);
+      gfc_add_include_path (arg, true, false, true);
       break;
 
     case OPT_J: