[patch, fortran] Fix PR 54033, problems with -I

[patch, fortran] Fix PR 54033, problems with -I

[Thomas Koenig]

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

Hello world,

the attached, rather obvious patch emits warnings for several
cases where there is something wrong with include directories.
No test case because I couldn't figure out how to test for a
warning with no line number.

OK for trunk?

	Thomas

2012-07-26  Thomas König  <tkoenig@gcc.gnu.org>

         PR fortran/54033
         * scanner.c (add_path_to_list): Emit warning if an error occurs
         for an include path, if it is not present or if it is not a
         directory.  Do not add the path in these cases.

[-- Attachment #2: p1.diff --]
[-- Type: text/x-patch, Size: 921 bytes --]

Index: scanner.c
===================================================================
--- scanner.c	(Revision 189754)
+++ scanner.c	(Arbeitskopie)
@@ -311,12 +311,31 @@ add_path_to_list (gfc_directorylist **list, const
 {
   gfc_directorylist *dir;
   const char *p;
-
+  struct stat st;
+  
   p = path;
   while (*p == ' ' || *p == '\t')  /* someone might do "-I include" */
     if (*p++ == '\0')
       return;
 
+  if (stat (p, &st))
+    {
+      if (errno != ENOENT)
+	gfc_warning_now ("Include directory \"%s\": %s", path,
+			 xstrerror(errno));
+      else
+	/* FIXME:  Also support -Wmissing-include-dirs.  */
+	gfc_warning_now ("Include directory \"%s\" does not exist",
+			 path);
+      return;
+    }
+
+  else if (!S_ISDIR (st.st_mode))
+    {
+      gfc_warning_now ("\"%s\" is not a directory", path);
+      return;
+    }
+
   if (head || *list == NULL)
     {
       dir = XCNEW (gfc_directorylist);

Re: [patch, fortran] Fix PR 54033, problems with -I

[Janis Johnson]

On 07/26/2012 10:16 AM, Thomas Koenig wrote:

> No test case because I couldn't figure out how to test for a
> warning with no line number.

Try using line number 0.

Janis

Re: [patch, fortran] Fix PR 54033, problems with -I

[Thomas Koenig]

Hi Janis,

> On 07/26/2012 10:16 AM, Thomas Koenig wrote:
>
>> No test case because I couldn't figure out how to test for a
>> warning with no line number.
>
> Try using line number 0.

That didn't work for me. Using

! { dg-do compile }
! { dg-options "-I include_6.f90 -I missing_dir" }
! { dg-warning "not a directory" "missing directory" target *-*-* 0 }
! { dg-warning "does not exist" "nonexisting directory" target *-*-* 0 }
end

got me

Warning: Include directory "include_6.f90" does not exist^M
Warning: Include directory "missing_dir" does not exist^M
output is:
Warning: Include directory "include_6.f90" does not exist^M
Warning: Include directory "missing_dir" does not exist^M

FAIL: gfortran.dg/include_6.f90  -O  (test for excess errors)
Excess errors:
:0:0: Warning: Include directory "include_6.f90" does not exist
:0:0: Warning: Include directory "missing_dir" does not exist

and

! { dg-do compile }
! { dg-options "-I include_6.f90 -I missing_dir" }
! { dg-warning "not a directory" "missing directory" target *-*-* 0 }
! { dg-warning "does not exist" "nonexisting directory" target *-*-* 0 }
! { dg-excess-errors "Include directory" }
end

resulted in an XFAIL:

Warning: Include directory "include_6.f90" does not exist^M
Warning: Include directory "missing_dir" does not exist^M
output is:
Warning: Include directory "include_6.f90" does not exist^M
Warning: Include directory "missing_dir" does not exist^M

XFAIL: gfortran.dg/include_6.f90  -O  (test for excess errors)
Excess errors:
:0:0: Warning: Include directory "include_6.f90" does not exist
:0:0: Warning: Include directory "missing_dir" does not exist

so dg-excess-errors seems to imply XFAIL.

The problem may be related to the fact that, when we process the
options, we do not yet have a file name, so dejagnu may have trouble
parsing the warning.

Any other ideas?

	Thomas

Re: [patch, fortran] Fix PR 54033, problems with -I

[Janis Johnson]

On 07/27/2012 01:06 PM, Thomas Koenig wrote:
> Hi Janis,
> 
>> On 07/26/2012 10:16 AM, Thomas Koenig wrote:
>>
>>> No test case because I couldn't figure out how to test for a
>>> warning with no line number.
>>
>> Try using line number 0.
> 
> That didn't work for me. Using
> 
> ! { dg-do compile }
> ! { dg-options "-I include_6.f90 -I missing_dir" }
> ! { dg-warning "not a directory" "missing directory" target *-*-* 0 }
> ! { dg-warning "does not exist" "nonexisting directory" target *-*-* 0 }
> end
> 
> got me
> 
> Warning: Include directory "include_6.f90" does not exist^M
> Warning: Include directory "missing_dir" does not exist^M
> output is:
> Warning: Include directory "include_6.f90" does not exist^M
> Warning: Include directory "missing_dir" does not exist^M
> 
> FAIL: gfortran.dg/include_6.f90  -O  (test for excess errors)
> Excess errors:
> :0:0: Warning: Include directory "include_6.f90" does not exist
> :0:0: Warning: Include directory "missing_dir" does not exist

Use "{ target *-*-* }" instead of "target *-*-*".

Notice that the two warnings have the same text, so the directive
looking for "not a directory" will fail.

Janis

[patch, fortran] Fix PR 54033, problems with -I, with test cases

[Thomas Koenig]

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

Hello world,

here is an updated patch for PR 54033, this time with test cases.
Thanks to Janis for pointing me in the right direction with these.

Regression-tested. OK for trunk?

Thomas

2012-07-29  Thomas König  <tkoenig@gcc.gnu.org>

         PR fortran/54033
         * scanner.c (add_path_to_list): Emit warning if an error occurs
         for an include path, if it is not present or if it is not a
         directory.  Do not add the path in these cases.

2012-07-29  Thomas König  <tkoenig@gcc.gnu.org>

         PR fortran/54033
         * gfortran.dg/include_6.f90:  New test case.
         * gfortran.dg/include_7.f90:  New test case.
         * gfortran.dg/include_3.f90:  Add dg-warning for missing directory.



[-- Attachment #2: include_6.f90 --]
[-- Type: text/x-fortran, Size: 121 bytes --]

! { dg-do compile }
! { dg-options "-I gfortran.log" }
! { dg-warning "is not a directory" "" { target *-*-* } 0 }
end 


[-- Attachment #3: include_7.f90 --]
[-- Type: text/x-fortran, Size: 144 bytes --]

! { dg-do compile }
! { dg-options "-I nothere" }
! { dg-warning "Nonexistent include directory" "missing directory" { target *-*-* } 0 }
end 


[-- Attachment #4: p3.diff --]
[-- Type: text/x-patch, Size: 1343 bytes --]

Index: fortran/scanner.c
===================================================================
--- fortran/scanner.c	(Revision 189754)
+++ fortran/scanner.c	(Arbeitskopie)
@@ -311,12 +311,30 @@ add_path_to_list (gfc_directorylist **list, const
 {
   gfc_directorylist *dir;
   const char *p;
-
+  struct stat st;
+  
   p = path;
   while (*p == ' ' || *p == '\t')  /* someone might do "-I include" */
     if (*p++ == '\0')
       return;
 
+  if (stat (p, &st))
+    {
+      if (errno != ENOENT)
+	gfc_warning_now ("Include directory \"%s\": %s", path,
+			 xstrerror(errno));
+      else
+	/* FIXME:  Also support -Wmissing-include-dirs.  */
+	gfc_warning_now ("Nonexistent include directory \"%s\"", path);
+      return;
+    }
+
+  else if (!S_ISDIR (st.st_mode))
+    {
+      gfc_warning_now ("\"%s\" is not a directory", path);
+      return;
+    }
+
   if (head || *list == NULL)
     {
       dir = XCNEW (gfc_directorylist);
Index: testsuite/gfortran.dg/include_3.f95
===================================================================
--- testsuite/gfortran.dg/include_3.f95	(Revision 189754)
+++ testsuite/gfortran.dg/include_3.f95	(Arbeitskopie)
@@ -24,3 +24,4 @@ end function
 
 ! { dg-do compile }
 ! { dg-options "-fpreprocessed -g3" }
+! { dg-warning "Nonexistent include directory" "missing directory" { target *-*-* } 0 }

Re: [patch, fortran] Fix PR 54033, problems with -I, with test cases

[Tobias Burnus]

On 07/29/2012 11:24 AM, Thomas Koenig wrote:
> here is an updated patch for PR 54033, this time with test cases.
> Thanks to Janis for pointing me in the right direction with these.
> Regression-tested. OK for trunk?

Ok. Thanks for the patch - and to Janis for the first review.

Can you eliminate the extra line before "else if"?

Tobias

> 2012-07-29 Thomas König  <tkoenig@gcc.gnu.org>
>
>         PR fortran/54033
>         * scanner.c (add_path_to_list): Emit warning if an error occurs
>         for an include path, if it is not present or if it is not a
>         directory.  Do not add the path in these cases.
>
> 2012-07-29  Thomas König  <tkoenig@gcc.gnu.org>
>
>         PR fortran/54033
>         * gfortran.dg/include_6.f90:  New test case.
>         * gfortran.dg/include_7.f90:  New test case.
>         * gfortran.dg/include_3.f90:  Add dg-warning for missing 
> directory.
>
>

Re: [patch, fortran] Fix PR 54033, problems with -I, with test cases

[Richard Guenther]

On Tue, Jul 31, 2012 at 3:47 PM, Tobias Burnus <burnus@net-b.de> wrote:
> On 07/29/2012 11:24 AM, Thomas Koenig wrote:
>>
>> here is an updated patch for PR 54033, this time with test cases.
>> Thanks to Janis for pointing me in the right direction with these.
>> Regression-tested. OK for trunk?
>
>
> Ok. Thanks for the patch - and to Janis for the first review.
>
> Can you eliminate the extra line before "else if"?

Seems to break testing, all testcases emit

Warning: Nonexistent include directory "finclude"^M

now and thus all testcases fail like

FAIL: gfortran.dg/alloc_comp_basics_1.f90  -O0  (test for excess errors)

Richard.

> Tobias
>
>
>> 2012-07-29 Thomas König  <tkoenig@gcc.gnu.org>
>>
>>         PR fortran/54033
>>         * scanner.c (add_path_to_list): Emit warning if an error occurs
>>         for an include path, if it is not present or if it is not a
>>         directory.  Do not add the path in these cases.
>>
>> 2012-07-29  Thomas König  <tkoenig@gcc.gnu.org>
>>
>>         PR fortran/54033
>>         * gfortran.dg/include_6.f90:  New test case.
>>         * gfortran.dg/include_7.f90:  New test case.
>>         * gfortran.dg/include_3.f90:  Add dg-warning for missing
>> directory.
>>
>>
>

Re: [patch, fortran] Fix PR 54033, problems with -I, with test cases

[Tobias Burnus]

On 08/02/2012 10:54 AM, Richard Guenther wrote:
> Seems to break testing, all testcases emit
>
> Warning: Nonexistent include directory "finclude"^M
>
> now and thus all testcases fail like
>
> FAIL: gfortran.dg/alloc_comp_basics_1.f90  -O0  (test for excess errors)

First, I actually wonder why it only lists "finclude" and not the full 
pathname.

I think the problem is that gfortran (the driver) passes something like
    -fintrinsic-modules-path 
/$PREFIX/lib64/gcc/x86_64-unknown-linux-gnu/4.8.0/finclude
to the actual compiler (f951).

That directory only exists if the compiler is installed but not if one 
runs the test suite without installing it. I wonder how the OpenMP tests 
handle it - there one needs the files from that directory. (It currently 
contains the following files: omp_lib.f90 omp_lib.h  omp_lib_kinds.mod  
omp_lib.mod.) Answer: Theose seemingly include "$BUILD/$triplet/libgomp" 
in the -I path, which also contain those files. (That probably clashes 
with "use, intrinsic :: omp_lib", but that shouldn't matter.)

I am not sure whether it is the best solution, but one possibility would 
be to ignore -fintrinsic-modules-path for the warning. (That assumes 
that the warning is (almost) never needed for an installed compiler.)

Tobias

Re: [patch, fortran] Fix PR 54033, problems with -I, with test cases

[Thomas Koenig]

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

Hi Tobias,

 > I am not sure whether it is the best solution, but one possibility would
> be to ignore -fintrinsic-modules-path for the warning. (That assumes
> that the warning is (almost) never needed for an installed compiler.)

I think this is the right approach.  The attached patch does this.
Regression-tested with the finclude directory from the installation.

OK for trunk?

	Thomas

2012-08-02  Thomas König  <tkoenig@gcc.gnu.org>

         PR fortran/54033
         * scanner.c (add_path_to_list):  New argument warn.  Don't
         warn if it is true.
         (gfc_add_include_path):  Warn if directory is missing.
         (gfc_add_intrinsic_modules_path):  Do not warn if directory
         is missing.
         * optinons.c (gfc_handle_option):  Do not add directory
         for intrinsic modules to normal include path.


[-- Attachment #2: regfix.diff --]
[-- Type: text/x-patch, Size: 1952 bytes --]

Index: scanner.c
===================================================================
--- scanner.c	(Revision 190054)
+++ scanner.c	(Arbeitskopie)
@@ -307,7 +307,7 @@ gfc_scanner_done_1 (void)
 
 static void
 add_path_to_list (gfc_directorylist **list, const char *path,
-		  bool use_for_modules, bool head)
+		  bool use_for_modules, bool head, bool warn)
 {
   gfc_directorylist *dir;
   const char *p;
@@ -324,8 +324,11 @@ add_path_to_list (gfc_directorylist **list, const
 	gfc_warning_now ("Include directory \"%s\": %s", path,
 			 xstrerror(errno));
       else
-	/* FIXME:  Also support -Wmissing-include-dirs.  */
-	gfc_warning_now ("Nonexistent include directory \"%s\"", path);
+	{
+	  /* FIXME:  Also support -Wmissing-include-dirs.  */
+	  if (warn)
+	    gfc_warning_now ("Nonexistent include directory \"%s\"", path);
+	}
       return;
     }
   else if (!S_ISDIR (st.st_mode))
@@ -363,7 +366,7 @@ add_path_to_list (gfc_directorylist **list, const
 void
 gfc_add_include_path (const char *path, bool use_for_modules, bool file_dir)
 {
-  add_path_to_list (&include_dirs, path, use_for_modules, file_dir);
+  add_path_to_list (&include_dirs, path, use_for_modules, file_dir, true);
 
   /* For '#include "..."' these directories are automatically searched.  */
   if (!file_dir)
@@ -374,7 +377,7 @@ gfc_add_include_path (const char *path, bool use_f
 void
 gfc_add_intrinsic_modules_path (const char *path)
 {
-  add_path_to_list (&intrinsic_modules_dirs, path, true, false);
+  add_path_to_list (&intrinsic_modules_dirs, path, true, false, false);
 }
 
 
Index: options.c
===================================================================
--- options.c	(Revision 189754)
+++ options.c	(Arbeitskopie)
@@ -819,7 +819,6 @@ gfc_handle_option (size_t scode, const char *arg,
       break;
 
     case OPT_fintrinsic_modules_path:
-      gfc_add_include_path (arg, false, false);
       gfc_add_intrinsic_modules_path (arg);
       break;
 

Re: [patch, fortran] Fix PR 54033, problems with -I, with test cases

[Thomas Koenig]

Hi,

> OK for trunk?

I will be on holiday starting tomorrow, so I would like to commit
this today, if possible (with a mistake in the ChangeLog noted
by Uros fixed.  Thanks!)

If not, somebody please commit this (or another fix).

Thomas

Re: [patch, fortran] Fix PR 54033, problems with -I, with test cases

[H.J. Lu]

On Fri, Aug 3, 2012 at 10:09 AM, Thomas Koenig <tkoenig@netcologne.de> wrote:
> Hi,
>
>> OK for trunk?
>
>
> I will be on holiday starting tomorrow, so I would like to commit
> this today, if possible (with a mistake in the ChangeLog noted
> by Uros fixed.  Thanks!)
>
> If not, somebody please commit this (or another fix).
>

I will commit it after verifying that it fixes the problem.

Thanks.


-- 
H.J.