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

2021-09-22 Thread Thomas Schwinge
Hi Tobias!

On 2021-09-21T21:22:44+0200, Tobias Burnus  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


[PATCH, Fortran] diagnostic for argument w/type parameters for assumed-type dummy

2021-09-22 Thread Sandra Loosemore
This patch is adds the missing diagnostic noted in PR fortran/101319. 
OK to commit?


-Sandra
commit 9d5b9062d728d1b1bf5acfb914e06d776bdcdb60
Author: Sandra Loosemore 
Date:   Wed Sep 22 07:49:17 2021 -0700

Fortran: diagnostic for argument w/type parameters for assumed-type dummy

2021-09-22  Sandra Loosemore  

	PR fortran/101319

gcc/fortran/
	* interface.c (gfc_compare_actual_formal): Extend existing
	assumed-type diagnostic to also check for argument with type
	parameters.

gcc/testsuite/
	* gfortran.dg/c-interop/assumed-type-dummy.f90: Remove xfail.

diff --git a/gcc/fortran/interface.c b/gcc/fortran/interface.c
index f9a7c9c..dae4b95 100644
--- a/gcc/fortran/interface.c
+++ b/gcc/fortran/interface.c
@@ -3183,21 +3183,21 @@ gfc_compare_actual_formal (gfc_actual_arglist **ap, gfc_formal_arglist *formal,
 			  is_elemental, where))
 	return false;
 
-  /* TS 29113, 6.3p2.  */
+  /* TS 29113, 6.3p2; F2018 15.5.2.4.  */
   if (f->sym->ts.type == BT_ASSUMED
 	  && (a->expr->ts.type == BT_DERIVED
 	  || (a->expr->ts.type == BT_CLASS && CLASS_DATA (a->expr
 	{
-	  gfc_namespace *f2k_derived;
-
-	  f2k_derived = a->expr->ts.type == BT_DERIVED
-			? a->expr->ts.u.derived->f2k_derived
-			: CLASS_DATA (a->expr)->ts.u.derived->f2k_derived;
-
-	  if (f2k_derived
-	  && (f2k_derived->finalizers || f2k_derived->tb_sym_root))
+	  gfc_symbol *derived = (a->expr->ts.type == BT_DERIVED
+ ? a->expr->ts.u.derived
+ : CLASS_DATA (a->expr)->ts.u.derived);
+	  gfc_namespace *f2k_derived = derived->f2k_derived;
+	  if (derived->attr.pdt_type
+	  || (f2k_derived
+		  && (f2k_derived->finalizers || f2k_derived->tb_sym_root)))
 	{
-	  gfc_error ("Actual argument at %L to assumed-type dummy is of "
+	  gfc_error ("Actual argument at %L to assumed-type dummy "
+			 "has type parameters or is of "
 			 "derived type with type-bound or FINAL procedures",
 			 &a->expr->where);
 	  return false;
diff --git a/gcc/testsuite/gfortran.dg/c-interop/assumed-type-dummy.f90 b/gcc/testsuite/gfortran.dg/c-interop/assumed-type-dummy.f90
index a14c9a5..24bdf2b 100644
--- a/gcc/testsuite/gfortran.dg/c-interop/assumed-type-dummy.f90
+++ b/gcc/testsuite/gfortran.dg/c-interop/assumed-type-dummy.f90
@@ -73,7 +73,7 @@ contains
 type(t4) :: a4
 
 call s1 (a1)  ! OK
-call s1 (a2)  ! { dg-error "assumed-type dummy" "pr101319" { xfail *-*-* } }
+call s1 (a2)  ! { dg-error "assumed-type dummy" }
 call s1 (a3)  ! { dg-error "assumed-type dummy" }
 call s1 (a4)  ! { dg-error "assumed-type dummy" }
   end subroutine


Re: [PATCH, Fortran] diagnostic for argument w/type parameters for assumed-type dummy

2021-09-22 Thread Tobias Burnus

On 22.09.21 16:58, Sandra Loosemore wrote:


This patch is adds the missing diagnostic noted in PR fortran/101319.
OK to commit?


LGTM. Thanks!

For reference, the F2018 wording is: "If the actual argument is of a
derived type that has type parameters, type-bound procedures, or final
subroutines, the dummy argument shall not be assumed-type."

Tobias


commit 9d5b9062d728d1b1bf5acfb914e06d776bdcdb60
Author: Sandra Loosemore
Date:   Wed Sep 22 07:49:17 2021 -0700

 Fortran: diagnostic for argument w/type parameters for assumed-type dummy

 2021-09-22  Sandra Loosemore

  PR fortran/101319

 gcc/fortran/
  * interface.c (gfc_compare_actual_formal): Extend existing
  assumed-type diagnostic to also check for argument with type
  parameters.

-
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


Re: PING**2 – Re: [Patch] Fortran: Handle allocated() with coindexed scalars [PR93834] (was: [PATCH] PR fortran/93834 - [9/10/11/12 Regression] ICE in trans_caf_is_present, at fortran/trans-intrinsic.

2021-09-22 Thread Tobias Burnus

(1) PING**2

(2) However, as it causes for others test-suite fails,* the
 https://gcc.gnu.org/pipermail/gcc-patches/2021-September/579965.html
 [Patch] Fortran: Improve -Wmissing-include-dirs warnings [PR55534]
is probably more important.

[* I don't see it locally as it probably uses and finds directories from
the install dir; however, others see it (hundreds of tails)
including the regression tracker at
https://gcc.gnu.org/pipermail/gcc-patches/2021-September/579963.html ]

(3) Also pending is
  https://gcc.gnu.org/pipermail/gcc-patches/2021-September/579903.html
  [Patch] Fortran: Fix assumed-size to assumed-rank passing [PR94070]
(Thanks Thomas for reviewing the auxiliary loop patch :-)

Tobias

PS: Also pending is the GFC <-> CFI descriptor conversion patch, but expect
a revised patch soon. (Fixes found issues, uses aux loop function, fixes
contiguous attribute handling, len=* with assumed-size/explicit-size arrays,
...)

On 16.09.21 14:26, Tobias Burnus wrote:

Patch PING – see comment in the follow-up email of the patch email -
and in the email(s) before in that thread.

Tobias

On 07.09.21 16:33, Tobias Burnus wrote:

Now I actually tested the patch – and fixed some issues.

OK? – It does add support for 'allocated(a[i])' by treating
it as 'allocated(a)', as 'a' must be collectively allocated
("established") on all images of the team.*

'a[i]' is (probably) an allocatable, following Malcolm in
answer to my question to the J3-list as linked below.

Tobias

* Ignoring issues related to failed images. It could
also be handled by fetching 'a' from the remote
image, but I am not sure that's better in terms of
handling failed images.

PS:
On 07.09.21 10:02, Tobias Burnus wrote:

Hi Harald,

I spend yesterday about two hours with this. Now I am still
tired but understand more. I think the confusion between the
two of us is due to wording and in which directions the
thoughts then go:


Talking about coindexed, all of a[i], b[i]%c and c%d[i] are
coindexed and there are many constraints like "shall not be
a coindexed variable" – which then rejects all of those.
That's what I was thinking of.

I think your starting point is that while ('a' = allocatable)
  a, b%a, c[5]%d(1)%a
are ALLOCATABLE, adding a subobject reference such as
  a(:), b%a(:,:), c[5]%d(1)%a(:,:,:)
makes the variable no longer allocatable.
I think that's what you were thinking of.

We then both argued along those different lines – which caused
the confusion as we both thought we talked about the same.


While those cases are clear, the question is whether
  a[i] or b%a[i]
is allocatable or not – assuming that 'a' is a scalar.
(For an array, '(:)' has to appear before the image-selector,
which in turn makes it nonallocatable.)


I tried to pinpoint the words for this in the standard – and
failed. I think I need a "how to read the Fortran standard" 101
and some long time actually reading it :-(

Malcolm has answered me – and he believes (but only offhand) that
  a[i]  and  b%a[i]
_are_ allocatable. See (6) at
https://mailman.j3-fortran.org/pipermail/j3/2021-September/013322.html


This implies that
  if ( allocated (a[i]) .and. allocated (b%a[i]) ) stop 1
is valid.

However, I do note that coarray allocatables have to be collectively
(de)allocated, therefore
  allocated (a[i]) .and. allocated (b%a[i])
is equivalent to
  allocated (a) .and. allocated (b%a)
at least assuming that no image has failed.


First: Does this answer all the questions you had and resolved the
confusion?
Secondly, do you agree about the last bits of the analysis?
Thirdly, what do you think of the attached patch?

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


Re: [Patch] Fortran: Handle allocated() with coindexed scalars [PR93834] (was: [PATCH] PR fortran/93834 - [9/10/11/12 Regression] ICE in trans_caf_is_present, at fortran/trans-intrinsic.c:8469)

2021-09-22 Thread Harald Anlauf via Fortran

Hi Tobias,

Am 07.09.21 um 16:33 schrieb Tobias Burnus:

Now I actually tested the patch – and fixed some issues.

OK? – It does add support for 'allocated(a[i])' by treating
it as 'allocated(a)', as 'a' must be collectively allocated
("established") on all images of the team.*

'a[i]' is (probably) an allocatable, following Malcolm in
answer to my question to the J3-list as linked below.


while still feeling somewhat unsure (given my previous comment
and the discussion), I think your patch is basically OK.

However, your testcase has a { dg-do compile }, so it does not
really do any runtime tests.  Is that intended?  If so, please
add a respective comment, or adjust the testcase.

Otherwise this LGTM.

Thanks for the patch!

Harald


Tobias

* Ignoring issues related to failed images. It could
also be handled by fetching 'a' from the remote
image, but I am not sure that's better in terms of
handling failed images.

PS:
On 07.09.21 10:02, Tobias Burnus wrote:

Hi Harald,

I spend yesterday about two hours with this. Now I am still
tired but understand more. I think the confusion between the
two of us is due to wording and in which directions the
thoughts then go:


Talking about coindexed, all of a[i], b[i]%c and c%d[i] are
coindexed and there are many constraints like "shall not be
a coindexed variable" – which then rejects all of those.
That's what I was thinking of.

I think your starting point is that while ('a' = allocatable)
  a, b%a, c[5]%d(1)%a
are ALLOCATABLE, adding a subobject reference such as
  a(:), b%a(:,:), c[5]%d(1)%a(:,:,:)
makes the variable no longer allocatable.
I think that's what you were thinking of.

We then both argued along those different lines – which caused
the confusion as we both thought we talked about the same.


While those cases are clear, the question is whether
  a[i] or b%a[i]
is allocatable or not – assuming that 'a' is a scalar.
(For an array, '(:)' has to appear before the image-selector,
which in turn makes it nonallocatable.)


I tried to pinpoint the words for this in the standard – and
failed. I think I need a "how to read the Fortran standard" 101
and some long time actually reading it :-(

Malcolm has answered me – and he believes (but only offhand) that
  a[i]  and  b%a[i]
_are_ allocatable. See (6) at
https://mailman.j3-fortran.org/pipermail/j3/2021-September/013322.html


This implies that
  if ( allocated (a[i]) .and. allocated (b%a[i]) ) stop 1
is valid.

However, I do note that coarray allocatables have to be collectively
(de)allocated, therefore
  allocated (a[i]) .and. allocated (b%a[i])
is equivalent to
  allocated (a) .and. allocated (b%a)
at least assuming that no image has failed.


First: Does this answer all the questions you had and resolved the
confusion?
Secondly, do you agree about the last bits of the analysis?
Thirdly, what do you think of the attached patch?

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





wtmoo.com

2021-09-22 Thread virat smith via Fortran
Hello,

Do you have interest in wtmoo.com? It will be on sale soon, you can leave
an offer via this email. Please note that no website comes with it, only
the name is on sale.

If you have any questions feel free to ask, we would be happy to hear from
you.
virat smith


Fortran: Improve file-reading error diagnostic [PR55534] (was: Re: [Patch] Fortran: Improve -Wmissing-include-dirs warnings [PR55534])

2021-09-22 Thread Tobias Burnus

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.

: 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  is the code in cpp.c's gfc_cpp_init,
which uses:
  cpp_change_file (cpp_in, LC_RENAME, _(""));

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
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 o