Re: [Patch, Fortran, F08] PR 85537: Invalid memory reference at runtime when calling subroutine through procedure pointer

[Janus Weil]

Am Mi., 27. März 2019 um 23:32 Uhr schrieb Thomas Koenig
:
>
> Am 27.03.19 um 22:54 schrieb Thomas Koenig:
> > I do not think this is correct.
>
> ... and I missed the point that this was specifically about
> initializations.
>
> So, OK from my side.  Thanks!

Great. Thanks for the reviews, everyone! Committed to trunk as r269980.

Cheers,
Janus

Re: [Patch, Fortran, F08] PR 85537: Invalid memory reference at runtime when calling subroutine through procedure pointer

[Janus Weil]

Hi Thomas,

> > the attached patch implements some missing constraints from Fortran
> > 2008 concerning procedure pointer initialization (cf. the standard
> > quote in comment #18), thus fixing two accepts-invalid and
> > ICE-on-invalid problems.
>
> I do not think this is correct.
>
> F2008:
>
> # 12.2.2.4 Procedure pointers
>
> #  A procedure pointer is a procedure that has the EXTERNAL and POINTER
> # attributes; it may be pointer associated with an external procedure, #
> # an internal procedure, [...]
>
> So a procedure pointer can be associated with an internal procedure.
>
> Comment#18 from the PR does not quote anything that says that
> a procedure pointer cannot be associated with an internal procedure.

Absolutely, a procedure pointer can in principle be associated with an
internal procedure, and my patch does not change that. What the patch
rejects is the ('static' in the C sense) pointer initialization of a
procedure pointer with an internal procedure, which is forbidden per
F2008:C1220.

procedure(..), pointer :: pp => internal_proc

A normal pointer assignment is still allowed per F2008:C729:

pp => internal_proc

Hope you agree (and sorry for not being more verbose in my explanation
in the first place).

Cheers,
Janus

Re: [Patch, Fortran, F08] PR 85537: Invalid memory reference at runtime when calling subroutine through procedure pointer

[Janus Weil]

Hi Steve,

> > the attached patch implements some missing constraints from Fortran
> > 2008 concerning procedure pointer initialization (cf. the standard
> > quote in comment #18), thus fixing two accepts-invalid and
> > ICE-on-invalid problems.
> >
> > It regtests cleanly on x86_64-linux-gnu. Ok for trunk?
>
> Looks OK to me.  Just minor comment.  If there are numbered
> constraints in either F2008 or F2018 for each of the if()
> conditionals, can you add a comment.

I fully agree. Luckily the reference to F08:C1220 is already present
in gfc_check_assign_symbol (but unfortunately not visible in the
diff), and it applies in the same way to the one IF clause that is
already there as well as the two new ones I'm adding, which is why I'm
not putting in an additional reference.

Cheers,
Janus

[Patch, Fortran, F08] PR 85537: Invalid memory reference at runtime when calling subroutine through procedure pointer

[Janus Weil]

Hi all,

the attached patch implements some missing constraints from Fortran
2008 concerning procedure pointer initialization (cf. the standard
quote in comment #18), thus fixing two accepts-invalid and
ICE-on-invalid problems.

It regtests cleanly on x86_64-linux-gnu. Ok for trunk?

Cheers,
Janus
diff --git a/gcc/fortran/ChangeLog b/gcc/fortran/ChangeLog
index e1fdb93f3d0..372c517487f 100644
--- a/gcc/fortran/ChangeLog
+++ b/gcc/fortran/ChangeLog
@@ -1,3 +1,9 @@
+2019-03-27  Janus Weil  
+
+	PR fortran/85537
+	* expr.c (gfc_check_assign_symbol): Reject internal and dummy procedures
+	in procedure pointer initialization.
+
 2019-03-27  Paul Thomas  
 
 	PR fortran/88247
diff --git a/gcc/fortran/expr.c b/gcc/fortran/expr.c
index f54affae18d..478a5557723 100644
--- a/gcc/fortran/expr.c
+++ b/gcc/fortran/expr.c
@@ -4407,6 +4407,20 @@ gfc_check_assign_symbol (gfc_symbol *sym, gfc_component *comp, gfc_expr *rvalue)
 		 "may not be a procedure pointer", >where);
 	  return false;
 	}
+  if (attr.proc == PROC_INTERNAL)
+	{
+	  gfc_error ("Internal procedure %qs is invalid in "
+		 "procedure pointer initialization at %L",
+		 rvalue->symtree->name, >where);
+	  return false;
+	}
+  if (attr.dummy)
+	{
+	  gfc_error ("Dummy procedure %qs is invalid in "
+		 "procedure pointer initialization at %L",
+		 rvalue->symtree->name, >where);
+	  return false;
+	}
 }
 
   return true;
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 0679cb72e52..f6df0b1281c 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,9 @@
+2019-03-27  Janus Weil  
+
+	PR fortran/85537
+	* gfortran.dg/dummy_procedure_11.f90: Fix test case.
+	* gfortran.dg/pointer_init_11.f90: New test case.
+
 2019-03-27  Richard Biener  
 
 	* gcc.dg/torture/20190327-1.c: New testcase.
diff --git a/gcc/testsuite/gfortran.dg/dummy_procedure_11.f90 b/gcc/testsuite/gfortran.dg/dummy_procedure_11.f90
index f51c5455c05..3e4b2b1d6f0 100644
--- a/gcc/testsuite/gfortran.dg/dummy_procedure_11.f90
+++ b/gcc/testsuite/gfortran.dg/dummy_procedure_11.f90
@@ -5,16 +5,18 @@
 ! Contributed by Vladimir Fuka 
 
 type :: t
-  procedure(g), pointer, nopass :: ppc => g
+  procedure(g), pointer, nopass :: ppc
 end type
 
-procedure(g), pointer :: pp => g
+procedure(g), pointer :: pp
 type(t)::x
 
 print *, f(g)
 print *, f(g())  ! { dg-error "Expected a procedure for argument" }
+pp => g
 print *, f(pp)
 print *, f(pp()) ! { dg-error "Expected a procedure for argument" }
+x%ppc => g
 print *, f(x%ppc)
 print *, f(x%ppc())  ! { dg-error "Expected a procedure for argument" }
 
diff --git a/gcc/testsuite/gfortran.dg/pointer_init_11.f90 b/gcc/testsuite/gfortran.dg/pointer_init_11.f90
new file mode 100644
index 000..3113e157687
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/pointer_init_11.f90
@@ -0,0 +1,44 @@
+! { dg-do compile }
+!
+! PR 85537: [F08] Invalid memory reference at runtime when calling subroutine through procedure pointer
+!
+! Contributed by Tiziano Müller 
+
+module m1
+implicit none
+contains
+subroutine foo()
+  integer :: a
+
+  abstract interface
+subroutine ibar()
+end subroutine
+  end interface
+
+  procedure(ibar), pointer :: bar_ptr => bar_impl  ! { dg-error "invalid in procedure pointer initialization" }
+
+contains
+  subroutine bar_impl()
+write (*,*) "foo"
+a = a + 1
+  end subroutine
+
+end subroutine
+end module
+
+
+module m2
+implicit none
+contains
+subroutine foo(dbar)
+  interface
+subroutine dbar()
+end subroutine
+  end interface
+
+  procedure(dbar), pointer :: bar_ptr => dbar  ! { dg-error "invalid in procedure pointer initialization" }
+
+  call bar_ptr()
+
+end subroutine
+end module

Re: [Patch, Fortran, F03] PR 71861: [7/8/9 Regression] ICE in write_symbol(): bad module symbol

[Janus Weil]

Am Mi., 20. März 2019 um 18:26 Uhr schrieb Thomas Koenig
:
>
> Hi Janus,
>
> > the attached one-line patch fixes an ICE-on-invalid regression with
> > abstract interfaces. Regtests cleanly on x86_64-linux-gnu. Ok for
> > trunk and the release branches (7 and 8)?
>
> OK for all.

Thanks, Thomas. Committed to trunk as r269827. Will do the backports
within a week or so.

Cheers,
Janus

[Patch, Fortran, F03] PR 71861: [7/8/9 Regression] ICE in write_symbol(): bad module symbol

[Janus Weil]

Hi all,

the attached one-line patch fixes an ICE-on-invalid regression with
abstract interfaces. Regtests cleanly on x86_64-linux-gnu. Ok for
trunk and the release branches (7 and 8)?

Cheers,
Janus
diff --git a/gcc/fortran/ChangeLog b/gcc/fortran/ChangeLog
index 4dd35ec6030..cee2b2cfd15 100644
--- a/gcc/fortran/ChangeLog
+++ b/gcc/fortran/ChangeLog
@@ -1,3 +1,9 @@
+2019-03-19  Janus Weil  
+
+	PR fortran/71861
+	* symbol.c (check_conflict): ABSTRACT attribute conflicts with
+	INTRINSIC attribute.
+
 2019-03-18  Thomas Koenig  
 
 	PR fortran/68009
diff --git a/gcc/fortran/symbol.c b/gcc/fortran/symbol.c
index c342d62ead1..ec753229a98 100644
--- a/gcc/fortran/symbol.c
+++ b/gcc/fortran/symbol.c
@@ -557,6 +557,7 @@ check_conflict (symbol_attribute *attr, const char *name, locus *where)
 
   conf (external, intrinsic);
   conf (entry, intrinsic);
+  conf (abstract, intrinsic);
 
   if ((attr->if_source == IFSRC_DECL && !attr->procedure) || attr->contained)
 conf (external, subroutine);
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index cf01b2fea19..4649e282f64 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2019-03-19  Janus Weil  
+
+	PR fortran/71861
+	* gfortran.dg/interface_abstract_5.f90: New test case.
+
 2019-03-19  Martin Sebor  
 
 	PR tree-optimization/89644
diff --git a/gcc/testsuite/gfortran.dg/interface_abstract_5.f90 b/gcc/testsuite/gfortran.dg/interface_abstract_5.f90
new file mode 100644
index 000..fddf6b89d27
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/interface_abstract_5.f90
@@ -0,0 +1,32 @@
+! { dg-do compile }
+!
+! PR 71861: [7/8/9 Regression] [F03] ICE in write_symbol(): bad module symbol
+!
+! Contributed by Gerhard Steinmetz 
+
+module m1
+   intrinsic abs
+   abstract interface
+  function abs(x)! { dg-error "ABSTRACT attribute conflicts with INTRINSIC attribute" }
+ real :: abs, x
+  end
+   end interface
+end
+
+module m2
+   abstract interface
+  function abs(x)
+ real :: abs, x
+  end
+   end interface
+   intrinsic abs! { dg-error "ABSTRACT attribute conflicts with INTRINSIC attribute" }
+end
+
+module m3
+   abstract interface
+  function f(x)
+ real :: f, x
+  end
+   end interface
+   intrinsic f! { dg-error "ABSTRACT attribute conflicts with INTRINSIC attribute" }
+end

Re: [Patch, Fortran] PR 89601: [8/9 Regression] [PDT] ICE: Segmentation fault (in resolve_component)

[Janus Weil]

I have just committed the updated patch to trunk as r269658. If anyone
thinks this should be backported to 8-branch, please let me know.

Cheers,
Janus


Am Di., 12. März 2019 um 13:00 Uhr schrieb Janus Weil :
>
> Hi Steve,
>
> > > Technically the ICE is a regression, but since it happens on invalid
> > > code only, backporting is not essential IMHO (but might still be
> > > useful). Ok for trunk? And 8-branch?
> >
> > Looks good to me with a minor suggestion.  See below.
>
> thanks for the comments. Updated patch attached.
>
>
> > You may want to wait a bit or ping Paul to give him
> > a chance to peek at the patch.
>
> I'm putting Paul in CC and will wait until tomorrow night before committing.
>
>
> > >if (gfc_match_char (')') == MATCH_YES)
> > > -goto ok;
> > > +  {
> > > +if (typeparam)
> > > +  {
> > > + gfc_error_now ("A parameter name is required at %C");
> >
> > Can you insert "type" so the error reads "A type parameter name"?
> > Unfortunately, the words parameter has a few too many meanings.
>
> True. I'm using 'type parameter list' now.
>
> Cheers,
> Janus

Re: [Patch, Fortran] PR 89601: [8/9 Regression] [PDT] ICE: Segmentation fault (in resolve_component)

[Janus Weil]

Hi Steve,

> > Technically the ICE is a regression, but since it happens on invalid
> > code only, backporting is not essential IMHO (but might still be
> > useful). Ok for trunk? And 8-branch?
>
> Looks good to me with a minor suggestion.  See below.

thanks for the comments. Updated patch attached.


> You may want to wait a bit or ping Paul to give him
> a chance to peek at the patch.

I'm putting Paul in CC and will wait until tomorrow night before committing.


> >if (gfc_match_char (')') == MATCH_YES)
> > -goto ok;
> > +  {
> > +if (typeparam)
> > +  {
> > + gfc_error_now ("A parameter name is required at %C");
>
> Can you insert "type" so the error reads "A type parameter name"?
> Unfortunately, the words parameter has a few too many meanings.

True. I'm using 'type parameter list' now.

Cheers,
Janus
diff --git a/gcc/fortran/ChangeLog b/gcc/fortran/ChangeLog
index a3b47ac4980..834c73fc27e 100644
--- a/gcc/fortran/ChangeLog
+++ b/gcc/fortran/ChangeLog
@@ -1,3 +1,10 @@
+2019-03-12  Janus Weil  
+
+	PR fortran/89601
+	* decl.c (gfc_match_formal_arglist): Reject empty type parameter lists.
+	(gfc_match_derived_decl): Mark as PDT only if type parameter list was
+	matched successfully.
+
 2019-03-11  Jakub Jelinek  
 
 	PR fortran/89651
diff --git a/gcc/fortran/decl.c b/gcc/fortran/decl.c
index a29e2db0bd6..3ebce880b39 100644
--- a/gcc/fortran/decl.c
+++ b/gcc/fortran/decl.c
@@ -6275,7 +6275,16 @@ gfc_match_formal_arglist (gfc_symbol *progname, int st_flag,
 }
 
   if (gfc_match_char (')') == MATCH_YES)
-goto ok;
+  {
+if (typeparam)
+  {
+	gfc_error_now ("A type parameter list is required at %C");
+	m = MATCH_ERROR;
+	goto cleanup;
+  }
+else
+  goto ok;
+  }
 
   for (;;)
 {
@@ -10217,13 +10226,14 @@ gfc_match_derived_decl (void)
   m = gfc_match_formal_arglist (sym, 0, 0, true);
   if (m != MATCH_YES)
 	gfc_error_recovery ();
+  else
+	sym->attr.pdt_template = 1;
   m = gfc_match_eos ();
   if (m != MATCH_YES)
 	{
 	  gfc_error_recovery ();
 	  gfc_error_now ("Garbage after PARAMETERIZED TYPE declaration at %C");
 	}
-  sym->attr.pdt_template = 1;
 }
 
   if (extended && !sym->components)
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 9b20bdfe787..8c8d282491b 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,9 @@
+2019-03-12  Janus Weil  
+
+	PR fortran/89601
+	* gfortran.dg/pdt_16.f03: Modified to avoid follow-up errors.
+	* gfortran.dg/pdt_30.f90: New test case.
+
 2019-03-12  Jakub Jelinek  
 
 	PR middle-end/89663
diff --git a/gcc/testsuite/gfortran.dg/pdt_16.f03 b/gcc/testsuite/gfortran.dg/pdt_16.f03
index 067d87d660d..22c6b83a084 100644
--- a/gcc/testsuite/gfortran.dg/pdt_16.f03
+++ b/gcc/testsuite/gfortran.dg/pdt_16.f03
@@ -12,7 +12,6 @@ end
 program p
type t(a  ! { dg-error "Expected parameter list" }
   integer, kind :: a
-  real(a) :: x
end type
type u(a, a)  ! { dg-error "Duplicate name" }
   integer, kind :: a ! { dg-error "already declared" }
diff --git a/gcc/testsuite/gfortran.dg/pdt_30.f90 b/gcc/testsuite/gfortran.dg/pdt_30.f90
new file mode 100644
index 000..47f7c775465
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/pdt_30.f90
@@ -0,0 +1,17 @@
+! { dg-do compile }
+!
+! PR 89601: [8/9 Regression] [PDT] ICE: Segmentation fault (in resolve_component)
+!
+! Contributed by Arseny Solokha 
+
+program vw
+  interface
+ real function ul (ki)
+   real :: ki
+ end function ul
+  end interface
+  type :: q8 ()  ! { dg-error "A type parameter list is required" }
+ procedure (ul), pointer, nopass :: pj
+  end type q8
+  type (q8) :: ki
+end program vw

[Patch, Fortran] PR 89601: [8/9 Regression] [PDT] ICE: Segmentation fault (in resolve_component)

[Janus Weil]

Hi all,

the attached patch fixes an ICE-on-invalid problem with parametrized
derived types, more precisely a PDT without any parameters, and
regtests cleanly x86_64-linux-gnu. For the relevant standard quote,
see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89601#c4.

Technically the ICE is a regression, but since it happens on invalid
code only, backporting is not essential IMHO (but might still be
useful). Ok for trunk? And 8-branch?

Cheers,
Janus
diff --git a/gcc/fortran/ChangeLog b/gcc/fortran/ChangeLog
index 1c738baedaa..5733bf2ac82 100644
--- a/gcc/fortran/ChangeLog
+++ b/gcc/fortran/ChangeLog
@@ -1,3 +1,10 @@
+2019-03-11  Janus Weil  
+
+	PR fortran/89601
+	* decl.c (gfc_match_formal_arglist): Reject empty type parameter lists.
+	(gfc_match_derived_decl): Mark as PDT only if type parameter list was
+	matched successfully.
+
 2019-03-11  Martin Liska  
 
 	* decl.c (match_record_decl): Wrap an option name
diff --git a/gcc/fortran/decl.c b/gcc/fortran/decl.c
index a29e2db0bd6..39ec413a953 100644
--- a/gcc/fortran/decl.c
+++ b/gcc/fortran/decl.c
@@ -6275,7 +6275,16 @@ gfc_match_formal_arglist (gfc_symbol *progname, int st_flag,
 }
 
   if (gfc_match_char (')') == MATCH_YES)
-goto ok;
+  {
+if (typeparam)
+  {
+	gfc_error_now ("A parameter name is required at %C");
+	m = MATCH_ERROR;
+	goto cleanup;
+  }
+else
+  goto ok;
+  }
 
   for (;;)
 {
@@ -10217,13 +10226,14 @@ gfc_match_derived_decl (void)
   m = gfc_match_formal_arglist (sym, 0, 0, true);
   if (m != MATCH_YES)
 	gfc_error_recovery ();
+  else
+	sym->attr.pdt_template = 1;
   m = gfc_match_eos ();
   if (m != MATCH_YES)
 	{
 	  gfc_error_recovery ();
 	  gfc_error_now ("Garbage after PARAMETERIZED TYPE declaration at %C");
 	}
-  sym->attr.pdt_template = 1;
 }
 
   if (extended && !sym->components)
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index aec924f27aa..a571002772c 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,9 @@
+2019-03-11  Janus Weil  
+
+	PR fortran/89601
+	* gfortran.dg/pdt_16.f03: Modified to avoid follow-up errors.
+	* gfortran.dg/pdt_30.f90: New test case.
+
 2019-03-11  Martin Liska  
 
 	* g++.dg/conversion/simd3.C (foo): Wrap option names
diff --git a/gcc/testsuite/gfortran.dg/pdt_16.f03 b/gcc/testsuite/gfortran.dg/pdt_16.f03
index 067d87d660d..22c6b83a084 100644
--- a/gcc/testsuite/gfortran.dg/pdt_16.f03
+++ b/gcc/testsuite/gfortran.dg/pdt_16.f03
@@ -12,7 +12,6 @@ end
 program p
type t(a  ! { dg-error "Expected parameter list" }
   integer, kind :: a
-  real(a) :: x
end type
type u(a, a)  ! { dg-error "Duplicate name" }
   integer, kind :: a ! { dg-error "already declared" }
diff --git a/gcc/testsuite/gfortran.dg/pdt_30.f90 b/gcc/testsuite/gfortran.dg/pdt_30.f90
new file mode 100644
index 000..d07b52f6fab
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/pdt_30.f90
@@ -0,0 +1,17 @@
+! { dg-do compile }
+!
+! PR 89601: [8/9 Regression] [PDT] ICE: Segmentation fault (in resolve_component)
+!
+! Contributed by Arseny Solokha 
+
+program vw
+  interface
+ real function ul (ki)
+   real :: ki
+ end function ul
+  end interface
+  type :: q8 ()  ! { dg-error "A parameter name is required" }
+ procedure (ul), pointer, nopass :: pj
+  end type q8
+  type (q8) :: ki
+end program vw

Re: [Patch, Fortran, F08] PR 84504: procedure pointer variables cannot be initialized with functions returning pointers

[Janus Weil]

Am Sa., 9. März 2019 um 18:46 Uhr schrieb Steve Kargl
:
>
> On Sat, Mar 09, 2019 at 04:23:01PM +0100, Janus Weil wrote:
> > Hi all,
> >
> > the attached patch is close to trivial and fixes a rejects-valid
> > problem concerning procedure pointers to pointer-valued functions. It
> > regtests cleanly on x86_64-linux-gnu. Ok for trunk?
> >
>
> Ok.

Thanks, Steve. Committed as r269529.

Cheers,
Janus

[Patch, Fortran, F08] PR 84504: procedure pointer variables cannot be initialized with functions returning pointers

[Janus Weil]

Hi all,

the attached patch is close to trivial and fixes a rejects-valid
problem concerning procedure pointers to pointer-valued functions. It
regtests cleanly on x86_64-linux-gnu. Ok for trunk?

Cheers,
Janus
diff --git a/gcc/fortran/ChangeLog b/gcc/fortran/ChangeLog
index 28403689bb1..e15ef5b6996 100644
--- a/gcc/fortran/ChangeLog
+++ b/gcc/fortran/ChangeLog
@@ -1,3 +1,9 @@
+2019-03-09  Janus Weil  
+
+	PR fortran/84504
+	* expr.c (gfc_check_assign_symbol): Deal with procedure pointers to
+	pointer-valued functions.
+
 2019-03-08  Jakub Jelinek  
 
 	PR other/80058
diff --git a/gcc/fortran/expr.c b/gcc/fortran/expr.c
index 51552a79cde..4e95f243661 100644
--- a/gcc/fortran/expr.c
+++ b/gcc/fortran/expr.c
@@ -4321,7 +4321,7 @@ gfc_check_assign_symbol (gfc_symbol *sym, gfc_component *comp, gfc_expr *rvalue)
   if (!r)
 return r;
 
-  if (pointer && rvalue->expr_type != EXPR_NULL)
+  if (pointer && rvalue->expr_type != EXPR_NULL && !proc_pointer)
 {
   /* F08:C461. Additional checks for pointer initialization.  */
   symbol_attribute attr;
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index f32b5afeed9..77d0f233f10 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2019-03-09  Janus Weil  
+
+	PR fortran/84504
+	* gfortran.dg/pointer_init_10.f90: New test case.
+
 2019-03-09  Jakub Jelinek  
 
 	PR rtl-optimization/89634
diff --git a/gcc/testsuite/gfortran.dg/pointer_init_10.f90 b/gcc/testsuite/gfortran.dg/pointer_init_10.f90
new file mode 100644
index 000..81e7d73755f
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/pointer_init_10.f90
@@ -0,0 +1,24 @@
+! { dg-do run }
+!
+! PR 84504: [F08] procedure pointer variables cannot be initialized with functions returning pointers
+!
+! Contributed by Sriram Swaminarayan 
+
+module test_mod
+  implicit none
+  private
+  integer, target :: i = 333
+  procedure(the_proc), pointer, public  :: ptr => the_proc
+contains
+  function the_proc() 
+integer, pointer :: the_proc
+the_proc => i
+  end function
+end module
+
+program test_prog
+  use test_mod
+  integer, pointer :: ip
+  ip => ptr()
+  if (ip /= 333) stop 1
+end

Re: [Patch, Fortran] PR 88047: [9 Regression] ICE in gfc_find_vtab, at fortran/class.c:2843

[Janus Weil]

Am Di., 8. Jan. 2019 um 16:28 Uhr schrieb Steve Kargl
:
>
> On Tue, Jan 08, 2019 at 10:11:33AM +0100, Janus Weil wrote:
> >
> > the attached patch is close to obvious and fixes another small
> > ICE-on-invalid regression. Since there was a bit of discussion in the
> > PR, I am submitting it for approval instead of just committing as
> > obvious.
> >
> > Regtests cleanly on x86_64-linux-gnu. Ok for trunk?
> >
>
> OK.

Thanks! Committed as r267735.

Cheers,
Janus

[Patch, Fortran] PR 88047: [9 Regression] ICE in gfc_find_vtab, at fortran/class.c:2843

[Janus Weil]

Hi all,

the attached patch is close to obvious and fixes another small
ICE-on-invalid regression. Since there was a bit of discussion in the
PR, I am submitting it for approval instead of just committing as
obvious.

Regtests cleanly on x86_64-linux-gnu. Ok for trunk?

Cheers,
Janus
diff --git a/gcc/fortran/ChangeLog b/gcc/fortran/ChangeLog
index ba95a26e6ae..4d50f880d38 100644
--- a/gcc/fortran/ChangeLog
+++ b/gcc/fortran/ChangeLog
@@ -1,3 +1,9 @@
+2019-01-08  Janus Weil  
+
+	PR fortran/88047
+	* class.c (gfc_find_vtab): For polymorphic typespecs, the components of
+	the class container may not be available (in case of invalid code).
+
 2019-01-07  Thomas Koenig  
 	Harald Anlauf 
 	Tobias Burnus 
diff --git a/gcc/fortran/class.c b/gcc/fortran/class.c
index 77f0fca9385..8809b5b5b6e 100644
--- a/gcc/fortran/class.c
+++ b/gcc/fortran/class.c
@@ -2846,7 +2846,10 @@ gfc_find_vtab (gfc_typespec *ts)
 case BT_DERIVED:
   return gfc_find_derived_vtab (ts->u.derived);
 case BT_CLASS:
-  return gfc_find_derived_vtab (ts->u.derived->components->ts.u.derived);
+  if (ts->u.derived->components && ts->u.derived->components->ts.u.derived)
+	return gfc_find_derived_vtab (ts->u.derived->components->ts.u.derived);
+  else
+	return NULL;
 default:
   return find_intrinsic_vtab (ts);
 }
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 23de2ea6f0b..45279ccae0c 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2019-01-08  Janus Weil  
+
+	PR fortran/88047
+	* gfortran.dg/class_69.f90: New test case.
+
 2019-01-07  David Malcolm  
 
 	PR jit/88747
diff --git a/gcc/testsuite/gfortran.dg/class_69.f90 b/gcc/testsuite/gfortran.dg/class_69.f90
new file mode 100644
index 000..e45e03528b7
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/class_69.f90
@@ -0,0 +1,21 @@
+! { dg-do compile }
+!
+! PR 88047: [9 Regression] ICE in gfc_find_vtab, at fortran/class.c:2843
+!
+! Contributed by G. Steinmetz 
+
+subroutine sub_a
+   type t
+   end type
+   class(t) :: x(2)   ! { dg-error "must be dummy, allocatable or pointer" }
+   class(t), parameter :: a(2) = t()  ! { dg-error "cannot have the PARAMETER attribute" }
+   x = a  ! { dg-error "Nonallocatable variable must not be polymorphic in intrinsic assignment" }
+end
+
+subroutine sub_b
+   type t
+  integer :: n
+   end type
+   class(t) :: a, x   ! { dg-error "must be dummy, allocatable or pointer" }
+   x = a  ! { dg-error "Nonallocatable variable must not be polymorphic in intrinsic assignment" }
+end

Re: [Patch, Fortran] PR 88009: [9 Regression] ICE in find_intrinsic_vtab, at fortran/class.c:2761

[Janus Weil]

Am Sa., 5. Jan. 2019 um 14:37 Uhr schrieb Thomas Koenig :
>
> Hi Janus,
>
> > the attached patch fixes PR 88009, an ICE-on-invalid regression caused
> > by one of my earlier commits. Apart from adding some extra checks to
> > avoid ICEs, it also uses the 'artificial' attribute to suppress bogus
> > errors (see comment #3) and does some minor cleanup in
> > resolve_fl_variable.
> >
> > Regtests cleanly on x86_64-linux-gnu. Ok for trunk?
>
>
> the patch looks good, OK for trunk.

thanks for the quick review, Thomas! Committed as r267598.

Cheers,
Janus

[Patch, Fortran] PR 88009: [9 Regression] ICE in find_intrinsic_vtab, at fortran/class.c:2761

[Janus Weil]

Hi all,

the attached patch fixes PR 88009, an ICE-on-invalid regression caused
by one of my earlier commits. Apart from adding some extra checks to
avoid ICEs, it also uses the 'artificial' attribute to suppress bogus
errors (see comment #3) and does some minor cleanup in
resolve_fl_variable.

Regtests cleanly on x86_64-linux-gnu. Ok for trunk?

Cheers,
Janus


2019-01-05  Janus Weil  

PR fortran/88009
* class.c (gfc_find_derived_vtab): Mark the _final component as
artificial.
(find_intrinsic_vtab): Ditto. Also add an extra check to avoid
dereferencing a null pointer and adjust indentation.
* resolve.c (resolve_fl_variable): Add extra check to avoid
dereferencing a null pointer. Move variable declarations to local scope.
(resolve_fl_procedure): Add extra check to avoid dereferencing a null
pointer.
* symbol.c (check_conflict): Suppress errors for artificial symbols.


2019-01-05  Janus Weil  

PR fortran/88009
* gfortran.dg/blockdata_10.f90: New test case.
diff --git a/gcc/fortran/class.c b/gcc/fortran/class.c
index e55ab25882f..77f0fca9385 100644
--- a/gcc/fortran/class.c
+++ b/gcc/fortran/class.c
@@ -2466,6 +2466,7 @@ gfc_find_derived_vtab (gfc_symbol *derived)
 		goto cleanup;
 	  c->attr.proc_pointer = 1;
 	  c->attr.access = ACCESS_PRIVATE;
+	  c->attr.artificial = 1;
 	  c->tb = XCNEW (gfc_typebound_proc);
 	  c->tb->ppc = 1;
 	  generate_finalization_wrapper (derived, ns, tname, c);
@@ -2762,9 +2763,9 @@ find_intrinsic_vtab (gfc_typespec *ts)
 	  /* This is elemental so that arrays are automatically
 		 treated correctly by the scalarizer.  */
 	  copy->attr.elemental = 1;
-	  if (ns->proc_name->attr.flavor == FL_MODULE)
+	  if (ns->proc_name && ns->proc_name->attr.flavor == FL_MODULE)
 		copy->module = ns->proc_name->name;
-		  gfc_set_sym_referenced (copy);
+	  gfc_set_sym_referenced (copy);
 	  /* Set up formal arguments.  */
 	  gfc_get_symbol ("src", sub_ns, );
 	  src->ts.type = ts->type;
@@ -2798,6 +2799,7 @@ find_intrinsic_vtab (gfc_typespec *ts)
 		goto cleanup;
 	  c->attr.proc_pointer = 1;
 	  c->attr.access = ACCESS_PRIVATE;
+	  c->attr.artificial = 1;
 	  c->tb = XCNEW (gfc_typebound_proc);
 	  c->tb->ppc = 1;
 	  c->initializer = gfc_get_null_expr (NULL);
diff --git a/gcc/fortran/resolve.c b/gcc/fortran/resolve.c
index a498d19e411..beafe8da8bc 100644
--- a/gcc/fortran/resolve.c
+++ b/gcc/fortran/resolve.c
@@ -12274,13 +12274,8 @@ deferred_requirements (gfc_symbol *sym)
 static bool
 resolve_fl_variable (gfc_symbol *sym, int mp_flag)
 {
-  int no_init_flag, automatic_flag;
-  gfc_expr *e;
-  const char *auto_save_msg;
-  bool saved_specification_expr;
-
-  auto_save_msg = "Automatic object %qs at %L cannot have the "
-		  "SAVE attribute";
+  const char *auto_save_msg = "Automatic object %qs at %L cannot have the "
+			  "SAVE attribute";
 
   if (!resolve_fl_var_and_proc (sym, mp_flag))
 return false;
@@ -12288,7 +12283,7 @@ resolve_fl_variable (gfc_symbol *sym, int mp_flag)
   /* Set this flag to check that variables are parameters of all entries.
  This check is effected by the call to gfc_resolve_expr through
  is_non_constant_shape_array.  */
-  saved_specification_expr = specification_expr;
+  bool saved_specification_expr = specification_expr;
   specification_expr = true;
 
   if (sym->ns->proc_name
@@ -12315,6 +12310,8 @@ resolve_fl_variable (gfc_symbol *sym, int mp_flag)
 {
   /* Make sure that character string variables with assumed length are
 	 dummy arguments.  */
+  gfc_expr *e = NULL;
+
   if (sym->ts.u.cl)
 	e = sym->ts.u.cl->length;
   else
@@ -12364,7 +12361,7 @@ resolve_fl_variable (gfc_symbol *sym, int mp_flag)
 apply_default_init_local (sym); /* Try to apply a default initialization.  */
 
   /* Determine if the symbol may not have an initializer.  */
-  no_init_flag = automatic_flag = 0;
+  int no_init_flag = 0, automatic_flag = 0;
   if (sym->attr.allocatable || sym->attr.external || sym->attr.dummy
   || sym->attr.intrinsic || sym->attr.result)
 no_init_flag = 1;
@@ -12494,7 +12491,7 @@ resolve_fl_procedure (gfc_symbol *sym, int mp_flag)
  module procedures are excluded by 2.2.3.3 - i.e., they are not
  externally accessible and can access all the objects accessible in
  the host.  */
-  if (!(sym->ns->parent
+  if (!(sym->ns->parent && sym->ns->parent->proc_name
 	&& sym->ns->parent->proc_name->attr.flavor == FL_MODULE)
   && gfc_check_symbol_access (sym))
 {
diff --git a/gcc/fortran/symbol.c b/gcc/fortran/symbol.c
index a88e7c01df7..cd52c73031b 100644
--- a/gcc/fortran/symbol.c
+++ b/gcc/fortran/symbol.c
@@ -440,6 +440,9 @@ check_conflict (symbol

Re: [patch, fortran, committed] Another fallout from the INTENT(OUT) patch

[Janus Weil]

Am Mo., 24. Sep. 2018 um 19:16 Uhr schrieb Thomas Koenig
:
>
> Hello world,
>
> another obvious and simple one-line fix for fallout from the INTENT(OUT)
> clobber patch.  Committed as r264539, after regression-testing.
>
> It seems our testsuite is not testing as many combinations in the
> language as I thought :-)

... but we keep on improving every day.

Thanks for the quick fix, Thomas!

Cheers,
Janus

Re: [patch, fortran, committed] Another fallout from clobbering INTENT(OUT) variables

[Janus Weil]

Hi Thomas,

I'm seeing some more fallout from your intent(out) patch:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87401

Cheers,
Janus

Am So., 23. Sep. 2018 um 22:23 Uhr schrieb Thomas König :
>
> Hello world,
>
> committed as obvious after regression-testing.  Instead of spending
> a lot of work trying to reducing the test case, I used all of it
> (retainging the copyright notice).
>
> Regards
>
> Thomas
>
> 2018-09-23  Thomas Koenig  
>
> PR fortran/87397
> * gfc_conv_procedure_call: Do not add clobber on INTENT(OUT)
> for variables having the dimension attribute.
>
> 2018-09-23  Thomas Koenig  
>
> PR fortran/87397
> * gfortran.dg/intent_out_11.f90: New test.
>

Re: [Patch, Fortran, OOP] PR 46313: OOP-ABI issue, ALLOCATE issue, CLASS renaming issue

[Janus Weil]

Am Mi., 19. Sep. 2018 um 16:50 Uhr schrieb Bernhard Reutner-Fischer
:
>
> On Mon, 17 Sep 2018 at 22:25, Janus Weil  wrote:
>
> > The regtest was successful. I don't think the off-by-two error for the
> > vtab/vtype comparisons is a big problem in practice, since the number
> > of internal symbols with leading underscores is very limited, but of
> > course it should still be fixed ...
>
> Luckily it should make no difference indeed as "__vta" and "__vtyp"
> are only used for this one purpose.
> I don't think the DTIO op keyword fix would hit any real user either.
> Thanks for taking care of it, patch LGTM.

I have now committed this as r264448.

Cheers,
Janus

Re: [Patch, Fortran, OOP] PR 46313: OOP-ABI issue, ALLOCATE issue, CLASS renaming issue

[Janus Weil]

Am Mo., 17. Sep. 2018 um 21:21 Uhr schrieb Janus Weil :
>
> Instead, for the sake of gfortran, how about a macro like this?
>
> #define gfc_str_startswith(str, pref) \
> (strncmp ((str), (pref), strlen (pref)) == 0)
>
> (In fact I just noticed that something like this already exists in
> trans-intrinsic.c, so I would just move it into gfortran.h and rename
> it.)
>
>
> > Looking at gcc/fortran alone there are
> > gcc/fortran/interface.c:  if (strncmp (mode, "unformatted", 9) == 0) // 11 !
>
> I think this one could actually be a 'strcmp'?
>
>
> > gcc/fortran/module.c: && (strncmp (name, "__vtab_", 5) == 0 // 7 !
> > gcc/fortran/module.c: || strncmp (name, "__vtype_", 6) == 0)) 
> > // 8!
> > gcc/fortran/module.c: || (strncmp (name, "__vtab_", 5) != 0 // 
> > 7!
> > gcc/fortran/module.c: && strncmp (name, "__vtype_", 6)
> > != 0)) // 8!
>
> Here the new macro could be applied (and in a few other cases as
> well), see attached patch.
>
> I'm regtesting the patch now. Ok for trunk if it passes?

The regtest was successful. I don't think the off-by-two error for the
vtab/vtype comparisons is a big problem in practice, since the number
of internal symbols with leading underscores is very limited, but of
course it should still be fixed ...

Cheers,
Janus

Re: [Patch, Fortran, OOP] PR 46313: OOP-ABI issue, ALLOCATE issue, CLASS renaming issue

[Janus Weil]

Am Mo., 17. Sep. 2018 um 10:59 Uhr schrieb Bernhard Reutner-Fischer
:
>
> On Tue, 9 Nov 2010 at 11:41, Janus Weil  wrote:
> >
> > >> Ok, so it seems to me that using two leading underscores is really the
> > >> best option, since it's safe against collisions with Fortran and C
> > >> user code, and also safe to use with -fdollar-ok.
> > >>
> > >> The attached patch adds double underscores for the vtabs, vtypes,
> > >> class containers and temporaries.
> > >
> > > OK. Thanks for the patch!
> >
> > Committed as r166480.
> >
> > Thanks for all the helpful comments, everyone!
>
> Index: gcc/fortran/module.c
> ===
> --- gcc/fortran/module.c (revision 166419)
> +++ gcc/fortran/module.c (working copy)
> @@ -4372,8 +4372,8 @@ read_module (void)
>  p = name;
>
>/* Exception: Always import vtabs & vtypes.  */
> -  if (p == NULL && (strncmp (name, "vtab$", 5) == 0
> -|| strncmp (name, "vtype$", 6) == 0))
> +  if (p == NULL && (strncmp (name, "__vtab_", 5) == 0
> +|| strncmp (name, "__vtype_", 6) == 0))
>  p = name;
>
>/* Skip symtree nodes not in an ONLY clause, unless there
>
> ---8<---
>
> Sorry for the late follow-up

'Late' is a pretty bold understatement for 8 years ;D

But in any case, 'late' is certainly better than 'never' ...


> but current trunk still has the code
> quoted above where we forgot to add 2 to the length parameter of both
> strncmp calls.

True! Thanks for noticing. I'll take care of fixing it.


> I think it would be nice to teach the C and C++ frontends to warn
> about this even though it might trigger in quite some code in the
> wild.

I don't really think this is a good idea. There are actually valid use
cases of strncmp, where the 'num' argument does not correspond to the
length of any of the two strings (in particular if they're not const).

Instead, for the sake of gfortran, how about a macro like this?

#define gfc_str_startswith(str, pref) \
(strncmp ((str), (pref), strlen (pref)) == 0)

(In fact I just noticed that something like this already exists in
trans-intrinsic.c, so I would just move it into gfortran.h and rename
it.)


> Looking at gcc/fortran alone there are
> gcc/fortran/interface.c:  if (strncmp (mode, "unformatted", 9) == 0) // 11 !

I think this one could actually be a 'strcmp'?


> gcc/fortran/module.c: && (strncmp (name, "__vtab_", 5) == 0 // 7 !
> gcc/fortran/module.c: || strncmp (name, "__vtype_", 6) == 0)) // 
> 8!
> gcc/fortran/module.c: || (strncmp (name, "__vtab_", 5) != 0 // 7!
> gcc/fortran/module.c: && strncmp (name, "__vtype_", 6)
> != 0)) // 8!

Here the new macro could be applied (and in a few other cases as
well), see attached patch.

I'm regtesting the patch now. Ok for trunk if it passes?

Cheers,
Janus
diff --git a/gcc/fortran/decl.c b/gcc/fortran/decl.c
index 3d19ad479e5..91a1f34d7f1 100644
--- a/gcc/fortran/decl.c
+++ b/gcc/fortran/decl.c
@@ -2529,7 +2529,7 @@ variable_decl (int elem)
 }
 
   /* %FILL components may not have initializers.  */
-  if (strncmp (name, "%FILL", 5) == 0 && gfc_match_eos () != MATCH_YES)
+  if (gfc_str_startswith (name, "%FILL") && gfc_match_eos () != MATCH_YES)
 {
   gfc_error ("%qs entity cannot have an initializer at %C", "%FILL");
   m = MATCH_ERROR;
@@ -7811,7 +7811,7 @@ gfc_match_end (gfc_statement *st)
 {
 case COMP_ASSOCIATE:
 case COMP_BLOCK:
-  if (!strncmp (block_name, "block@", strlen("block@")))
+  if (gfc_str_startswith (block_name, "block@"))
 	block_name = NULL;
   break;
 
diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h
index 04b0024a992..8f37a51d71c 100644
--- a/gcc/fortran/gfortran.h
+++ b/gcc/fortran/gfortran.h
@@ -3305,6 +3305,9 @@ bool gfc_is_compile_time_shape (gfc_array_spec *);
 bool gfc_ref_dimen_size (gfc_array_ref *, int dimen, mpz_t *, mpz_t *);
 
 
+#define gfc_str_startswith(str, pref) \
+	(strncmp ((str), (pref), strlen (pref)) == 0)
+
 /* interface.c -- FIXME: some of these should be in symbol.c */
 void gfc_free_interface (gfc_interface *);
 bool gfc_compare_derived_types (gfc_symbol *, gfc_symbol *);
diff --git a/gcc/fortran/interface.c b/gcc/fortran/interface.c
index f85c76bad0f..ff6b2bb7ece 100644
--- a/gcc/fortran/interface.c
+++ b/gcc/fortran/interface.c
@@ -122,9 +122,9 @@ fold_unary_intrinsic (gfc_intrinsic_op op)
 static gfc_intrinsic_op
 dtio_op (char* mode)
 {
-  if (strncmp (mode, "formatted", 9) == 0)
+  if (strcmp (mode, "formatted&qu

[Patch, Fortran, committed] PR 86484 & 84543

[Janus Weil]

Hi all,

I have just committed as obvious a small patch that fixes two very
related PRs concerning polymorphic assignments, by making sure that
the relevant vtabs (including copy procedures) are generated:

https://gcc.gnu.org/viewcvs/gcc?view=revision=264350

Cheers,
Janus

Re: [Patch, Fortran] PR 87172: [9 Regression] Spurious "Derived type 'c_funptr' at (1) has not been declared" error after r263782

[Janus Weil]

ping!
Am Mo., 3. Sep. 2018 um 21:19 Uhr schrieb Janus Weil :
>
> Hi all,
>
> attached is a simple patch that fixes a regression which was
> introduced by one of my recent commits (spotted by Dominique).
>
> My first impulse to fix the spurious error was to check for the
> 'intrinsic' attribute, however it is actually not set (yet?) in
> resolve_fl_derived, so the next-best thing is to check for
> 'use_assoc'. That is probably even better, since it's more general.
> Obviously, if a type has the 'use_assoc' attribute, then it has been
> declared in another module, so there is no need to complain that it
> was not declared.
>
> The patch regtests cleanly on x86_64-linux-gnu. Ok for trunk?
>
> Cheers,
> Janus

Re: [Patch, Fortran] PR 86830: [8/9 Regression] Contiguous array pointer function result not recognized as contiguous

[Janus Weil]

Am Mo., 10. Sep. 2018 um 08:54 Uhr schrieb Paul Richard Thomas
:
>
> Hi Janus,
>
> That's fine - OK for both branches.

Committed to trunk as r264201. Will do 8-branch soon.

Thanks for the review,
Janus


> On 9 September 2018 at 21:34, Janus Weil  wrote:
> > Hi all,
> >
> > the attached patch fixes a rejects-valid regression, where a
> > type-bound procedure call was not correctly detected to have a
> > contiguous result. The patch is functionally identical with comment #2
> > in the PR, with a little bit of cleanup on top of it. It regtests
> > cleanly on x86_64-linux-gnu. Ok for trunk and gcc-8-branch?
> >
> > Cheers,
> > Janus
>
>
>
> --
> "If you can't explain it simply, you don't understand it well enough"
> - Albert Einstein

Re: [Patch, Fortran, F03] PR 85395: private clause contained in derived type acquires spurious scope

[Janus Weil]

Am Mo., 10. Sep. 2018 um 08:55 Uhr schrieb Paul Richard Thomas
:
>
> Hi Janus,
>
> That's OK for trunk and, I would suggest 8-branch.

Thanks, Paul. Committed to trunk as r264196. Will backport to 8-branch
in a week or so.

Cheers,
Janus



> On 9 September 2018 at 17:31, Janus Weil  wrote:
> > Hi all,
> >
> > the attached patch fixes a problem with the accessibility of procedure
> > pointer components (private/public attribute). It is rather
> > straightforward and regtest cleanly on x86_64-linux-gnu (for further
> > details see the PR). Ok for trunk?
> >
> > Cheers,
> > Janus
>
>
>
> --
> "If you can't explain it simply, you don't understand it well enough"
> - Albert Einstein

[Patch, Fortran] PR 86830: [8/9 Regression] Contiguous array pointer function result not recognized as contiguous

[Janus Weil]

Hi all,

the attached patch fixes a rejects-valid regression, where a
type-bound procedure call was not correctly detected to have a
contiguous result. The patch is functionally identical with comment #2
in the PR, with a little bit of cleanup on top of it. It regtests
cleanly on x86_64-linux-gnu. Ok for trunk and gcc-8-branch?

Cheers,
Janus
diff --git a/gcc/fortran/ChangeLog b/gcc/fortran/ChangeLog
index 7cfb94ee115..7e2d6445237 100644
--- a/gcc/fortran/ChangeLog
+++ b/gcc/fortran/ChangeLog
@@ -1,3 +1,9 @@
+2018-09-09  Janus Weil  
+
+	PR fortran/86830
+	* expr.c (gfc_is_simply_contiguous): Handle type-bound procedure calls
+	with non-polymorphic objects.
+
 2018-09-03  Jerry DeLisle  
 
 	* simplify.c (gfc_simplify_modulo): Re-arrange code to test whether
diff --git a/gcc/fortran/expr.c b/gcc/fortran/expr.c
index c5bf822cd24..97792fe32a7 100644
--- a/gcc/fortran/expr.c
+++ b/gcc/fortran/expr.c
@@ -5385,16 +5385,13 @@ gfc_is_simply_contiguous (gfc_expr *expr, bool strict, bool permit_element)
 	return expr->value.function.esym->result->attr.contiguous;
   else
 	{
-	  /* We have to jump through some hoops if this is a vtab entry.  */
-	  gfc_symbol *s;
-	  gfc_ref *r, *rc;
-
-	  s = expr->symtree->n.sym;
-	  if (s->ts.type != BT_CLASS)
+	  /* Type-bound procedures.  */
+	  gfc_symbol *s = expr->symtree->n.sym;
+	  if (s->ts.type != BT_CLASS && s->ts.type != BT_DERIVED)
 	return false;
 
-	  rc = NULL;
-	  for (r = expr->ref; r; r = r->next)
+	  gfc_ref *rc = NULL;
+	  for (gfc_ref *r = expr->ref; r; r = r->next)
 	if (r->type == REF_COMPONENT)
 	  rc = r;
 
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 0c038441a8c..9e1ab44144f 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2018-09-09  Janus Weil  
+
+	PR fortran/86830
+	* gfortran.dg/typebound_call_30.f90: New test case.
+
 2018-09-08  Marek Polacek  
 
 	PR c++/87150 - wrong ctor with maybe-rvalue semantics.
diff --git a/gcc/testsuite/gfortran.dg/typebound_call_30.f90 b/gcc/testsuite/gfortran.dg/typebound_call_30.f90
new file mode 100644
index 000..3ca63bd2a95
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/typebound_call_30.f90
@@ -0,0 +1,32 @@
+! { dg-do compile }
+!
+! PR 86830: [8/9 Regression] Contiguous array pointer function result not recognized as contiguous
+!
+! Contributed by 
+
+module m
+  implicit none
+
+  type :: t1
+   contains
+ procedure :: get_ptr
+  end type
+
+  type :: t2
+ class(t1), allocatable :: c
+  end type
+
+contains
+
+  function get_ptr(this)
+class(t1) :: this
+real, dimension(:), contiguous, pointer :: get_ptr
+  end function
+
+  subroutine test()
+real, dimension(:), contiguous, pointer:: ptr
+type(t2) :: x
+ptr => x%c%get_ptr()
+  end subroutine
+
+end module

[Patch, Fortran, F03] PR 85395: private clause contained in derived type acquires spurious scope

[Janus Weil]

Hi all,

the attached patch fixes a problem with the accessibility of procedure
pointer components (private/public attribute). It is rather
straightforward and regtest cleanly on x86_64-linux-gnu (for further
details see the PR). Ok for trunk?

Cheers,
Janus
diff --git a/gcc/fortran/ChangeLog b/gcc/fortran/ChangeLog
index 7cfb94ee115..11adc5d13d5 100644
--- a/gcc/fortran/ChangeLog
+++ b/gcc/fortran/ChangeLog
@@ -1,3 +1,9 @@
+2018-09-09  Janus Weil  
+
+	PR fortran/85395
+	* decl.c (match_binding_attributes): Use correct default accessibility
+	for procedure pointer components.
+
 2018-09-03  Jerry DeLisle  
 
 	* simplify.c (gfc_simplify_modulo): Re-arrange code to test whether
diff --git a/gcc/fortran/decl.c b/gcc/fortran/decl.c
index 03298833c98..3d19ad479e5 100644
--- a/gcc/fortran/decl.c
+++ b/gcc/fortran/decl.c
@@ -10570,7 +10570,8 @@ match_binding_attributes (gfc_typebound_proc* ba, bool generic, bool ppc)
 
 done:
   if (ba->access == ACCESS_UNKNOWN)
-ba->access = gfc_typebound_default_access;
+ba->access = ppc ? gfc_current_block()->component_access
+ : gfc_typebound_default_access;
 
   if (ppc && !seen_ptr)
 {
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 0c038441a8c..118c03f17f7 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2018-09-09  Janus Weil  
+
+	PR fortran/85395
+	* gfortran.dg/proc_ptr_comp_52.f90: New test case.
+
 2018-09-08  Marek Polacek  
 
 	PR c++/87150 - wrong ctor with maybe-rvalue semantics.
diff --git a/gcc/testsuite/gfortran.dg/proc_ptr_comp_52.f90 b/gcc/testsuite/gfortran.dg/proc_ptr_comp_52.f90
new file mode 100644
index 000..631c0180753
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/proc_ptr_comp_52.f90
@@ -0,0 +1,33 @@
+! { dg-do compile }
+!
+! PR 85395: [F03] private clause contained in derived type acquires spurious scope
+!
+! Contributed by 
+
+module defs
+   implicit none
+
+   type :: base
+   contains
+  private
+   end type
+
+   type :: options
+  procedure(), pointer, nopass :: ptr
+   end type
+
+   type :: t
+  private
+  procedure(), pointer, nopass, public :: ptr
+   end type
+end module
+
+
+program p
+   use defs
+   implicit none
+   type(options) :: self
+   type(t) :: dt
+   self%ptr => null()
+   dt%ptr => null()
+end

[Patch, Fortran] PR 87172: [9 Regression] Spurious "Derived type 'c_funptr' at (1) has not been declared" error after r263782

[Janus Weil]

Hi all,

attached is a simple patch that fixes a regression which was
introduced by one of my recent commits (spotted by Dominique).

My first impulse to fix the spurious error was to check for the
'intrinsic' attribute, however it is actually not set (yet?) in
resolve_fl_derived, so the next-best thing is to check for
'use_assoc'. That is probably even better, since it's more general.
Obviously, if a type has the 'use_assoc' attribute, then it has been
declared in another module, so there is no need to complain that it
was not declared.

The patch regtests cleanly on x86_64-linux-gnu. Ok for trunk?

Cheers,
Janus
diff --git a/gcc/fortran/ChangeLog b/gcc/fortran/ChangeLog
index c386a649583..8dad9d35eaf 100644
--- a/gcc/fortran/ChangeLog
+++ b/gcc/fortran/ChangeLog
@@ -1,3 +1,10 @@
+2018-09-03  Janus Weil  
+
+	PR fortran/87172
+	* resolve.c (resolve_fl_derived): If a type has the 'use_assoc'
+	attribute, then it was declared in another module, so there should be
+	no error that it has not been declared.
+
 2018-08-31  Paul Thomas  
 
 	PR fortran/86328
diff --git a/gcc/fortran/resolve.c b/gcc/fortran/resolve.c
index ded27624283..ea0ce800743 100644
--- a/gcc/fortran/resolve.c
+++ b/gcc/fortran/resolve.c
@@ -14245,7 +14245,7 @@ resolve_fl_derived (gfc_symbol *sym)
 			  >declared_at))
 return false;
 
-  if (sym->components == NULL && !sym->attr.zero_comp)
+  if (sym->components == NULL && !sym->attr.zero_comp && !sym->attr.use_assoc)
 {
   gfc_error ("Derived type %qs at %L has not been declared",
 		  sym->name, >declared_at);
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index b44c714d7d2..da267de64ef 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2018-09-03  Janus Weil  
+
+	PR fortran/87172
+	* gfortran.dg/iso_c_binding_only_2.f90: New test case.
+
 2018-09-03  Richard Biener  
 
 	PR tree-optimization/87200
diff --git a/gcc/testsuite/gfortran.dg/iso_c_binding_only_2.f90 b/gcc/testsuite/gfortran.dg/iso_c_binding_only_2.f90
new file mode 100644
index 000..03fafe3f5cb
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/iso_c_binding_only_2.f90
@@ -0,0 +1,14 @@
+! { dg-do compile }
+!
+! PR 87172: [9 Regression] Spurious "Derived type 'c_funptr' at (1) has not been declared" error after r263782
+!
+! Contributed by Dominique d'Humieres 
+
+module m1
+   use iso_c_binding, only: c_funptr
+end module
+
+module m2
+  use m1
+  use iso_c_binding
+end module

Re: [Patch, fortran] PR86328 - [8/9 Regression] Runtime segfault reading an allocatable class(*) object in allocate statements

[Janus Weil]

Hi Paul,

> The attached patch fixes PR86328 and PR86760. The regression was
> caused by my commit r252949.
>
> The parts of the patch that fix the PRs are in trans.c and
> trans-array.c. The problem was caused by fixing the expressions that
> would provide the 'span' in gfc_build_array_ref, since the latter
> expected a variable expression. A number of evaluations of component
> array elements were producing pre blocks that were not added and so
> the temporaries were not being evaluated.
>
> The fix is to pass the COMPONENT_REF and extract the 'span' directly from it.
>
> The rest of the patch arises from PR86328 comment #12. In fact, this
> took most of the time that I have spent on these PRs :-(  Having done
> this, I felt that I had to include this part of the patch in the
> submission. However, I have found a host of related bugs, which I will
> put together in one PR.
>
> My inclination is to commit the patch without the parts in resolve.c,
> trans-expr.c and pr86328_12.f90, especially for 8-branch. I am open to
> suggestions for 9-branch.
>
> Bootstraps and regtests on FC28/x68_64 - OK for 8- and 9-branches?

the patch is ok for trunk from my side. I also agree that it makes
sense to backport those parts that address the regression to 8-branch.
Thanks for the fix!

Cheers,
Janus

[Patch, Fortran, committed] PR 86545: ICE in transfer_expr on invalid WRITE statement

[Janus Weil]

Hi all,

I have just committed a small patch for an ICE-on-invalid problem:

https://gcc.gnu.org/viewcvs/gcc?view=revision=263854

The patch was approved by Jerry on bugzilla and I checked that it
shows no failures in the testsuite.

Cheers,
Janus

Re: [Patch, Fortran, F08] PR 86888: allocatable components of indirectly recursive type

[Janus Weil]

Am Mi., 22. Aug. 2018 um 15:44 Uhr schrieb Paul Richard Thomas
:
>
> > the attached patch fixes the PR in the subject line in a rather
> > straightforward fashion. Pointer components of indirectly recursive
> > type are working already, as well as allocatable components of
> > directly recursive type. It seems this case was simply forgotten.
>
> That is correct. I was aware that it had been forgotten and is
> somewhere far, far down on my TODO list. Thank you for dealing with
> it.

Thanks, Paul. Committed as r263782.

Cheers,
Janus

[Patch, Fortran, F08] PR 86888: allocatable components of indirectly recursive type

[Janus Weil]

Hi all,

the attached patch fixes the PR in the subject line in a rather
straightforward fashion. Pointer components of indirectly recursive
type are working already, as well as allocatable components of
directly recursive type. It seems this case was simply forgotten.

The patch regtests cleanly on x86_64-linux-gnu. Ok for trunk?

Cheers,
Janus


https://github.com/janusw/gcc/commit/6f5a1b637e562b86d06d9a0d852c18ecb219c5ec
diff --git a/gcc/fortran/ChangeLog b/gcc/fortran/ChangeLog
index dc4aa1acf74..03e8b137e8f 100644
--- a/gcc/fortran/ChangeLog
+++ b/gcc/fortran/ChangeLog
@@ -1,3 +1,11 @@
+2018-08-21  Janus Weil  
+
+	PR fortran/86888
+	* decl.c (gfc_match_data_decl): Allow allocatable components of
+	indirectly recursive type.
+	* resolve.c (resolve_component): Remove two errors messages ...
+	(resolve_fl_derived): ... and replace them by a new one.
+
 2018-08-16  Nathan Sidwell  
 
 	* cpp.c (dump_macro): Use cpp_user_macro_p.
diff --git a/gcc/fortran/decl.c b/gcc/fortran/decl.c
index 1384bc717d8..03298833c98 100644
--- a/gcc/fortran/decl.c
+++ b/gcc/fortran/decl.c
@@ -5864,8 +5864,7 @@ gfc_match_data_decl (void)
   if (current_attr.pointer && gfc_comp_struct (gfc_current_state ()))
 	goto ok;
 
-  if (current_attr.allocatable && gfc_current_state () == COMP_DERIVED
-	  && current_ts.u.derived == gfc_current_block ())
+  if (current_attr.allocatable && gfc_current_state () == COMP_DERIVED)
 	goto ok;
 
   gfc_find_symbol (current_ts.u.derived->name,
diff --git a/gcc/fortran/resolve.c b/gcc/fortran/resolve.c
index d65118dfae3..4ad4dcf780d 100644
--- a/gcc/fortran/resolve.c
+++ b/gcc/fortran/resolve.c
@@ -14001,28 +14001,6 @@ resolve_component (gfc_component *c, gfc_symbol *sym)
 CLASS_DATA (c)->ts.u.derived
 = gfc_find_dt_in_generic (CLASS_DATA (c)->ts.u.derived);
 
-  if (!sym->attr.is_class && c->ts.type == BT_DERIVED && !sym->attr.vtype
-  && c->attr.pointer && c->ts.u.derived->components == NULL
-  && !c->ts.u.derived->attr.zero_comp)
-{
-  gfc_error ("The pointer component %qs of %qs at %L is a type "
- "that has not been declared", c->name, sym->name,
- >loc);
-  return false;
-}
-
-  if (c->ts.type == BT_CLASS && c->attr.class_ok
-  && CLASS_DATA (c)->attr.class_pointer
-  && CLASS_DATA (c)->ts.u.derived->components == NULL
-  && !CLASS_DATA (c)->ts.u.derived->attr.zero_comp
-  && !UNLIMITED_POLY (c))
-{
-  gfc_error ("The pointer component %qs of %qs at %L is a type "
- "that has not been declared", c->name, sym->name,
- >loc);
-  return false;
-}
-
   /* If an allocatable component derived type is of the same type as
  the enclosing derived type, we need a vtable generating so that
  the __deallocate procedure is created.  */
@@ -14258,6 +14236,13 @@ resolve_fl_derived (gfc_symbol *sym)
 			  >declared_at))
 return false;
 
+  if (sym->components == NULL && !sym->attr.zero_comp)
+{
+  gfc_error ("Derived type %qs at %L has not been declared",
+		  sym->name, >declared_at);
+  return false;
+}
+
   /* Resolve the finalizer procedures.  */
   if (!gfc_resolve_finalizers (sym, NULL))
 return false;
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 6c87f8017d3..5de896bdf37 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,15 @@
+2018-08-21  Janus Weil  
+
+	PR fortran/86888
+	* gfortran.dg/alloc_comp_basics_6.f90: Update an error message and add
+	an additional case.
+	* gfortran.dg/alloc_comp_basics_7.f90: New test case.
+	* gfortran.dg/class_17.f03: Update error message.
+	* gfortran.dg/class_55.f90: Ditto.
+	* gfortran.dg/dtio_11.f90: Update error messages.
+	* gfortran.dg/implicit_actual.f90: Add an error message.
+	* gfortran.dg/typebound_proc_12.f90: Update error message.
+
 2018-08-21  Marek Polacek  
 
 	PR c++/86981, Implement -Wpessimizing-move.
diff --git a/gcc/testsuite/gfortran.dg/alloc_comp_basics_6.f90 b/gcc/testsuite/gfortran.dg/alloc_comp_basics_6.f90
index 3ed221db24f..4eb0e49a7e5 100644
--- a/gcc/testsuite/gfortran.dg/alloc_comp_basics_6.f90
+++ b/gcc/testsuite/gfortran.dg/alloc_comp_basics_6.f90
@@ -5,7 +5,8 @@
 ! Contributed by Joost VandeVondele 
 
   type sysmtx_t
- type(ext_complex_t), allocatable :: S(:)  ! { dg-error "has not been previously defined" }
+ type(ext_complex_t), allocatable :: S(:)  ! { dg-error "has not been declared" }
+ class(some_type), allocatable :: X! { dg-error "has not been declared" }
   end type
 
 end
diff --git a/gcc/testsuite/gfortran.dg/alloc_comp_basics_7.f90 b/gcc/testsuite/gfortran.dg/alloc_comp_basic

Re: [Patch, Fortran] PR 86116: Ambiguous generic interface not recognised

[Janus Weil]

Am Di., 14. Aug. 2018 um 16:16 Uhr schrieb Fritz Reese :
>
> Looks OK to me.

Thanks, Fritz. Committed as r263540.

Since this PR is a regression, it should probably be backported to the
release branches as well. However, I'll a wait a week or two with
that, in order to check for possible problems on trunk ...

Cheers,
Janus



> On Tue, Aug 14, 2018 at 4:12 AM Janus Weil  wrote:
> >
> > ping!
> >
> >
> > Am So., 5. Aug. 2018 um 15:23 Uhr schrieb Janus Weil :
> > >
> > > Hi all,
> > >
> > > the attached patch fixes PR 86116 by splitting up the function
> > > 'compare_type' into two variants: One that is used for checking
> > > generic interfaces and operators (keeping the old name), and one that
> > > is used for checking dummy functions and procedure pointer assignments
> > > ('compare_type_characteristics'). The latter calls the former, but
> > > includes an additional check that must not be done when checking
> > > generics.
> > >
> > > Regtests cleanly on x86_64-linux-gnu. Ok for trunk?
> > >
> > > Cheers,
> > > Janus
> > >
> > >
> > > 2018-08-05  Janus Weil  
> > >
> > > PR fortran/86116
> > > * interface.c (compare_type): Remove a CLASS/TYPE check.
> > > (compare_type_characteristics): New function that behaves like the old
> > > 'compare_type'.
> > > (gfc_check_dummy_characteristics, gfc_check_result_characteristics):
> > > Call 'compare_type_characteristics' instead of 'compare_type'.
> > >
> > > 2018-08-05  Janus Weil  
> > >
> > > PR fortran/86116
> > > * gfortran.dg/generic_34.f90: New test case.

Re: [Patch, Fortran] PR 86116: Ambiguous generic interface not recognised

[Janus Weil]

ping!


Am So., 5. Aug. 2018 um 15:23 Uhr schrieb Janus Weil :
>
> Hi all,
>
> the attached patch fixes PR 86116 by splitting up the function
> 'compare_type' into two variants: One that is used for checking
> generic interfaces and operators (keeping the old name), and one that
> is used for checking dummy functions and procedure pointer assignments
> ('compare_type_characteristics'). The latter calls the former, but
> includes an additional check that must not be done when checking
> generics.
>
> Regtests cleanly on x86_64-linux-gnu. Ok for trunk?
>
> Cheers,
> Janus
>
>
> 2018-08-05  Janus Weil  
>
> PR fortran/86116
> * interface.c (compare_type): Remove a CLASS/TYPE check.
> (compare_type_characteristics): New function that behaves like the old
> 'compare_type'.
> (gfc_check_dummy_characteristics, gfc_check_result_characteristics):
> Call 'compare_type_characteristics' instead of 'compare_type'.
>
> 2018-08-05  Janus Weil  
>
> PR fortran/86116
> * gfortran.dg/generic_34.f90: New test case.

[Patch, Fortran] PR 86935: Bad locus in ASSOCIATE statement

[Janus Weil]

Hi all,

this simple patch improves some of the diagnostics for invalid
ASSOCIATE statements:

https://github.com/janusw/gcc/commit/2f484479c741abddc8ac473cb0c1b9010397e006

In particular it gives a more precise locus and improved wording,
which is achieved basically by splitting a 'gfc_match' into two
separate ones. Regtests cleanly on x86_64-linux-gnu. Ok for trunk?

Cheers,
Janus

[Patch, Fortran] PR 86116: Ambiguous generic interface not recognised

[Janus Weil]

Hi all,

the attached patch fixes PR 86116 by splitting up the function
'compare_type' into two variants: One that is used for checking
generic interfaces and operators (keeping the old name), and one that
is used for checking dummy functions and procedure pointer assignments
('compare_type_characteristics'). The latter calls the former, but
includes an additional check that must not be done when checking
generics.

Regtests cleanly on x86_64-linux-gnu. Ok for trunk?

Cheers,
Janus


2018-08-05  Janus Weil  

PR fortran/86116
* interface.c (compare_type): Remove a CLASS/TYPE check.
(compare_type_characteristics): New function that behaves like the old
'compare_type'.
(gfc_check_dummy_characteristics, gfc_check_result_characteristics):
Call 'compare_type_characteristics' instead of 'compare_type'.

2018-08-05  Janus Weil  

PR fortran/86116
* gfortran.dg/generic_34.f90: New test case.
Index: gcc/fortran/interface.c
===
--- gcc/fortran/interface.c	(revision 263308)
+++ gcc/fortran/interface.c	(working copy)
@@ -735,6 +735,13 @@ compare_type (gfc_symbol *s1, gfc_symbol *s2)
   if (s2->attr.ext_attr & (1 << EXT_ATTR_NO_ARG_CHECK))
 return true;
 
+  return gfc_compare_types (>ts, >ts) || s2->ts.type == BT_ASSUMED;
+}
+
+
+static bool
+compare_type_characteristics (gfc_symbol *s1, gfc_symbol *s2)
+{
   /* TYPE and CLASS of the same declared type are type compatible,
  but have different characteristics.  */
   if ((s1->ts.type == BT_CLASS && s2->ts.type == BT_DERIVED)
@@ -741,7 +748,7 @@ compare_type (gfc_symbol *s1, gfc_symbol *s2)
   || (s1->ts.type == BT_DERIVED && s2->ts.type == BT_CLASS))
 return false;
 
-  return gfc_compare_types (>ts, >ts) || s2->ts.type == BT_ASSUMED;
+  return compare_type (s1, s2);
 }
 
 
@@ -1309,7 +1316,8 @@ gfc_check_dummy_characteristics (gfc_symbol *s1, g
   /* Check type and rank.  */
   if (type_must_agree)
 {
-  if (!compare_type (s1, s2) || !compare_type (s2, s1))
+  if (!compare_type_characteristics (s1, s2)
+	  || !compare_type_characteristics (s2, s1))
 	{
 	  snprintf (errmsg, err_len, "Type mismatch in argument '%s' (%s/%s)",
 		s1->name, gfc_typename (>ts), gfc_typename (>ts));
@@ -1528,7 +1536,7 @@ gfc_check_result_characteristics (gfc_symbol *s1,
 return true;
 
   /* Check type and rank.  */
-  if (!compare_type (r1, r2))
+  if (!compare_type_characteristics (r1, r2))
 {
   snprintf (errmsg, err_len, "Type mismatch in function result (%s/%s)",
 		gfc_typename (>ts), gfc_typename (>ts));
! { dg-do compile }
!
! PR 86116: [6/7/8/9 Regression] Ambiguous generic interface not recognised
!
! Contributed by martin 

module mod

   type :: t
   end type t

   interface sub
  module procedure s1
  module procedure s2
   end interface

contains

   subroutine s1(x)  ! { dg-error "Ambiguous interfaces in generic interface" }
  type(t) :: x
   end subroutine

   subroutine s2(x)  ! { dg-error "Ambiguous interfaces in generic interface" }
  class(*), allocatable :: x
   end subroutine

end

[Patch, Fortran, F08] PR 45521: GENERIC resolution with ALLOCATABLE/POINTER and PROCEDURE

[Janus Weil]

Hi all,

this patch should finally fix up the last wrinkles of PR 45521, which
deals with disambiguating specific procedures in a generic interface
via the pointer/allocatable attributes of the arguments (legal in
F08).

For 'ordinary' generic interfaces this already works (cf.
'generic_correspondence'), but not for operator interfaces, which are
treated a bit differently (see 'gfc_compare_interfaces'). The patch
basically copies over the usage of 'compare_ptr_alloc' from
'generic_correspondence' to the relevant part of
'gfc_compare_interfaces'.

Regtests cleanly on x86_64-linux-gnu. Ok for trunk?

Cheers,
Janus


2018-08-04  Janus Weil  

PR fortran/45521
* interface.c (gfc_compare_interfaces): Apply additional
distinguishability criteria of F08 to operator interfaces.


2018-08-04  Janus Weil  

PR fortran/45521
* gfortran.dg/interface_assignment_6.f90: New test case.
Index: gcc/fortran/interface.c
===
--- gcc/fortran/interface.c	(revision 263178)
+++ gcc/fortran/interface.c	(working copy)
@@ -1776,7 +1776,7 @@
 	  }
 	else
 	  {
-	/* Only check type and rank.  */
+	/* Operators: Only check type and rank of arguments.  */
 	if (!compare_type (f2->sym, f1->sym))
 	  {
 		if (errmsg != NULL)
@@ -1794,6 +1794,15 @@
 			symbol_rank (f2->sym));
 		return false;
 	  }
+	if ((gfc_option.allow_std & GFC_STD_F2008)
+		&& (compare_ptr_alloc(f1->sym, f2->sym)
+		|| compare_ptr_alloc(f2->sym, f1->sym)))
+	  {
+		if (errmsg != NULL)
+		  snprintf (errmsg, err_len, "Mismatching POINTER/ALLOCATABLE "
+			"attribute in argument '%s' ", f1->sym->name);
+		return false;
+	  }
 	  }
   }
 
! { dg-do compile }
!
! PR 45521: [F08] GENERIC resolution with ALLOCATABLE/POINTER and PROCEDURE
!
! Contributed by Janus Weil 

module inteface_assignment_6

  type :: t
  end type

  ! this was rejected as ambiguous, but is valid in F08
  interface assignment(=)
procedure testAlloc
procedure testPtr
  end interface

contains

  subroutine testAlloc(obj, val)
type(t), allocatable, intent(out) :: obj
integer, intent(in) :: val
  end subroutine

  subroutine testPtr(obj, val)
type(t), pointer, intent(out) :: obj
integer, intent(in) :: val
  end subroutine

end

Re: [Patch, fortran] A first small step towards CFI descriptor implementation

[Janus Weil]

Hi Paul,

2018-07-31 14:06 GMT+02:00 Paul Richard Thomas :
> Daniel Celis Garza and Damian Rouson have developed a runtime library
> and include file for the TS 29113 and F2018 C descriptors.
> https://github.com/sourceryinstitute/ISO_Fortran_binding
>
> The ordering of types is different to the current 'bt' enum in
> libgfortran.h. This patch interchanges BT_DERIVED and BT_CHARACTER to
> fix this.

is this ordering actually fixed by the F18 standard, or is there any
other reason why it needs to be like this? What's wrong with
gfortran's current ordering?

Cheers,
Janus

Re: [Patch, fortran] PR80477 - [OOP] Polymorphic function result generates memory leak

[Janus Weil]

Hi Paul,

2018-07-28 9:32 GMT+02:00 Paul Richard Thomas :
> Several attempts, including mine, were made to fix this bug since it
> was posted. They were all attacking the wrong place. Instead of
> providing the free of the class _data as part of the call to
> 'add_a_type' it should be included in the post block of the argument
> processing in the call to 'assign_a_type'. The comment in the patch
> says the rest.
>
> Bootstrapped and regtested on FC28/x86_64 - OK for trunk?

great that you managed to solve this one! The patch looks very good to
me, but I'm afraid two details may be missing:

1) If the type has allocatable components, those need to be freed first.
2) If the type has a finalizer, that needs to be called as well.

I believe that both points can be fixed by calling the _final
component of the vtab before freeing the class data. Should not be
hard to add, I hope (gfc_add_finalizer_call might be useful).

Thanks for your efforts ...

Cheers,
Janus

Re: [Patch, Fortran] PR 57160: short-circuit IF only with -ffrontend-optimize

[Janus Weil]

2018-07-24 17:41 GMT+02:00 Janne Blomqvist :
> Optimization bugs that pop up at different optimization levels are hard
> enough for users to figure out

Right, and they're impossible to detect if there is no way to disable
the optimization, which is what this PR is about.


> without the frontend generating different
> code to begin with depending on the optimization level.

In the end it doesn't make much of a difference whether the
optimizations are done in the front or the middle end. The user knows
nothing about this, and he doesn't need to.

The problematic point here is that short-circuiting is an optimization
that is enabled already at -O0.


> Also, with a
> separate option it would be easy to check how it affects performance at
> different optimization levels.

For the case at hand, the short-circuiting is an absolutely valid
optimization. There is no reason why you wouldn't wanna do it (with -O
flags).


> What about making it a -fcheck=short-circuit-logicals (or however you want
> to spell it?) option, that also would be enabled with -fcheck=all?

What would such a flag even do? The actually invalid operation in the
test case is a null-pointer access, which could be caught by
-fcheck=pointer if we disable the optimization that removes it (i.e.
short-circuiting).

However, it also seems like -fcheck=pointer could use some
enhancement, since it does not even seem to catch simple cases like
this:

integer, pointer :: p => null()
print *,p
end

Cheers,
Janus

Re: [Patch, Fortran] PR 57160: short-circuit IF only with -ffrontend-optimize

[Janus Weil]

2018-07-24 11:12 GMT+02:00 Dominique d'Humières :
> Hi Janus,
>
>> gfortran currently does short-circuiting, and after my patch for PR
>> 85599 warns about cases where this might remove an impure function
>> call (which potentially can change results).
>>
>> Now, this PR (57160) is about code which relies on the
>> short-circuiting behavior. Since short-circuiting is not guaranteed by
>> the standard, such code is invalid. Generating a warning or an error
>> at compile-time is a bit harder here, though, since there are multiple
>> variations of such a situation, e.g.:
>> * ASSOCIATED(p) .AND. p%T
>> * ALLOCATED(a) .AND. a%T
>> * i> * …
>>
>
> Aren’t you confusing portability with validity?

I don' think so.


> The above codes are indeed invalid without short-circuit evaluation,
>  but I did not find anything in the standard saying such codes are
> invalid with short-circuit evaluation.

If they're not valid without short-circuiting, then they're not valid
at all, because the Fortran standard does not require a processor to
do short-circuiting.


>> The suggestion in the PR was to do short-circuiting only with
>> optimization flags, but inhibit it with -O0, so that the faulty code
>> will run into a segfault (or runtime error) at least when
>> optimizations are disabled, and the problem can be identified.
>
> This PR has nothing to do with optimization and I think
>  it is a very bad idea to (ab)use any optimization option.

The PR is definitely related to optimization, because the fact that
the invalid test case works at all is only due to an optional
optimization called short-circuiting.

Don't get me wrong. I think it would be great if the Fortran standard
would *require* short-circuiting for and/or operators (or had separate
operators which do that). That would allow to write code like
"ASSOCIATED(p) .AND. p%T" (which would actually be useful).
Unfortunately that's not the case, therefore such code is plain
invalid.


> Please leave the default behavior (and test) as they are now.

You seriously prefer to keep an invalid test case in the testsuite?


> If you want non short-circuit evaluation, introduce an option for it.

Your argument could easily be reversed: If you want short-circuiting,
go introduce an option for it.

I'm sure we'll not get anywhere this way, and I do think that Joost's
suggestion to avoid short-circuiting with -O0 (and possibly -Og) is
very reasonable: People who want maximum performance still get that
with -O3, and at the same time they can still check their codes for
errors with -O0.

What is your problem?!?


> Note that the warning introduce for pr85599 should be disabled
> for non short-circuit evaluations.

That's a valid point.

Cheers,
Janus

Re: [Patch, fortran] PR66679 - [OOP] ICE with class(*) and transfer

[Janus Weil]

Hi Paul,

2018-07-23 17:51 GMT+02:00 Paul Richard Thomas :
> Ping!
> On Thu, 5 Jul 2018 at 08:51, Paul Richard Thomas
>  wrote:
>>
>> The comment in the patch says it all.
>>
>> Bootstrapped and regtested on FC28/x86_64 - OK for trunk?

I don't see anything wrong with the patch, so as far as I'm concerned
it's ok for trunk.

Just one minor style-wise suggestion: I'd move the declaration of the
'class_expr' into the else branch where it's used (in order to have it
as locally as possible).

Thanks for the patch,
Janus



>> 2018-07-05  Paul Thomas  
>>
>> PR fortran/66679
>> * trans-intrinsic.c (gfc_conv_intrinsic_transfer): Class array
>> elements are returned as references to the data element. Get
>> the class expression by stripping back the references. Use this
>> for the element size.
>>
>> 2018-07-05  Paul Thomas  
>>
>> PR fortran/66679
>> * gfortran.dg/transfer_class_3.f90: New test.
>
>
>
> --
> "If you can't explain it simply, you don't understand it well enough"
> - Albert Einstein

[Patch, Fortran] PR 57160: short-circuit IF only with -ffrontend-optimize

[Janus Weil]

Hi all,

here is a follow-up patch to my recent commit for PR 85599, also
dealing with the short-circuiting of logical operators. In the course
of the extensive discussion of that PR it became clear that the
Fortran standard allows the short-circuiting of .AND. and .OR.
operators, but does not mandate it.

gfortran currently does short-circuiting, and after my patch for PR
85599 warns about cases where this might remove an impure function
call (which potentially can change results).

Now, this PR (57160) is about code which relies on the
short-circuiting behavior. Since short-circuiting is not guaranteed by
the standard, such code is invalid. Generating a warning or an error
at compile-time is a bit harder here, though, since there are multiple
variations of such a situation, e.g.:
* ASSOCIATED(p) .AND. p%T
* ALLOCATED(a) .AND. a%T
* i

PR fortran/57160
* trans-expr.c (gfc_conv_expr_op): Use short-circuiting operators only
with -ffrontend-optimize. Without that flag, make sure that both
operands are evaluated.


2018-07-20  Janus Weil  

PR fortran/57160
* gfortran.dg/actual_pointer_function_1.f90: Fix invalid test case.
Index: gcc/fortran/trans-expr.c
===
--- gcc/fortran/trans-expr.c	(revision 262908)
+++ gcc/fortran/trans-expr.c	(working copy)
@@ -3348,12 +3348,12 @@ gfc_conv_expr_op (gfc_se * se, gfc_expr * expr)
   return;
 
 case INTRINSIC_AND:
-  code = TRUTH_ANDIF_EXPR;
+  code = flag_frontend_optimize ? TRUTH_ANDIF_EXPR : TRUTH_AND_EXPR;
   lop = 1;
   break;
 
 case INTRINSIC_OR:
-  code = TRUTH_ORIF_EXPR;
+  code = flag_frontend_optimize ? TRUTH_ORIF_EXPR : TRUTH_OR_EXPR;
   lop = 1;
   break;
 
Index: gcc/testsuite/gfortran.dg/actual_pointer_function_1.f90
===
--- gcc/testsuite/gfortran.dg/actual_pointer_function_1.f90	(revision 262908)
+++ gcc/testsuite/gfortran.dg/actual_pointer_function_1.f90	(working copy)
@@ -17,7 +17,11 @@ CONTAINS
 
   logical function cp_logger_log(logger)
 TYPE(cp_logger_type), POINTER ::logger
-cp_logger_log = associated (logger) .and. (logger%a .eq. 42)
+if (associated (logger)) then
+  cp_logger_log = (logger%a .eq. 42)
+else
+  cp_logger_log = .false.
+end if
   END function
 
   FUNCTION cp_get_default_logger(v) RESULT(res)

Re: [Patch, Fortran] PR 85599: warn about short-circuiting of logical expressions for non-pure functions

[Janus Weil]

2018-07-17 20:55 GMT+02:00 Fritz Reese :
> On Tue, Jul 17, 2018 at 2:36 PM Janus Weil  wrote:
>>
>> 2018-07-17 19:34 GMT+02:00 Thomas Koenig :
>> > Am 17.07.2018 um 19:19 schrieb Janus Weil:
> [...]
>>
>> I do hope that things have converged by now and that this will be the
>> last incarnation of the patch. If there is no more feedback in the
>> next 24 hours, I'll commit this tomorrow.
>
> I hate to be pedantic but it is still worth fixing the style discrepancies:

Oh, sure. Such things are non-optional in GCC, I was just a bit sloppy
with this. Thanks for catching! Should be fixed in the attached
update.


> My only other comment is I am not sure why you make
> pure_function()/gfc_implicit_pure_function() public... but I have no
> real problem with it. Just means rebuilding all of f951 instead of one
> object. :-(

Well, originally they were only used in resolve.c, but now I need them
also in frontend-passes.c, therefore they have to be public.


> Otherwise if the patch does what it appears to do and passes tests
> then it seems fine to me.

Thanks for the review!

Cheers,
Janus
Index: gcc/fortran/dump-parse-tree.c
===
--- gcc/fortran/dump-parse-tree.c	(revision 262828)
+++ gcc/fortran/dump-parse-tree.c	(working copy)
@@ -716,6 +716,8 @@ show_attr (symbol_attribute *attr, const char * mo
 fputs (" ELEMENTAL", dumpfile);
   if (attr->pure)
 fputs (" PURE", dumpfile);
+  if (attr->implicit_pure)
+fputs (" IMPLICIT_PURE", dumpfile);
   if (attr->recursive)
 fputs (" RECURSIVE", dumpfile);
 
Index: gcc/fortran/frontend-passes.c
===
--- gcc/fortran/frontend-passes.c	(revision 262828)
+++ gcc/fortran/frontend-passes.c	(working copy)
@@ -840,17 +840,22 @@ create_var (gfc_expr * e, const char *vname)
 static void
 do_warn_function_elimination (gfc_expr *e)
 {
-  if (e->expr_type != EXPR_FUNCTION)
-return;
-  if (e->value.function.esym)
-gfc_warning (OPT_Wfunction_elimination,
-		 "Removing call to function %qs at %L",
-		 e->value.function.esym->name, &(e->where));
-  else if (e->value.function.isym)
-gfc_warning (OPT_Wfunction_elimination,
-		 "Removing call to function %qs at %L",
-		 e->value.function.isym->name, &(e->where));
+  const char *name;
+  if (e->expr_type == EXPR_FUNCTION
+  && !gfc_pure_function (e, ) && !gfc_implicit_pure_function (e))
+   {
+  if (name)
+	  gfc_warning (OPT_Wfunction_elimination,
+		  "Removing call to impure function %qs at %L", name,
+		  &(e->where));
+  else
+	  gfc_warning (OPT_Wfunction_elimination,
+		  "Removing call to impure function at %L",
+		  &(e->where));
+   }
 }
+
+
 /* Callback function for the code walker for doing common function
elimination.  This builds up the list of functions in the expression
and goes through them to detect duplicates, which it then replaces
Index: gcc/fortran/gfortran.h
===
--- gcc/fortran/gfortran.h	(revision 262828)
+++ gcc/fortran/gfortran.h	(working copy)
@@ -3275,6 +3275,8 @@ bool gfc_resolve_intrinsic (gfc_symbol *, locus *)
 bool gfc_explicit_interface_required (gfc_symbol *, char *, int);
 extern int gfc_do_concurrent_flag;
 const char* gfc_lookup_function_fuzzy (const char *, gfc_symtree *);
+int gfc_pure_function (gfc_expr *e, const char **name);
+int gfc_implicit_pure_function (gfc_expr *e);
 
 
 /* array.c */
Index: gcc/fortran/gfortran.texi
===
--- gcc/fortran/gfortran.texi	(revision 262828)
+++ gcc/fortran/gfortran.texi	(working copy)
@@ -1177,6 +1177,7 @@ might in some way or another become visible to the
 @menu
 * KIND Type Parameters::
 * Internal representation of LOGICAL variables::
+* Evaluation of logical expressions::
 * Thread-safety of the runtime library::
 * Data consistency and durability::
 * Files opened without an explicit ACTION= specifier::
@@ -1251,6 +1252,19 @@ values: @code{1} for @code{.TRUE.} and @code{0} fo
 See also @ref{Argument passing conventions} and @ref{Interoperability with C}.
 
 
+@node Evaluation of logical expressions
+@section Evaluation of logical expressions
+
+The Fortran standard does not require the compiler to evaluate all parts of an
+expression, if they do not contribute to the final result.  For logical
+expressions with @code{.AND.} or @code{.OR.} operators, in particular, GNU
+Fortran will optimize out function calls (even to impure functions) if the
+result of the expression can be established without them.  However, since not
+all compilers do that, and such an optimization can potentially modify the
+program flow and subsequent

Re: [Patch, Fortran] PR 85599: warn about short-circuiting of logical expressions for non-pure functions

[Janus Weil]

2018-07-17 19:34 GMT+02:00 Thomas Koenig :
> Am 17.07.2018 um 19:19 schrieb Janus Weil:
>> However, the test case above seems to indicate that the
>> function-elimination optimization is not applied to impure functions
>> anyway (which is good IMHO).
>
> If you specify -faggressive-function-elimination, it is also
> done for impure (and non implicitly-pure) functions.

Ah, ok.


> Problem is that, in all probability, nobody uses this option at the
> moment.

That's probably true, but it should get a little better once we move
it into -Wextra.


>> It that is true, then my modifications
>> practically disable the old -Wfunction-elimination warnings completely
>> :/
>
> I do not think it would be a problem not to warn for removing
> calls to pure or implicitly pure fuctions. The test cases can
> easily be modified not to emit this warning, as you did.
>
> As the author of the original test cases, I may be able to
> say so with a certain amount of credibility.

Good, so let's do this. Attached is another update of my patch, which
incorporates all of Fritz' comments and should regtest cleanly now.

In function_optimize_5.f90 I have removed the warnings for pure
functions and instead added -faggressive-function-elimination and the
corresponding warnings for impure functions, which were apparently not
covered by the testsuite before.

I do hope that things have converged by now and that this will be the
last incarnation of the patch. If there is no more feedback in the
next 24 hours, I'll commit this tomorrow.

Cheers,
Janus
Index: gcc/fortran/dump-parse-tree.c
===
--- gcc/fortran/dump-parse-tree.c	(revision 262828)
+++ gcc/fortran/dump-parse-tree.c	(working copy)
@@ -716,6 +716,8 @@ show_attr (symbol_attribute *attr, const char * mo
 fputs (" ELEMENTAL", dumpfile);
   if (attr->pure)
 fputs (" PURE", dumpfile);
+  if (attr->implicit_pure)
+fputs (" IMPLICIT_PURE", dumpfile);
   if (attr->recursive)
 fputs (" RECURSIVE", dumpfile);
 
Index: gcc/fortran/frontend-passes.c
===
--- gcc/fortran/frontend-passes.c	(revision 262828)
+++ gcc/fortran/frontend-passes.c	(working copy)
@@ -840,17 +840,22 @@ create_var (gfc_expr * e, const char *vname)
 static void
 do_warn_function_elimination (gfc_expr *e)
 {
-  if (e->expr_type != EXPR_FUNCTION)
-return;
-  if (e->value.function.esym)
-gfc_warning (OPT_Wfunction_elimination,
-		 "Removing call to function %qs at %L",
-		 e->value.function.esym->name, &(e->where));
-  else if (e->value.function.isym)
-gfc_warning (OPT_Wfunction_elimination,
-		 "Removing call to function %qs at %L",
-		 e->value.function.isym->name, &(e->where));
+  const char *name;
+  if (e->expr_type == EXPR_FUNCTION
+  && !gfc_pure_function (e, ) && !gfc_implicit_pure_function (e))
+   {
+  if (name)
+ gfc_warning (OPT_Wfunction_elimination,
+		  "Removing call to impure function %qs at %L", name,
+		  &(e->where));
+  else
+ gfc_warning (OPT_Wfunction_elimination,
+		  "Removing call to impure function at %L",
+		  &(e->where));
+   }
 }
+
+
 /* Callback function for the code walker for doing common function
elimination.  This builds up the list of functions in the expression
and goes through them to detect duplicates, which it then replaces
Index: gcc/fortran/gfortran.h
===
--- gcc/fortran/gfortran.h	(revision 262828)
+++ gcc/fortran/gfortran.h	(working copy)
@@ -3275,6 +3275,8 @@ bool gfc_resolve_intrinsic (gfc_symbol *, locus *)
 bool gfc_explicit_interface_required (gfc_symbol *, char *, int);
 extern int gfc_do_concurrent_flag;
 const char* gfc_lookup_function_fuzzy (const char *, gfc_symtree *);
+int gfc_pure_function (gfc_expr *e, const char **name);
+int gfc_implicit_pure_function (gfc_expr *e);
 
 
 /* array.c */
Index: gcc/fortran/gfortran.texi
===
--- gcc/fortran/gfortran.texi	(revision 262828)
+++ gcc/fortran/gfortran.texi	(working copy)
@@ -1177,6 +1177,7 @@ might in some way or another become visible to the
 @menu
 * KIND Type Parameters::
 * Internal representation of LOGICAL variables::
+* Evaluation of logical expressions::
 * Thread-safety of the runtime library::
 * Data consistency and durability::
 * Files opened without an explicit ACTION= specifier::
@@ -1251,6 +1252,19 @@ values: @code{1} for @code{.TRUE.} and @code{0} fo
 See also @ref{Argument passing conventions} and @ref{Interoperability with C}.
 
 
+@node Evaluation of logical expressions
+@section Evaluation of logical expressions
+
+The Fortran standard does not 

Re: [Patch, Fortran] PR 85599: warn about short-circuiting of logical expressions for non-pure functions

[Janus Weil]

2018-07-17 17:18 GMT+02:00 Fritz Reese :
>> 2018-07-17 9:52 GMT+02:00 Janus Weil :
>> > In other words: Does it make sense to tone down
>> > -Wfunction-elimination, by only warning about impure functions?
>>
>> Here is an update of the patch which does that. Regtesting now ...
>
> Would not this break the testcase function_optimize_5.f90 :

My regtest says so as well ;)


> The docs for -Wfunction-elimination would read:
>
>> Warn if any calls to functions are eliminated by the optimizations
>> enabled by the @option{-ffrontend-optimize} option.
>> This option is implied by @option{-Wextra}.
>
> However, with your patch, it should probably read something like "warn
> if any calls to impure functions are eliminated..." Possibly with an
> explicit remark indicating that pure functions are not warned.

Yes.

However, the test case above seems to indicate that the
function-elimination optimization is not applied to impure functions
anyway (which is good IMHO). It that is true, then my modifications
practically disable the old -Wfunction-elimination warnings completely
:/



> Have you considered using levels for the flag, such that the original
> behavior of -Wfunction-elimination would be retained with e.g.
> -Wfunction-elimination=2 whereas one could use
> -Wfunction-elimination=1 (or similar) to omit warnings about impure
> functions?

One could do that, but IMHO it would be overkill. I don't see much
sense in warning for the elimination of pure functions. But maybe I'm
missing something?

Cheers,
Janus

Re: [Patch, Fortran] PR 85599: warn about short-circuiting of logical expressions for non-pure functions

[Janus Weil]

2018-07-17 9:52 GMT+02:00 Janus Weil :
>> 2018-07-16 21:50 GMT+02:00 Thomas Koenig :
>>> What I would suggest is to enable -Wfunction-eliminiation with
>>> -Wextra and also use that for your new warning.
>>
>> Thanks for the comments. Makes sense. Updated patch attached.
>
>
> Huh, after actually trying -Wfunction-elimination, I'm not so sure any
> more if it really makes sense to throw the new warnings in there,
> mainly for two reasons:
>
> a) -Wfunction-elimination is slightly different, in that it warns
> about removing *any* kind of function, not just impure ones. This
> makes it pretty verbose, and I'm not sure why one would need to know
> if a pure function call is removed.
> b) It gives warnings on places that I don't understand. Simple example:
>
> subroutine sub(r)
>implicit none
>real, intent(in) :: r
>if ((abs(r) > 1e6) .or. (abs(r) < 1e-6)) then
>   print *, "rrr"
>end if
> end
>
> Compiling this with "gfortran-8 -O1 -Wfunction-elimination" gives me:
>
> if ((abs(r) > 1e6) .or. (abs(r) < 1e-6)) then
> 1
> Warning: Removing call to function ‘abs’ at (1) [-Wfunction-elimination]
>
>
> Why can I remove the call to ABS there? If I read the dump correctly,
> then the two calls to ABS are optimized into a single one, which is
> used for both comparisons via a temporary. Is that what the warning is
> trying to tell me? And if yes, why should I care (if the function is
> pure)? The middle-end certainly does tons of optimizations that
> rearrange various expressions, without warning me about it ...
>
> In other words: Does it make sense to tone down
> -Wfunction-elimination, by only warning about impure functions?

Here is an update of the patch which does that. Regtesting now ...

Cheers,
Janus
Index: gcc/fortran/dump-parse-tree.c
===
--- gcc/fortran/dump-parse-tree.c	(revision 262764)
+++ gcc/fortran/dump-parse-tree.c	(working copy)
@@ -716,6 +716,8 @@ show_attr (symbol_attribute *attr, const char * mo
 fputs (" ELEMENTAL", dumpfile);
   if (attr->pure)
 fputs (" PURE", dumpfile);
+  if (attr->implicit_pure)
+fputs (" IMPLICIT_PURE", dumpfile);
   if (attr->recursive)
 fputs (" RECURSIVE", dumpfile);
 
Index: gcc/fortran/frontend-passes.c
===
--- gcc/fortran/frontend-passes.c	(revision 262764)
+++ gcc/fortran/frontend-passes.c	(working copy)
@@ -840,17 +840,17 @@ create_var (gfc_expr * e, const char *vname)
 static void
 do_warn_function_elimination (gfc_expr *e)
 {
-  if (e->expr_type != EXPR_FUNCTION)
-return;
-  if (e->value.function.esym)
-gfc_warning (OPT_Wfunction_elimination,
-		 "Removing call to function %qs at %L",
-		 e->value.function.esym->name, &(e->where));
-  else if (e->value.function.isym)
-gfc_warning (OPT_Wfunction_elimination,
-		 "Removing call to function %qs at %L",
-		 e->value.function.isym->name, &(e->where));
+  const char *name;
+  if (e->expr_type == EXPR_FUNCTION && !gfc_pure_function (e, ) && !gfc_implicit_pure_function (e))
+   {
+  if (name)
+ gfc_warning (OPT_Wfunction_elimination, "Removing call to impure function %qs at %L", name, &(e->where));
+  else
+ gfc_warning (OPT_Wfunction_elimination, "Removing call to impure function at %L", &(e->where));
+   }
 }
+
+
 /* Callback function for the code walker for doing common function
elimination.  This builds up the list of functions in the expression
and goes through them to detect duplicates, which it then replaces
Index: gcc/fortran/gfortran.h
===
--- gcc/fortran/gfortran.h	(revision 262764)
+++ gcc/fortran/gfortran.h	(working copy)
@@ -3275,6 +3275,8 @@ bool gfc_resolve_intrinsic (gfc_symbol *, locus *)
 bool gfc_explicit_interface_required (gfc_symbol *, char *, int);
 extern int gfc_do_concurrent_flag;
 const char* gfc_lookup_function_fuzzy (const char *, gfc_symtree *);
+int gfc_pure_function (gfc_expr *e, const char **name);
+int gfc_implicit_pure_function (gfc_expr *e);
 
 
 /* array.c */
Index: gcc/fortran/gfortran.texi
===
--- gcc/fortran/gfortran.texi	(revision 262764)
+++ gcc/fortran/gfortran.texi	(working copy)
@@ -1177,6 +1177,7 @@ might in some way or another become visible to the
 @menu
 * KIND Type Parameters::
 * Internal representation of LOGICAL variables::
+* Evaluation of logical expressions::
 * Thread-safety of the runtime library::
 * Data consistency and durability::
 * Files opened without an explicit ACTIO

Re: [PR fortran/83184] Fix matching code for clist/old-style initializers when encountering assumed-rank declarations

[Janus Weil]

Did someone actually approve this patch? Apparently it was committed
as r262744 and caused the following regression:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86543

Cheers,
Janus



2018-06-27 23:07 GMT+02:00 Fritz Reese :
> The attached patch fixes PR fortran/83184, which is actually two
> distinct bugs as described in the PR. Passes regtests.
>
> The patch is attached. OK for trunk and 7/8-branch?
>
> From 238f0a0e80c93209bb4e62ba2f719f74f5da164f Mon Sep 17 00:00:00 2001
> From: Fritz Reese 
> Date: Wed, 27 Jun 2018 16:16:31 -0400
> Subject: [PATCH 2/3] PR fortran/83184
>
> Fix handling of invalid assumed-shape/size arrays in legacy initializer
> lists.
>
> gcc/fortran/
>
> * decl.c (match_old_style_init): Initialize locus of variable expr 
> when
> creating a data variable.
> (match_clist_expr): Verify array is explicit shape/size before
> attempting to allocate constant array constructor.
>
> gcc/testsuite/
>
> * gfortran.dg/assumed_rank_14.f90: New testcase.
> * gfortran.dg/assumed_rank_15.f90: New testcase.
> * gfortran.dg/dec_structure_8.f90: Update error messages.
> * gfortran.dg/dec_structure_23.f90: Update error messages.
> ---
>  gcc/fortran/decl.c | 63 
> +++---
>  gcc/testsuite/gfortran.dg/assumed_rank_14.f90  | 11 +
>  gcc/testsuite/gfortran.dg/assumed_rank_15.f90  | 11 +
>  gcc/testsuite/gfortran.dg/dec_structure_23.f90 |  6 +--
>  gcc/testsuite/gfortran.dg/dec_structure_8.f90  |  6 +--
>  5 files changed, 64 insertions(+), 33 deletions(-)
>  create mode 100644 gcc/testsuite/gfortran.dg/assumed_rank_14.f90
>  create mode 100644 gcc/testsuite/gfortran.dg/assumed_rank_15.f90

Re: [Patch, Fortran] PR 85599: warn about short-circuiting of logical expressions for non-pure functions

[Janus Weil]

> 2018-07-16 21:50 GMT+02:00 Thomas Koenig :
>> What I would suggest is to enable -Wfunction-eliminiation with
>> -Wextra and also use that for your new warning.
>
> Thanks for the comments. Makes sense. Updated patch attached.


Huh, after actually trying -Wfunction-elimination, I'm not so sure any
more if it really makes sense to throw the new warnings in there,
mainly for two reasons:

a) -Wfunction-elimination is slightly different, in that it warns
about removing *any* kind of function, not just impure ones. This
makes it pretty verbose, and I'm not sure why one would need to know
if a pure function call is removed.
b) It gives warnings on places that I don't understand. Simple example:

subroutine sub(r)
   implicit none
   real, intent(in) :: r
   if ((abs(r) > 1e6) .or. (abs(r) < 1e-6)) then
  print *, "rrr"
   end if
end

Compiling this with "gfortran-8 -O1 -Wfunction-elimination" gives me:

if ((abs(r) > 1e6) .or. (abs(r) < 1e-6)) then
1
Warning: Removing call to function ‘abs’ at (1) [-Wfunction-elimination]


Why can I remove the call to ABS there? If I read the dump correctly,
then the two calls to ABS are optimized into a single one, which is
used for both comparisons via a temporary. Is that what the warning is
trying to tell me? And if yes, why should I care (if the function is
pure)? The middle-end certainly does tons of optimizations that
rearrange various expressions, without warning me about it ...

In other words: Does it make sense to tone down
-Wfunction-elimination, by only warning about impure functions? (We
certainly have the diagnostic capabilites for this.) If not, I'd
rather have a separate flag for the new warnings.
-Wfunction-elimination is just too noisy for my taste in its current
form.

Cheers,
Janus

Re: [Patch, Fortran] PR 85599: warn about short-circuiting of logical expressions for non-pure functions

[Janus Weil]

2018-07-16 21:50 GMT+02:00 Thomas Koenig :
> Am 16.07.2018 um 10:06 schrieb Janus Weil:
>>>
>>> However, one point: I think that the warning should be under a separate
>>> warning, which should then be enabled by -Wextra.
>>> -Waggressive-function-elimination, could be reused for this,
>>> or something else
>>
>> I don't actually see such a flag in the manual.
>
> Ah, sorry, I misremembered the option, it is actually
> -Wfunction-elimination.
>
> What I would suggest is to enable -Wfunction-eliminiation with
> -Wextra and also use that for your new warning.

Thanks for the comments. Makes sense. Updated patch attached.

I'll wait two more days to allow for further comments and will commit
this to trunk on Thursday if I hear no further complaints.

Cheers,
Janus
Index: gcc/fortran/dump-parse-tree.c
===
--- gcc/fortran/dump-parse-tree.c	(revision 262563)
+++ gcc/fortran/dump-parse-tree.c	(working copy)
@@ -716,6 +716,8 @@ show_attr (symbol_attribute *attr, const char * mo
 fputs (" ELEMENTAL", dumpfile);
   if (attr->pure)
 fputs (" PURE", dumpfile);
+  if (attr->implicit_pure)
+fputs (" IMPLICIT_PURE", dumpfile);
   if (attr->recursive)
 fputs (" RECURSIVE", dumpfile);
 
Index: gcc/fortran/gfortran.texi
===
--- gcc/fortran/gfortran.texi	(revision 262563)
+++ gcc/fortran/gfortran.texi	(working copy)
@@ -1177,6 +1177,7 @@ might in some way or another become visible to the
 @menu
 * KIND Type Parameters::
 * Internal representation of LOGICAL variables::
+* Evaluation of logical expressions::
 * Thread-safety of the runtime library::
 * Data consistency and durability::
 * Files opened without an explicit ACTION= specifier::
@@ -1251,6 +1252,19 @@ values: @code{1} for @code{.TRUE.} and @code{0} fo
 See also @ref{Argument passing conventions} and @ref{Interoperability with C}.
 
 
+@node Evaluation of logical expressions
+@section Evaluation of logical expressions
+
+The Fortran standard does not require the compiler to evaluate all parts of an
+expression, if they do not contribute to the final result. For logical
+expressions with @code{.AND.} or @code{.OR.} operators, in particular, GNU
+Fortran will optimize out function calls (even to impure functions) if the
+result of the expression can be established without them. However, since not
+all compilers do that, and such an optimization can potentially modify the
+program flow and subsequent results, GNU Fortran throws warnings for such
+situations with the @option{-Wfunction-elimination} flag.
+
+
 @node Thread-safety of the runtime library
 @section Thread-safety of the runtime library
 @cindex thread-safety, threads
Index: gcc/fortran/invoke.texi
===
--- gcc/fortran/invoke.texi	(revision 262563)
+++ gcc/fortran/invoke.texi	(working copy)
@@ -1058,6 +1058,7 @@ off via @option{-Wno-align-commons}. See also @opt
 @cindex warnings, function elimination
 Warn if any calls to functions are eliminated by the optimizations
 enabled by the @option{-ffrontend-optimize} option.
+This option is implied by @option{-Wextra}.
 
 @item -Wrealloc-lhs
 @opindex @code{Wrealloc-lhs}
Index: gcc/fortran/lang.opt
===
--- gcc/fortran/lang.opt	(revision 262563)
+++ gcc/fortran/lang.opt	(working copy)
@@ -250,7 +250,7 @@ Fortran Var(flag_warn_frontend_loop_interchange)
 Warn if loops have been interchanged.
 
 Wfunction-elimination
-Fortran Warning Var(warn_function_elimination)
+Fortran Warning Var(warn_function_elimination) LangEnabledBy(Fortran,Wextra)
 Warn about function call elimination.
 
 Wimplicit-interface
Index: gcc/fortran/resolve.c
===
--- gcc/fortran/resolve.c	(revision 262563)
+++ gcc/fortran/resolve.c	(working copy)
@@ -2982,6 +2982,21 @@ pure_function (gfc_expr *e, const char **name)
 }
 
 
+/* Check if the expression is a reference to an implicitly pure function.  */
+
+static int
+implicit_pure_function (gfc_expr *e)
+{
+  gfc_component *comp = gfc_get_proc_ptr_comp (e);
+  if (comp)
+return gfc_implicit_pure (comp->ts.interface);
+  else if (e->value.function.esym)
+return gfc_implicit_pure (e->value.function.esym);
+  else
+return 0;
+}
+
+
 static bool
 impure_stmt_fcn (gfc_expr *e, gfc_symbol *sym,
 		 int *f ATTRIBUTE_UNUSED)
@@ -3034,7 +3049,8 @@ static bool check_pure_function (gfc_expr *e)
 		 "within a PURE procedure", name, >where);
 	  return false;
 	}
-  gfc_unset_implicit_pure (NULL);
+  if (!implicit_pure_function (e))
+	gfc_unset_implicit_pure (NULL);
 }
   return true;
 }
@@ -3822,6 +3838,40 @@ lookup_uop_fuzzy (const char *op, gfc_sy

Re: [Patch, Fortran] PR 85599: warn about short-circuiting of logical expressions for non-pure functions

[Janus Weil]

Hi Thomas,

>> I tested it on a fairly large code base and found no further false
>> positives. Also it still regtests cleanly. Ok for trunk?
>
>
> while I still disagree with this on principle, I will not stand
> in the way.

IIUC you disagree in the sense that you would like gfortran to have
more such optimizations (with more corresponding warnings). Is that
correct?

I hope you can at least agree that the warnings I'm adding in the
patch are a) useful and b) sufficient for the current state of
optimizations?

Modifying gfortran's runtime behavior is really a separate question
that we can continue to discuss later, probably with some amount of
controversy. But before we even go there, I would really like to have
warnings for the optimizations we have now ...


> However, one point: I think that the warning should be under a separate
> warning, which should then be enabled by -Wextra.
> -Waggressive-function-elimination, could be reused for this,
> or something else

I don't actually see such a flag in the manual.

I do see "-faggressive-function-elimination"
(https://gcc.gnu.org/onlinedocs/gfortran/Code-Gen-Options.html), but
that is about changing runtime behavior, not about throwing warnings.

Do you propose to couple the warning to
-faggressive-function-elimination (and also the short-circuiting
behavior itself)?
Or do you propose to add a flag named -Waggressive-function-elimination?

Cheers,
Janus

Re: [Patch, Fortran] PR 85599: warn about short-circuiting of logical expressions for non-pure functions

[Janus Weil]

Hi all,

here is another update of the patch. It cures the previously-mentioned
problems with generic interfaces, adds some documentation (as
suggested by Dominique) and slightly enhances the error message
(mentioning the impurity of the function we're warning about).

I tested it on a fairly large code base and found no further false
positives. Also it still regtests cleanly. Ok for trunk?

Cheers,
Janus


2018-07-15  Thomas Koenig  
Janus Weil  

PR fortran/85599
* dump-parse-tree.c (show_attr): Add handling of implicit_pure.
* gfortran.texi: Add chapter on evaluation of logical expressions.
* resolve.c (implicit_pure_function): New function.
(check_pure_function): Use it here.
(impure_function_callback): New function.
(resolve_operator): Call it via gfc_expr_walker.


2018-07-15  Janus Weil  

PR fortran/85599
* gfortran.dg/short_circuiting.f90: New test.



2018-07-13 10:02 GMT+02:00 Janus Weil :
> Just noticed another problematic case: Calls to generic interfaces are
> not detected as implicitly pure, see enhanced test case in attachment.
> Will look into this problem on the weekend ...
>
> Cheers,
> Janus
>
> 2018-07-12 21:43 GMT+02:00 Janus Weil :
>> Hi all,
>>
>> here is a minor update of the patch, which cures some problems with
>> implicitly pure functions in the previous version.
>>
>> Most implicitly pure functions were actually detected correctly, but
>> implicitly pure functions that called other implicitly pure functions
>> were not detected properly, and therefore could trigger a warning.
>> This is fixed in the current version, which still regtests cleanly
>> (note that alloc-comp-3.f90 currently fails due to PR 86417). The test
>> case is also enhanced to include the problematic case.
>>
>> Ok for trunk?
>>
>> Cheers,
>> Janus
>>
>>
>>
>> 2018-07-11 23:06 GMT+02:00 Janus Weil :
>>> Hi all,
>>>
>>> after the dust of the heated discussion around this PR has settled a
>>> bit, here is another attempt to implement at least some basic warnings
>>> about compiler-dependent behavior concerning the short-circuiting of
>>> logical expressions.
>>>
>>> As a reminder (and recap of the previous discussion), the Fortran
>>> standard unfortunately is a bit sloppy in this area: It allows
>>> compilers to short-circuit the second operand of .AND. / .OR.
>>> operators, but does not require this. As a result, compilers can do
>>> what they want without conflicting with the standard, and they do:
>>> gfortran does short-circuiting (via TRUTH_ANDIF_EXPR/TRUTH_ORIF_EXPR),
>>> ifort does not.
>>>
>>> I'm continuing here the least-invasive approach of keeping gfortran's
>>> current behavior, but warning about cases where compilers may produce
>>> different results.
>>>
>>> The attached patch is very close to the version I posted previously
>>> (which was already approved by Janne), with the difference that the
>>> warnings are now triggered by -Wextra and not -Wsurprising (which is
>>> included in -Wall), as suggested by Nick Maclaren. I think this is
>>> more reasonable, since not everyone may want to see these warnings.
>>>
>>> Note that I don't want to warn about all possible optimizations that
>>> might be allowed by the standard, but only about those that are
>>> actually problematic in practice and result in compiler-dependent
>>> behavior.
>>>
>>> The patch regtests cleanly on x86_64-linux-gnu. Ok for trunk?
>>>
>>> Cheers,
>>> Janus
>>>
>>>
>>> 2018-07-11  Thomas Koenig  
>>> Janus Weil  
>>>
>>> PR fortran/85599
>>> * dump-parse-tree (show_attr): Add handling of implicit_pure.
>>> * resolve.c (impure_function_callback): New function.
>>> (resolve_operator): Call it vial gfc_expr_walker.
>>>
>>>
>>> 2018-07-11  Janus Weil  
>>>
>>> PR fortran/85599
>>> * gfortran.dg/short_circuiting.f90: New test.
Index: gcc/fortran/dump-parse-tree.c
===
--- gcc/fortran/dump-parse-tree.c	(revision 262563)
+++ gcc/fortran/dump-parse-tree.c	(working copy)
@@ -716,6 +716,8 @@ show_attr (symbol_attribute *attr, const char * mo
 fputs (" ELEMENTAL", dumpfile);
   if (attr->pure)
 fputs (" PURE", dumpfile);
+  if (attr->implicit_pure)
+fputs (" IMPLICIT_PURE", dumpfile);
   if (attr->recursive)
 fpu

Re: [Patch, Fortran] PR 85599: warn about short-circuiting of logical expressions for non-pure functions

[Janus Weil]

Just noticed another problematic case: Calls to generic interfaces are
not detected as implicitly pure, see enhanced test case in attachment.
Will look into this problem on the weekend ...

Cheers,
Janus

2018-07-12 21:43 GMT+02:00 Janus Weil :
> Hi all,
>
> here is a minor update of the patch, which cures some problems with
> implicitly pure functions in the previous version.
>
> Most implicitly pure functions were actually detected correctly, but
> implicitly pure functions that called other implicitly pure functions
> were not detected properly, and therefore could trigger a warning.
> This is fixed in the current version, which still regtests cleanly
> (note that alloc-comp-3.f90 currently fails due to PR 86417). The test
> case is also enhanced to include the problematic case.
>
> Ok for trunk?
>
> Cheers,
> Janus
>
>
>
> 2018-07-11 23:06 GMT+02:00 Janus Weil :
>> Hi all,
>>
>> after the dust of the heated discussion around this PR has settled a
>> bit, here is another attempt to implement at least some basic warnings
>> about compiler-dependent behavior concerning the short-circuiting of
>> logical expressions.
>>
>> As a reminder (and recap of the previous discussion), the Fortran
>> standard unfortunately is a bit sloppy in this area: It allows
>> compilers to short-circuit the second operand of .AND. / .OR.
>> operators, but does not require this. As a result, compilers can do
>> what they want without conflicting with the standard, and they do:
>> gfortran does short-circuiting (via TRUTH_ANDIF_EXPR/TRUTH_ORIF_EXPR),
>> ifort does not.
>>
>> I'm continuing here the least-invasive approach of keeping gfortran's
>> current behavior, but warning about cases where compilers may produce
>> different results.
>>
>> The attached patch is very close to the version I posted previously
>> (which was already approved by Janne), with the difference that the
>> warnings are now triggered by -Wextra and not -Wsurprising (which is
>> included in -Wall), as suggested by Nick Maclaren. I think this is
>> more reasonable, since not everyone may want to see these warnings.
>>
>> Note that I don't want to warn about all possible optimizations that
>> might be allowed by the standard, but only about those that are
>> actually problematic in practice and result in compiler-dependent
>> behavior.
>>
>> The patch regtests cleanly on x86_64-linux-gnu. Ok for trunk?
>>
>> Cheers,
>> Janus
>>
>>
>> 2018-07-11  Thomas Koenig  
>> Janus Weil  
>>
>> PR fortran/85599
>> * dump-parse-tree (show_attr): Add handling of implicit_pure.
>> * resolve.c (impure_function_callback): New function.
>> (resolve_operator): Call it vial gfc_expr_walker.
>>
>>
>> 2018-07-11  Janus Weil  
>>
>> PR fortran/85599
>> * gfortran.dg/short_circuiting.f90: New test.
! { dg-do compile }
! { dg-additional-options "-Wextra" }
!
! PR 85599: warn about short-circuiting of logical expressions for non-pure functions
!
! Contributed by Janus Weil 

module a

   interface impl_pure_a
  module procedure impl_pure_a1
   end interface

contains

logical function impl_pure_a1()
  impl_pure_a1 = .true.
   end function

end module


program short_circuit

   use a

   logical :: flag
   flag = .false.
   flag = check() .and. flag
   flag = flag .and. check()! { dg-warning "might not be evaluated" }
   flag = flag .and. pure_check()
   flag = flag .and. impl_pure_1()
   flag = flag .and. impl_pure_2()
   flag = flag .and. impl_pure_a1()
   flag = flag .and. impl_pure_a()  ! bogus warning here

contains

   logical function check()
  integer, save :: i = 1
  print *, "check", i
  i = i + 1
  check = .true.
   end function

   logical pure function pure_check()
  pure_check = .true.
   end function

   logical function impl_pure_1()
  impl_pure_1 = .true.
   end function

   logical function impl_pure_2()
  impl_pure_2 = impl_pure_1()
   end function


end

Re: [Patch, Fortran] PR 85599: warn about short-circuiting of logical expressions for non-pure functions

[Janus Weil]

2018-07-12 21:53 GMT+02:00 Thomas Koenig :
> Hi Janus,
>
>> The cleaner approach would certainly be to avoid short-circuiting of
>> impure functions altogether. If we can all agree that this is a good
>> idea,
>
>
> This is a fine example of logical short-circuiting - the condition you
> mention is false, therefore the rest need not be evaluated :-)

Thought so :(

Also: Yay, the discussion reached the first meta level ;)

Since you seem to be capable of giving warnings about
short-circuiting, we only need to teach that trick to gfortran now
...!

Cheers,
Janus

Re: [Patch, Fortran] PR 85599: warn about short-circuiting of logical expressions for non-pure functions

[Janus Weil]

Hi all,

here is a minor update of the patch, which cures some problems with
implicitly pure functions in the previous version.

Most implicitly pure functions were actually detected correctly, but
implicitly pure functions that called other implicitly pure functions
were not detected properly, and therefore could trigger a warning.
This is fixed in the current version, which still regtests cleanly
(note that alloc-comp-3.f90 currently fails due to PR 86417). The test
case is also enhanced to include the problematic case.

Ok for trunk?

Cheers,
Janus



2018-07-11 23:06 GMT+02:00 Janus Weil :
> Hi all,
>
> after the dust of the heated discussion around this PR has settled a
> bit, here is another attempt to implement at least some basic warnings
> about compiler-dependent behavior concerning the short-circuiting of
> logical expressions.
>
> As a reminder (and recap of the previous discussion), the Fortran
> standard unfortunately is a bit sloppy in this area: It allows
> compilers to short-circuit the second operand of .AND. / .OR.
> operators, but does not require this. As a result, compilers can do
> what they want without conflicting with the standard, and they do:
> gfortran does short-circuiting (via TRUTH_ANDIF_EXPR/TRUTH_ORIF_EXPR),
> ifort does not.
>
> I'm continuing here the least-invasive approach of keeping gfortran's
> current behavior, but warning about cases where compilers may produce
> different results.
>
> The attached patch is very close to the version I posted previously
> (which was already approved by Janne), with the difference that the
> warnings are now triggered by -Wextra and not -Wsurprising (which is
> included in -Wall), as suggested by Nick Maclaren. I think this is
> more reasonable, since not everyone may want to see these warnings.
>
> Note that I don't want to warn about all possible optimizations that
> might be allowed by the standard, but only about those that are
> actually problematic in practice and result in compiler-dependent
> behavior.
>
> The patch regtests cleanly on x86_64-linux-gnu. Ok for trunk?
>
> Cheers,
> Janus
>
>
> 2018-07-11  Thomas Koenig  
> Janus Weil  
>
> PR fortran/85599
> * dump-parse-tree (show_attr): Add handling of implicit_pure.
> * resolve.c (impure_function_callback): New function.
> (resolve_operator): Call it vial gfc_expr_walker.
>
>
> 2018-07-11  Janus Weil  
>
> PR fortran/85599
> * gfortran.dg/short_circuiting.f90: New test.
Index: gcc/fortran/dump-parse-tree.c
===
--- gcc/fortran/dump-parse-tree.c	(revision 262563)
+++ gcc/fortran/dump-parse-tree.c	(working copy)
@@ -716,6 +716,8 @@ show_attr (symbol_attribute *attr, const char * mo
 fputs (" ELEMENTAL", dumpfile);
   if (attr->pure)
 fputs (" PURE", dumpfile);
+  if (attr->implicit_pure)
+fputs (" IMPLICIT_PURE", dumpfile);
   if (attr->recursive)
 fputs (" RECURSIVE", dumpfile);
 
Index: gcc/fortran/resolve.c
===
--- gcc/fortran/resolve.c	(revision 262563)
+++ gcc/fortran/resolve.c	(working copy)
@@ -2982,6 +2982,19 @@ pure_function (gfc_expr *e, const char **name)
 }
 
 
+static int
+implicit_pure_function (gfc_expr *e)
+{
+  gfc_component *comp = gfc_get_proc_ptr_comp (e);
+  if (comp)
+return gfc_implicit_pure (comp->ts.interface);
+  else if (e->value.function.esym)
+return gfc_implicit_pure (e->value.function.esym);
+  else
+return 0;
+}
+
+
 static bool
 impure_stmt_fcn (gfc_expr *e, gfc_symbol *sym,
 		 int *f ATTRIBUTE_UNUSED)
@@ -3034,7 +3047,8 @@ static bool check_pure_function (gfc_expr *e)
 		 "within a PURE procedure", name, >where);
 	  return false;
 	}
-  gfc_unset_implicit_pure (NULL);
+  if (!implicit_pure_function (e))
+	gfc_unset_implicit_pure (NULL);
 }
   return true;
 }
@@ -3822,6 +3836,46 @@ lookup_uop_fuzzy (const char *op, gfc_symtree *uop
 }
 
 
+/* Callback finding an impure function as an operand to an .and. or
+   .or.  expression.  Remember the last function warned about to
+   avoid double warnings when recursing.  */
+
+static int
+impure_function_callback (gfc_expr **e, int *walk_subtrees ATTRIBUTE_UNUSED,
+			  void *data)
+{
+  gfc_expr *f = *e;
+  const char *name;
+  static gfc_expr *last = NULL;
+  bool *found = (bool *) data;
+
+  if (f->expr_type == EXPR_FUNCTION)
+{
+  *found = 1;
+  if (f != last && !pure_function (f, ))
+	{
+	  /* This could still be a function without side effects, i.e.
+	 implicit pure.  Do not warn for that case.  */
+	  if (f->symtree == NULL || f->symtree->n.sym == NULL
+	  || !gfc_implicit_pure (f->symtree->n.

Re: [Patch, Fortran] PR 85599: warn about short-circuiting of logical expressions for non-pure functions

[Janus Weil]

2018-07-12 16:35 GMT+02:00 Dominique d'Humières :
>
 after the dust of the heated discussion around this PR has settled a
 bit, here is another attempt to implement at least some basic warnings
 about compiler-dependent behavior concerning the short-circuiting of
 logical expressions. …
>>>
>>> IMO your patch is missing the only point I agree with you on this issue, 
>>> i.e.,
>>
>> So you don't agree that it's a good idea to warn about non-portable code?
>
> Wel, as you might guess I don’t like warnings in general: too much false 
> positives
> and missed targets (see recent bootstrap breakage).

So then: Do you have a better solution for the problem at hand?

The cleaner approach would certainly be to avoid short-circuiting of
impure functions altogether. If we can all agree that this is a good
idea, I'll be happy to submit a patch in that direction. But recent
discussions have shown that there is a broad opposition of hardliners
who favor optimization to portability. I don't really have the nerves
and the time to continue this endless fight, so for now I would be
satisfied if we at least had the warnings.


>>> What to do with
>>>
>>> if(x>0 .and. sort(x)<10.0*log(x)) …
>>>
>>> which is not portable to compilers computing the two expressions?
>>
>> That's one of the typical cases that the patch should be able to
>> handle. If 'sort' is impure, this should trigger a warning with the
>> patch. If it is pure, it doesn't matter whether the rhs of the .and.
>> operator is executed (since there are no side effects). Or am I
>> missing something?
>
> I meant ‘sqrt’ (changed behind my back by the stupid spell-checker).

sqrt and log are both elemental, thus pure, so there will be no warning.

Cheers,
Janus

Re: [Patch, Fortran] PR 85599: warn about short-circuiting of logical expressions for non-pure functions

[Janus Weil]

Hi Dominique,

thanks for the comments.

>> after the dust of the heated discussion around this PR has settled a
>> bit, here is another attempt to implement at least some basic warnings
>> about compiler-dependent behavior concerning the short-circuiting of
>> logical expressions. …
>
> IMO your patch is missing the only point I agree with you on this issue, i.e.,

So you don't agree that it's a good idea to warn about non-portable code?


> the short-circuit evaluation and the related portability issues should be
> documented.

I fully agree. Documentation never hurts. Do you have a suggestion
where this should go? There does not seem to be a particular chapter
in the manual that deals with portability across compilers (probably
because one usually assumes that sticking to the Fortran standard is
sufficient for achieving portability). The best match might be the
chapter on compiler characteristics:

https://gcc.gnu.org/onlinedocs/gcc-8.1.0/gfortran/Compiler-Characteristics.html


> With your patch what happens if check() is an external function?

Haven't tried, but I guess you would get a warning, since the compiler
cannot tell whether 'check' is pure (unless you have an explicit
interface block that contains the PURE attribute).


> Your patch is focusing on pr85599 and ignore pr57160.

Well, yes. PR 57160 is somewhat related, but orthogonal. It concerns
essentially the same issue, namely short-circuiting with logical
expressions, but proposes a different solution (making
short-circuiting depend on the -ffrontend-optimize flag). I wouldn't
say that's unreasonable, but independent of that I might still want to
know that my code is not portable, thus a warning makes sense in any
case (IMHO).


> What to do with
>
> if(x>0 .and. sort(x)<10.0*log(x)) …
>
> which is not portable to compilers computing the two expressions?

That's one of the typical cases that the patch should be able to
handle. If 'sort' is impure, this should trigger a warning with the
patch. If it is pure, it doesn't matter whether the rhs of the .and.
operator is executed (since there are no side effects). Or am I
missing something?

Cheers,
Janus


2018-07-12 13:16 GMT+02:00 Dominique d'Humières :
> Hi Janus,
>
>> after the dust of the heated discussion around this PR has settled a
>> bit, here is another attempt to implement at least some basic warnings
>> about compiler-dependent behavior concerning the short-circuiting of
>> logical expressions. …
>
> IMO your patch is missing the only point I agree with you on this issue, i.e.,
> the short-circuit evaluation and the related portability issues should be
> documented.
>
> With your patch what happens if check() is an external function?
>
> Your patch is focusing on pr85599 and ignore pr57160.
>
> What to do with
>
> if(x>0 .and. sort(x)<10.0*log(x)) …
>
> which is not portable to compilers computing the two expressions?
>
> Cheers,
>
> Dominique
>

[Patch, Fortran] PR 85599: warn about short-circuiting of logical expressions for non-pure functions

[Janus Weil]

Hi all,

after the dust of the heated discussion around this PR has settled a
bit, here is another attempt to implement at least some basic warnings
about compiler-dependent behavior concerning the short-circuiting of
logical expressions.

As a reminder (and recap of the previous discussion), the Fortran
standard unfortunately is a bit sloppy in this area: It allows
compilers to short-circuit the second operand of .AND. / .OR.
operators, but does not require this. As a result, compilers can do
what they want without conflicting with the standard, and they do:
gfortran does short-circuiting (via TRUTH_ANDIF_EXPR/TRUTH_ORIF_EXPR),
ifort does not.

I'm continuing here the least-invasive approach of keeping gfortran's
current behavior, but warning about cases where compilers may produce
different results.

The attached patch is very close to the version I posted previously
(which was already approved by Janne), with the difference that the
warnings are now triggered by -Wextra and not -Wsurprising (which is
included in -Wall), as suggested by Nick Maclaren. I think this is
more reasonable, since not everyone may want to see these warnings.

Note that I don't want to warn about all possible optimizations that
might be allowed by the standard, but only about those that are
actually problematic in practice and result in compiler-dependent
behavior.

The patch regtests cleanly on x86_64-linux-gnu. Ok for trunk?

Cheers,
Janus


2018-07-11  Thomas Koenig  
Janus Weil  

PR fortran/85599
* dump-parse-tree (show_attr): Add handling of implicit_pure.
* resolve.c (impure_function_callback): New function.
(resolve_operator): Call it vial gfc_expr_walker.


2018-07-11  Janus Weil  

PR fortran/85599
* gfortran.dg/short_circuiting.f90: New test.
Index: gcc/fortran/dump-parse-tree.c
===
--- gcc/fortran/dump-parse-tree.c	(revision 262563)
+++ gcc/fortran/dump-parse-tree.c	(working copy)
@@ -716,6 +716,8 @@ show_attr (symbol_attribute *attr, const char * mo
 fputs (" ELEMENTAL", dumpfile);
   if (attr->pure)
 fputs (" PURE", dumpfile);
+  if (attr->implicit_pure)
+fputs (" IMPLICIT_PURE", dumpfile);
   if (attr->recursive)
 fputs (" RECURSIVE", dumpfile);
 
Index: gcc/fortran/resolve.c
===
--- gcc/fortran/resolve.c	(revision 262563)
+++ gcc/fortran/resolve.c	(working copy)
@@ -3822,6 +3822,46 @@ lookup_uop_fuzzy (const char *op, gfc_symtree *uop
 }
 
 
+/* Callback finding an impure function as an operand to an .and. or
+   .or.  expression.  Remember the last function warned about to
+   avoid double warnings when recursing.  */
+
+static int
+impure_function_callback (gfc_expr **e, int *walk_subtrees ATTRIBUTE_UNUSED,
+			  void *data)
+{
+  gfc_expr *f = *e;
+  const char *name;
+  static gfc_expr *last = NULL;
+  bool *found = (bool *) data;
+
+  if (f->expr_type == EXPR_FUNCTION)
+{
+  *found = 1;
+  if (f != last && !pure_function (f, ))
+	{
+	  /* This could still be a function without side effects, i.e.
+	 implicit pure.  Do not warn for that case.  */
+	  if (f->symtree == NULL || f->symtree->n.sym == NULL
+	  || !gfc_implicit_pure (f->symtree->n.sym))
+	{
+	  if (name)
+		gfc_warning (OPT_Wextra,
+			 "Function %qs at %L might not be evaluated",
+			 name, >where);
+	  else
+		gfc_warning (OPT_Wextra,
+			 "Function at %L might not be evaluated",
+			 >where);
+	}
+	}
+  last = f;
+}
+
+  return 0;
+}
+
+
 /* Resolve an operator expression node.  This can involve replacing the
operation with a user defined function call.  */
 
@@ -3930,6 +3970,14 @@ resolve_operator (gfc_expr *e)
 	gfc_convert_type (op1, >ts, 2);
 	  else if (op2->ts.kind < e->ts.kind)
 	gfc_convert_type (op2, >ts, 2);
+
+	  if (e->value.op.op == INTRINSIC_AND || e->value.op.op == INTRINSIC_OR)
+	{
+	  /* Warn about short-circuiting
+	 with impure function as second operand.  */
+	  bool op2_f = false;
+	  gfc_expr_walker (, impure_function_callback, _f);
+	}
 	  break;
 	}
 
! { dg-do compile }
! { dg-additional-options "-Wextra" }
!
! PR 85599: warn about short-circuiting of logical expressions for non-pure functions
!
! Contributed by Janus Weil 

program short_circuit

   logical :: flag
   flag = .false.
   flag = check() .and. flag
   flag = flag .and. check()  ! { dg-warning "might not be evaluated" }
   flag = flag .and. pure_check()

contains

   logical function check()
  integer, save :: i = 1
  print *, "check", i
  i = i + 1
  check = .true.
   end function

   logical pure function pure_check()
  pure_check = .true.
   end function

end

Re: [patch, fortran] Handling of .and. and .or. expressions

[Janus Weil]

2018-06-29 9:28 GMT+02:00 Jakub Jelinek :
> On Thu, Jun 28, 2018 at 07:36:56PM -0700, Steve Kargl wrote:
>> === gfortran Summary ===
>>
>> # of expected passes47558
>> # of unexpected failures6
>> # of expected failures  104
>> # of unsupported tests  85
>>
>> FAIL: gfortran.dg/actual_pointer_function_1.f90   -O0  execution test
>> FAIL: gfortran.dg/actual_pointer_function_1.f90   -O1  execution test
>> FAIL: gfortran.dg/actual_pointer_function_1.f90   -O2  execution test
>> FAIL: gfortran.dg/actual_pointer_function_1.f90   -O3 -fomit-frame-pointer 
>> -funroll-loops -fpeel-loops -ftracer -finline-functions  execution test
>> FAIL: gfortran.dg/actual_pointer_function_1.f90   -O3 -g  execution test
>> FAIL: gfortran.dg/actual_pointer_function_1.f90   -Os  execution test
>>
>> Execution timeout is: 300
>> spawn [open ...]
>>
>> Program received signal SIGSEGV: Segmentation fault - invalid memory 
>> reference.
>>
>> Backtrace for this error:
>> #0  0x71a2 in ???
>> #1  0x400c09 in ???
>> #2  0x400b91 in ???
>> #3  0x400c51 in ???
>> #4  0x400854 in _start
>> at /usr/src/lib/csu/amd64/crt1.c:74
>> #5  0x200627fff in ???
>
> If you have a test that is broken by the TRUTH_ANDIF_EXPR -> TRUTH_AND_EXPR
> change, then the test must be broken, because from the snippets that were
> posted, Fortran does not require (unlike C/C++) that the second operand is
> not evaluated if the first one evaluates to false for (and) or true (for
> or), it just allows it.

Exactly.


> So, the optimizing away of the function calls should be an optimization, and
> as such should be done only when optimizing.  So for -O0 at least always use
> TRUTH_{AND,OR}_EXPR, so that people can actually make sure that their
> programs are valid Fortran and can also step into those functions when
> debugging.  For -O1 and higher perhaps use temporarily the *IF_EXPR, or
> better, as I said in another mail, let's add an attribute that will optimize
> all the calls that can be optimized, not just one special case.

Thanks for the comments, Jakub. I fully agree. This is pretty much the
sanest strategy I've heard so far in all of this monstrous thread and
I definitely support it.

Cheers,
Janus

Re: [patch, fortran] Handling of .and. and .or. expressions

[Janus Weil]

2018-06-28 18:22 GMT+02:00 Steve Kargl :
> On Thu, Jun 28, 2018 at 05:52:43PM +0200, Janus Weil wrote:
>> 2018-06-28 16:41 GMT+02:00 Steve Kargl :
>> >> > Technically, the standard says an operand need not be evaluate,
>> >> > but you've asked people not to cite the Standard.  I've also
>> >> > pointed you to F2018 Note 10.28 where it very clearly says that
>> >> > a function need not be evaluated with example nearly identical
>> >> > to the one in the PR.  PURE vs IMPURE (or unmarked) function
>> >> > is a red herring.  The standard makes no distinction.
>> >>
>> >> Look, Steve. This argument has been going in circles for weeks now. I
>> >> think we can stop it here.
>> >>
>> >
>> > I've already stated that I have no problem with the warning.  As
>> > Thomas noted, gfortran should warn for both '.false. .and. check()'
>> > and 'check() .and. .false.'
>>
>> It doesn't really help the discussion if you just re-state other
>> people's positions. It might help if you would actually tell use *why*
>> you think both cases should be checked?
>>
>> gfortran's current implementation of .and. is intrinsically asymmetric
>> and only optimizes away the second operand if possible. My motivation
>> for the warning is mostly to signal compiler-dependent behavior, and I
>> still haven't seen a compiler that treats the case 'check() .and.
>> .false.' different from gfortran. Are you aware of one?
>
> Why I think it a warning should be emitted:  symmetry!.
>
> You complained about the lack of symmetry in 'check() .and. .false.'
> and '.false. .and. check()'.

well, my original complaint in PR85599 was that the second one is
broken, and your reaction to that is to break the first one as well.


>> > In fact, I'll be submitting a bug report for a missed optimization.
>> >
>> > subroutine foo(x,y)   subroutine foo(x,y)
>> >real x(10), y(10) real x(10), y(10)
>> >y = 0*sin(x)  y = 0
>> > end subroutine fooend subroutine foo
>> >
>> > .L2:  pxor%xmm0, %xmm0
>> >callsinf   movq$0, 32(%rsi)
>> >pxor%xmm1, %xmm1   movups  %xmm0, (%rsi)
>> >mulss   %xmm1, %xmm0   movups  %xmm0, 16(%rsi)
>> >movss   %xmm0, 0(%rbp,%rbx)
>> >addq$4, %rbx
>> >cmpq$40, %rbx
>> >jne .L2
>> >
>> > which I'm sure you'll just be thrilled with.
>>
>> I can't say I'm totally thrilled with it, but, yes, I do agree it's a
>> missed optimization. That probably comes as a surprise to you, since
>> you are apparently trying to tease me in some way here (didn't work).
>> After all, SIN is an elemental function, thus pure and without any
>> side effects. The call can certainly be removed if the value is not
>> needed. Please submit your bug report, but please don't put me in CC.
>
> Change sin(x) to my_function_with_side_effects() if like.  It
> is a missed optimization regardless of the function's pureness.

Yes, if you're an orthodox believer in The One True Standard. I'd
rather use my own brain cells now and then.

In this case my brain just tells me that it's not a good idea to apply
an optimization that can totally change the results of my code, and
that it's not a good idea for a 'standard' to not define the semantics
of an operator / expression, but instead leave it to the compiler how
it should be evaluated.

You insist that the standard does not forbid this optimization. The
standard also does not forbid explicitly that you shoot yourself it
the foot with a nuclear missile. So, you go ahead and shoot. Then
someone comes along and asks why you did that, and you reply: "The
standard did not forbid it."

One thing that I always failed to comprehend is people's fixation on
optimization. What's so great about your code running 0.1% faster if
the second compiler you try gives you totally different results, with
no hints whether it's your code that's broken, or the first compiler,
or the second one, or the standard that both compilers tried to
implement? With a ten-line code that might not be such a big problem,
but above 100.000 loc or so it can quickly become a huge issue.

Cheers,
Janus

Re: [patch, fortran] Handling of .and. and .or. expressions

[Janus Weil]

2018-06-28 16:41 GMT+02:00 Steve Kargl :
>> > Technically, the standard says an operand need not be evaluate,
>> > but you've asked people not to cite the Standard.  I've also
>> > pointed you to F2018 Note 10.28 where it very clearly says that
>> > a function need not be evaluated with example nearly identical
>> > to the one in the PR.  PURE vs IMPURE (or unmarked) function
>> > is a red herring.  The standard makes no distinction.
>>
>> Look, Steve. This argument has been going in circles for weeks now. I
>> think we can stop it here.
>>
>
> I've already stated that I have no problem with the warning.  As
> Thomas noted, gfortran should warn for both '.false. .and. check()'
> and 'check() .and. .false.'

It doesn't really help the discussion if you just re-state other
people's positions. It might help if you would actually tell use *why*
you think both cases should be checked?

gfortran's current implementation of .and. is intrinsically asymmetric
and only optimizes away the second operand if possible. My motivation
for the warning is mostly to signal compiler-dependent behavior, and I
still haven't seen a compiler that treats the case 'check() .and.
.false.' different from gfortran. Are you aware of one?


> I am opposed to the change of TRUTH_AND_EXPR to TRUTH_ANDIF_EXPR,
> where you are special casing logical expressions that involve
> .and. and .or.

It's the other way around. We currently have TRUTH_ANDIF_EXPR. And no
one really wants to change it right now. Let's move on.


> In fact, I'll be submitting a bug report for a missed optimization.
>
> subroutine foo(x,y)   subroutine foo(x,y)
>real x(10), y(10) real x(10), y(10)
>y = 0*sin(x)  y = 0
> end subroutine fooend subroutine foo
>
> .L2:  pxor%xmm0, %xmm0
>callsinf   movq$0, 32(%rsi)
>pxor%xmm1, %xmm1   movups  %xmm0, (%rsi)
>mulss   %xmm1, %xmm0   movups  %xmm0, 16(%rsi)
>movss   %xmm0, 0(%rbp,%rbx)
>addq$4, %rbx
>cmpq$40, %rbx
>jne .L2
>
> which I'm sure you'll just be thrilled with.

I can't say I'm totally thrilled with it, but, yes, I do agree it's a
missed optimization. That probably comes as a surprise to you, since
you are apparently trying to tease me in some way here (didn't work).
After all, SIN is an elemental function, thus pure and without any
side effects. The call can certainly be removed if the value is not
needed. Please submit your bug report, but please don't put me in CC.

Thanks,
Janus

Re: [patch, fortran] Handling of .and. and .or. expressions

[Janus Weil]

2018-06-28 13:05 GMT+02:00 N.M. Maclaren :
> On Jun 28 2018, Janus Weil wrote:
>>
>>
>>> if we add a warning, we should add it both for
>>>
>>> if (flag .and. func())
>>> and for
>>> if (func() .and. flag)
>>>
>>> because the standard also allows reversing the test (which my
>>> original test did).
>>
>>
>> well, I really don't want to warn for hypothetical problems. Since we
>> currently do not apply such an optimization, we should not warn about
>> it (unless you find any other compiler which actually does).
>
>
> I have used such compilers; they used to be fairly common and may still be.

You mean compilers which transform "if (func() .and. flag)" into "if
(flag .and. func())", and then possibly remove the call?
If yes, could you tell us which compilers you are talking about specifically?


> I agree with Thomas, and think it should be in -Wextra.  -Wsurprising is in
> -Wall, and this is done in so many programs (usually with not-pure functions
> that are actually pure, or effectively so) that the number of warnings would
> put people off using -Wall.

I agree that -Wextra might be more suitable than -Wall.

Btw, effectively pure functions which are not marked with the PURE
attribute should hopefully not be a problem, since gfortran has
implicit_pure detection that should deal with that. (However, it might
need some further improvements, I think.)

Regarding warnings for "if (func() .and. flag)", I would like to have
a very concrete reason, otherwise it will not be very useful. Which
other compilers optimize this call away?

Thanks for the constructive comments!

Cheers,
Janus

Re: [patch, fortran] Handling of .and. and .or. expressions

[Janus Weil]

Hi Thomas,

> if we add a warning, we should add it both for
>
> if (flag .and. func())
>
> and for
>
> if (func() .and. flag)
>
> because the standard also allows reversing the test (which my
> original test did).

well, I really don't want to warn for hypothetical problems. Since we
currently do not apply such an optimization, we should not warn about
it (unless you find any other compiler which actually does).

Whether to add this optimization in the future is a different topic of
course, and can still be discussed later.

Cheers,
Janus

Re: [patch, fortran] Handling of .and. and .or. expressions

[Janus Weil]

2018-06-27 23:47 GMT+02:00 Steve Kargl :
> On Wed, Jun 27, 2018 at 10:46:05PM +0200, Janus Weil wrote:
>> 2018-06-27 21:34 GMT+02:00 Thomas Koenig :
>> >
>> > And we are not going to change Fortran semantics. And I also oppose
>> > defining our own subset of Fortran in the hope that people will make
>> > fewer mistakes.
>> >
>> > A function is something that produces a value.  If the value is not
>> > needed to produce the correct result, the function need not be called.
>>
>> That's a bit oversimplified, isn't it?
>
> Technically, the standard says an operand need not be evaluate,
> but you've asked people not to cite the Standard.  I've also
> pointed you to F2018 Note 10.28 where it very clearly says that
> a function need not be evaluated with example nearly identical
> to the one in the PR.  PURE vs IMPURE (or unmarked) function
> is a red herring.  The standard makes no distinction.

Look, Steve. This argument has been going in circles for weeks now. I
think we can stop it here.

So, again: I know that the Fortran standard allows the
short-circuiting of impure functions. I'm not trying to change that
behavior. (I don't like it but I certainly have to accept it.)

But that still leaves us with a problem: The standard allows the
short-circuiting but it doesn't require it. Meaning that any other
compiler that does not do it (like ifort) is not in conflict with the
standard either.

Many people may want to avoid such compiler-dependent behavior. That's
why we need a warning. All of the discussion here has shown that using
impure functions in logical expressions is not a good idea in Fortran.
It is not illegal, but it should be considered bad style. That's why
we need a warning:

https://gcc.gnu.org/ml/fortran/2018-06/msg00160.html

Does anyone agree with me that this is a useful diagnostic? Whether
this warning should be included in -Wall or -Wextra or -Whereever can
be debated. Is this patch ok for trunk?

Cheers,
Janus


2018-06-27 23:47 GMT+02:00 Steve Kargl :
> On Wed, Jun 27, 2018 at 10:46:05PM +0200, Janus Weil wrote:
>> 2018-06-27 21:34 GMT+02:00 Thomas Koenig :
>> >
>> > And we are not going to change Fortran semantics. And I also oppose
>> > defining our own subset of Fortran in the hope that people will make
>> > fewer mistakes.
>> >
>> > A function is something that produces a value.  If the value is not
>> > needed to produce the correct result, the function need not be called.
>>
>> That's a bit oversimplified, isn't it?
>
> Technically, the standard says an operand need not be evaluate,
> but you've asked people not to cite the Standard.  I've also
> pointed you to F2018 Note 10.28 where it very clearly says that
> a function need not be evaluated with example nearly identical
> to the one in the PR.  PURE vs IMPURE (or unmarked) function
> is a red herring.  The standard makes no distinction.
>
> --
> Steve

Re: [patch, fortran] Handling of .and. and .or. expressions

[Janus Weil]

2018-06-27 15:43 GMT+02:00 N.M. Maclaren :
> On Jun 27 2018, Janus Weil wrote:
>>
>>
>> It is very unfortunate, and it means that the Fortran standard simply
>> does not provide a measure for "more correct" here. (My common-sense
>> drop-in notion of correctness would be that an optimization is
>> 'correct' as long as it can be proven to not change any results.)
>
>
> Actually, yes, it does.  Fortran 2008 7.1.4p2, 7.1.5.2.4p2, 7.1.5.3.2p1,
> 7.1.5.4.2p1, 7.1.5.5.2p1 and 7.1.6.3p1/2.

I'm not completely sure what you deduce from all these quoted
paragraphs, but applied to the cases we're discussing here, e.g.

A = .false. .and. my_boldly_impure_function()

I read them as saying that a compiler does not have to invoke the
function if it doesn't feel like it, but with no definite obligation
to remove the call either. Do you agree?


> The problem is that the Fortran
> standard has never resolved the inconsistency that OTHER side-effects in
> functions are not explicitly forbidden

Exactly.


> it's one of the few places in
> Fortran where there are conforming constructs that do not have specified
> semantics or at least a clearly specified intention.

And I really wonder why one would do such a thing. Like, ever.

I'm curious: Are there actually other such cases in Fortran that
you're aware of?


> Many, many people have tried to sort that out, and all have failed dismally.
> I regret that the proposal to deprecate impure functions did not succeed,
> because at least that would have made one sane direction clear.

I don't know. Why do so many people in the Fortran community have such
a big problem with functions? Ok, they haven't been around in 77, but
if you even bother to introduce them, why do it so half-heartedly and
not just embrace them in their full beauty?

What is so complicated about putting a statement into the Fortran
standard that says: "Ok, if this function has side effects, we
definitely must evaluate it, otherwise we lose the side effects. They
might be important."  ???


> This has been made much worse by IEEE 754 and (worse) coarray support. Like
> almost all languages with threading and shared-memory, Fortran's concept of
> pure is satisfactory for single-threaded code but not parallel. You need the
> stronger concept, where a pure function does not depend on anything that
> might change in its scope.

We don't even need to start discussing these things if we can't even
figure out what to do with an impure function in a simple
single-threaded program.


> Look on the bright side.  If you thought this problem is a mess (it is), I
> can assure you that most other languages get it far, far worse - often with
> logically impossible or contradictory promises and requirements.

I don't quite see that. Which languages are you talking about
specifically? I see that C/C++ gives you a nice choice of writing

A = false & my_boldly_impure_function()

or

A = false && my_boldly_impure_function()

where the first option guarantees that your function is evaluated,
while the second one allows (or even demands?) it to be optimized
away. That's a much clearer situation than the Fortran mess, isn't it?


My wish would be that instead of deprecating impure functions, the
Fortran standard should rather introduce .ANDIF. / .ORELSE. operators.


Cheers,
Janus

Re: [patch, fortran] Handling of .and. and .or. expressions

[Janus Weil]

2018-06-27 10:09 GMT+02:00 Janne Blomqvist :
> On Wed, Jun 27, 2018 at 10:52 AM, Janus Weil  wrote:
>>
>> 2018-06-27 9:42 GMT+02:00 Jakub Jelinek :
>> > On Wed, Jun 27, 2018 at 09:35:59AM +0200, Janus Weil wrote:
>> >> > and have some non-default aggressive
>> >> > optimization option that would optimize away in the FE impure
>> >> > function calls
>> >>
>> >> IMHO an optimization to remove functions calls is unproblematic only
>> >> for pure functions, but as long as it is guarded by a non-default
>> >
>> > For pure functions, which are hopefully marked as ECF_PURE for the
>> > middle-end
>>
>> Not sure if that's the case. Grepping for ECF_PURE in
>> trunk/gcc/fortran only yields a single occurrence:
>>
>> f95-lang.c:#define ATTR_PURE_NOTHROW_LEAF_LIST(ECF_NOTHROW |
>> ECF_LEAF | ECF_PURE)
>>
>> Seems like that is only used for built-ins?
>>
>>
>> > the middle-end can already optimize away those calls where the
>> > result isn't needed.
>>
>> Ah, great. Even with TRUTH_AND_EXPR, you mean?
>
> How about committing a patch changing to use TRUTH_{AND|OR}_EXPR, and then
> check benchmark results (polyhedron, spec etc.)? If performance worsens, we
> revert, if it improves, great, lets keep it?

I would definitely support that. I cannot imagine that it will have a
large impact on benchmarks, but it's certainly a good idea to check.
(After all: ifort, which is usually perceived as being slightly ahead
of GCC in terms of performance, does not do this kind of
short-circuiting either.)

If the middle-end already does all 'non-problematic' optimizations by
itself, all the better.


> To clarify, I'm not objecting to using TRUTH_{AND|OR}_EXPR, I'm objecting to
> the notion that this is somehow "more correct" or "less bad" than the
> current short-circuiting. The Fortran standard does not specify an
> evaluation strategy; IMHO very unfortunately, but it is what it is.

It is very unfortunate, and it means that the Fortran standard simply
does not provide a measure for "more correct" here. (My common-sense
drop-in notion of correctness would be that an optimization is
'correct' as long as it can be proven to not change any results.)

Cheers,
Janus

Re: [patch, fortran] Handling of .and. and .or. expressions

[Janus Weil]

2018-06-27 9:42 GMT+02:00 Jakub Jelinek :
> On Wed, Jun 27, 2018 at 09:35:59AM +0200, Janus Weil wrote:
>> > and have some non-default aggressive
>> > optimization option that would optimize away in the FE impure function 
>> > calls
>>
>> IMHO an optimization to remove functions calls is unproblematic only
>> for pure functions, but as long as it is guarded by a non-default
>
> For pure functions, which are hopefully marked as ECF_PURE for the
> middle-end

Not sure if that's the case. Grepping for ECF_PURE in
trunk/gcc/fortran only yields a single occurrence:

f95-lang.c:#define ATTR_PURE_NOTHROW_LEAF_LIST(ECF_NOTHROW |
ECF_LEAF | ECF_PURE)

Seems like that is only used for built-ins?


> the middle-end can already optimize away those calls where the
> result isn't needed.

Ah, great. Even with TRUTH_AND_EXPR, you mean?

Cheers,
Janus

Re: [patch, fortran] Handling of .and. and .or. expressions

[Janus Weil]

2018-06-27 8:15 GMT+02:00 Jakub Jelinek :
> On Wed, Jun 27, 2018 at 07:52:59AM +0200, Janus Weil wrote:
>> >> with your patch, we would only warn about
>> >>
>> >> var .and. func()
>> >>
>> >> and not about
>> >>
>> >> func() .and. var.
>> >>
>> >> AFAIK, TRUTH_AND_EXPR does not guarantee that func() will
>> >> always be exectued, or if it does, I have not seen the
>> >> documentation; it just happens to match what we currently
>> >> see (which may be due to missed optimizatins in the back end,
>> >> or the lack of test cases exposing this).
>> >
>> > If you are talking about what the middle-end does, TRUTH_AND_EXPR
>> > always evaluates side-effects in both operands, while for
>> > TRUTH_ANDIF_EXPR it evaluates the second operand only if the first operand
>> > is not false.
>>
>> thanks for the comments. Looking into gfc_conv_expr_op, I see:
>>
>> case INTRINSIC_AND:
>>   code = TRUTH_ANDIF_EXPR;
>>   lop = 1;
>>   break;
>>
>> case INTRINSIC_OR:
>>   code = TRUTH_ORIF_EXPR;
>>   lop = 1;
>>   break;
>>
>> That seems to indicate that Fortran's logical .AND. operator is
>> translated into TRUTH_ANDIF_EXPR, right? Also the empirically observed
>> behavior matches Jakub's description of TRUTH_ANDIF_EXPR, where the
>> first operator is always evaluated and the second one only on demand.
>> Therefore I think my patch is correct in warning about an impure
>> function only if it occurs in the second operand of an and/or
>> operator.
>
> Is that what we want though?

Probably depends a bit on who "we" is ;)
See the discussion upthread and in PR85599.


> Are there in Fortran any ways to have
> side-effects in expressions other than function calls?  E.g.
> VOLATILE variable reads?

Don't think so.


> Is it ok not to evaluate those on .and. or .or.
> expressions?  I think best would be to change the above to
> code = TRUTH_AND_EXPR and code = TRUTH_OR_EXPR

I don't think it's ok to not evaluate expressions that have side
effects, and I would highly welcome a change to TRUTH_{AND,OR}_EXPR,
but there was quite some opposition to that position.
The Fortran standard unfortunately is somewhat agnostic of that matter
(and leaves quite some freedom to the compiler), which essentially is
the origin of all this discussion.


> and have some non-default aggressive
> optimization option that would optimize away in the FE impure function calls

IMHO an optimization to remove functions calls is unproblematic only
for pure functions, but as long as it is guarded by a non-default
optimization flag, I'm perfectly happy with optimizing away stuff with
side effects as well.

Cheers,
Janus

Re: [patch, fortran] Handling of .and. and .or. expressions

[Janus Weil]

2018-06-26 23:16 GMT+02:00 Jakub Jelinek :
> On Tue, Jun 26, 2018 at 11:12:40PM +0200, Thomas Koenig wrote:
>> Hi Janus,
>>
>> with your patch, we would only warn about
>>
>> var .and. func()
>>
>> and not about
>>
>> func() .and. var.
>>
>> AFAIK, TRUTH_AND_EXPR does not guarantee that func() will
>> always be exectued, or if it does, I have not seen the
>> documentation; it just happens to match what we currently
>> see (which may be due to missed optimizatins in the back end,
>> or the lack of test cases exposing this).
>
> If you are talking about what the middle-end does, TRUTH_AND_EXPR
> always evaluates side-effects in both operands, while for
> TRUTH_ANDIF_EXPR it evaluates the second operand only if the first operand
> is not false.

thanks for the comments. Looking into gfc_conv_expr_op, I see:

case INTRINSIC_AND:
  code = TRUTH_ANDIF_EXPR;
  lop = 1;
  break;

case INTRINSIC_OR:
  code = TRUTH_ORIF_EXPR;
  lop = 1;
  break;

That seems to indicate that Fortran's logical .AND. operator is
translated into TRUTH_ANDIF_EXPR, right? Also the empirically observed
behavior matches Jakub's description of TRUTH_ANDIF_EXPR, where the
first operator is always evaluated and the second one only on demand.
Therefore I think my patch is correct in warning about an impure
function only if it occurs in the second operand of an and/or
operator.

Cheers,
Janus

Re: [patch, fortran] Handling of .and. and .or. expressions

[Janus Weil]

Hi Thomas, hi all,

I'm back from holidays and have more time to deal with this now ...

2018-06-17 0:38 GMT+02:00 Janus Weil :
>
> Am 16. Juni 2018 23:14:08 MESZ schrieb Thomas Koenig :
>>In my patch, I have tried to do all three things at the same time, and
>>after this discussion, I still think that this is the right path
>>to follow.
>>
>>So, here is an update on the patch, which also covers ALLOCATED.
>>
>>Regression-tested. OK?
>
> I absolutely welcome the warnings for impure functions as second operand to 
> .and./.or. operators, but (as noted earlier) I disagree with changing the 
> ordering of operands. As our lengthy discussions show, things are already 
> complicated enough, and this additional optimization just adds to the 
> confusion IMHO.

the previously proposed patch by Thomas did various things at once,
not all of which we could agree upon unfortunately.

However, there also were some things that everyone seemed to agree
upon, namely that there should be warnings for the cases where impure
functions are currently short-circuited.

The patch in the attachment contains those parts of Thomas' patch that
implement that warning, but does no further optimizations or
special-casing.

Regtests cleanly on x86_64-linux-gnu. Ok for trunk?

Cheers,
Janus


2018-06-25  Thomas Koenig  
Janus Weil  

PR fortran/85599
* dump-parse-tree (show_attr): Add handling of implicit_pure.
* resolve.c (impure_function_callback): New function.
    (resolve_operator): Call it vial gfc_expr_walker.


2018-06-25  Janus Weil  

PR fortran/85599
* gfortran.dg/short_circuiting.f90: New test.
Index: gcc/fortran/dump-parse-tree.c
===
--- gcc/fortran/dump-parse-tree.c	(revision 262024)
+++ gcc/fortran/dump-parse-tree.c	(working copy)
@@ -716,6 +716,8 @@ show_attr (symbol_attribute *attr, const char * mo
 fputs (" ELEMENTAL", dumpfile);
   if (attr->pure)
 fputs (" PURE", dumpfile);
+  if (attr->implicit_pure)
+fputs (" IMPLICIT_PURE", dumpfile);
   if (attr->recursive)
 fputs (" RECURSIVE", dumpfile);
 
Index: gcc/fortran/resolve.c
===
--- gcc/fortran/resolve.c	(revision 262024)
+++ gcc/fortran/resolve.c	(working copy)
@@ -3821,6 +3821,44 @@ lookup_uop_fuzzy (const char *op, gfc_symtree *uop
 }
 
 
+/* Callback finding an impure function as an operand to an .and. or
+   .or.  expression.  Remember the last function warned about to
+   avoid double warnings when recursing.  */
+
+static int
+impure_function_callback (gfc_expr **e, int *walk_subtrees ATTRIBUTE_UNUSED,
+			  void *data)
+{
+  gfc_expr *f = *e;
+  const char *name;
+  static gfc_expr *last = NULL;
+  bool *found = (bool *) data;
+
+  if (f->expr_type == EXPR_FUNCTION)
+{
+  *found = 1;
+  if (f != last && !pure_function (f, ))
+	{
+	  /* This could still be a function without side effects, i.e.
+	 implicit pure.  Do not warn for that case.  */
+	  if (f->symtree == NULL || f->symtree->n.sym == NULL
+	  || !gfc_implicit_pure (f->symtree->n.sym))
+	{
+	  if (name)
+		gfc_warning (OPT_Wsurprising, "Function %qs at %L "
+			 "might not be evaluated", name, >where);
+	  else
+		gfc_warning (OPT_Wsurprising, "Function at %L "
+			 "might not be evaluated", >where);
+	}
+	}
+  last = f;
+}
+
+  return 0;
+}
+
+
 /* Resolve an operator expression node.  This can involve replacing the
operation with a user defined function call.  */
 
@@ -3929,6 +3967,14 @@ resolve_operator (gfc_expr *e)
 	gfc_convert_type (op1, >ts, 2);
 	  else if (op2->ts.kind < e->ts.kind)
 	gfc_convert_type (op2, >ts, 2);
+
+	  if (e->value.op.op == INTRINSIC_AND || e->value.op.op == INTRINSIC_OR)
+	{
+	  /* Warn about short-circuiting
+	 with impure function as second operand.  */
+	  bool op2_f = false;
+	  gfc_expr_walker (, impure_function_callback, _f);
+	}
 	  break;
 	}
 
! { dg-do compile }
! { dg-additional-options "-Wsurprising" }
!
! PR 85599: Prevent short-circuiting of logical expressions for non-pure functions
!
! Contributed by Janus Weil 

program short_circuit

   logical :: flag
   flag = .false.
   flag = check() .and. flag
   flag = flag .and. check()  ! { dg-warning "might not be evaluated" }
   flag = flag .and. pure_check()

contains

   logical function check()
  integer, save :: i = 1
  print *, "check", i
  i = i + 1
  check = .true.
   end function

   logical pure function pure_check()
  pure_check = .true.
   end function

end

Re: [patch, fortran] Handling of .and. and .or. expressions

[Janus Weil]

Am 16. Juni 2018 23:14:08 MESZ schrieb Thomas Koenig :
>Hi Janus,
>
>> I happen to hold the opinion that optimizing out a call to a pure 
>> function may be reasonable if it does not influence the result of an 
>> expression, but optimizing out an effectively impure function (i.e.
>one 
>> with side effects) is not a good idea at any time, since such an 
>> 'optimization' can drastically change the program flow and all
>numerical 
>> results of a piece of code.
>
>Well, I am of a different opinion

Of course opinions can differ. But could you explain what in my above 
statements sounds unreasonable to you? Otherwise it's hard to have an 
argument-based discussion.


>and so is the Fortran standard.

I don't think anything I wrote is in conflict with the standard. As I see it, 
the standard neither demands nor forbids short-circuiting. I'm just trying to 
fill that void in a reasonable way (having as much optimization as possible 
without altering results).


>I think the compiler should strive to, in that order,
>
>- conform to the language standard
>- generate fast programs
>- warn about features which may trip the user

I certainly agree to all of that, with the amendment that generating correct 
results should have precedence over generating fast programs.


>In my patch, I have tried to do all three things at the same time, and
>after this discussion, I still think that this is the right path
>to follow.
>
>So, here is an update on the patch, which also covers ALLOCATED.
>
>Regression-tested. OK?

I absolutely welcome the warnings for impure functions as second operand to 
.and./.or. operators, but (as noted earlier) I disagree with changing the 
ordering of operands. As our lengthy discussions show, things are already 
complicated enough, and this additional optimization just adds to the confusion 
IMHO.

Cheers,
Janus

Re: [patch, fortran] Handling of .and. and .or. expressions

[Janus Weil]

Am 16. Juni 2018 21:43:08 MESZ schrieb Steve Kargl 
:
>On Sat, Jun 16, 2018 at 09:21:16PM +0200, Janus Weil wrote:
>> 
>> 
>> Am 16. Juni 2018 18:38:40 MESZ schrieb Steve Kargl
>:
>> >On Sat, Jun 16, 2018 at 01:09:36PM +0200, Janus Weil wrote:
>> >> 
>> >> 
>> >> Am 15. Juni 2018 20:38:17 MESZ schrieb Steve Kargl
>> >:
>> >> >> But at least for pure functions, this optimization looks Ok.
>> >> >> 
>> >> >
>> >> >Why is everyone fixated on PURE vs IMPURE functions?
>> >> 
>> >> Simply because it makes a difference in this context!
>> >
>> >It does not!  A function marked as PURE by a programmer
>> >must meet certain requirement of the Fortran standard.
>> >An unmarked function or a function marked as IMPURE can
>> >still meet those same requirements.  Marking something as 
>> >IMPURE has only a single requirement in the standard.
>> >
>> >   An impure elemental procedure processes array arguments
>> >   in array element order.
>> >
>> >That's it.  Marking a function as IMPURE does not mean 
>> >the function has side effects.  It does not mean that
>> >a function must be evaluated for each reference.  Are
>> >you advocating that gfortran must evaluate ping() 
>> >twice for
>> >
>> >  impure real function ping()
>> >  end function ping
>> >  
>> >  x = ping() + 0 * ping()
>> >  end
>> 
>> Absolutely, sir. That's what I'm advocating. If someone
>> deliberately declares a function as impure, he should be
>> prepared to deal with the consequences. In general, though,
>> one can wonder whether the compiler could not warn that
>> the impure declaration is not necessary (or, in other words,i
>> that the function would be better declared as pure).
>
>This is a different answer than what you gave in
>the PR when I asked if you were special casing
>.and. and .or.

Possibly we're having a communication issue here. I don't quite understand what 
you mean by "special casing".


>It is trivial to force the evaluation
>of operands.  I already posted the diff in the PR

I know it's rather trivial to do this. Instead of using temporaries, as you 
propose, one could also use a proper middle-end operator, like BIT_AND_EXPR 
instead of TRUTH_AND_EXPR.

The non-trivial part is obviously to find a consensus about what should be done.


>> In any case, the example you're showing is probably not
>> the most problematic one. I assume a much more frequent
>> and critical case would be the one where people forget
>> to mark a function that is effectively pure with the
>> PURE attribute.
>
>See Fortran 2018, Note 10.28 (if I remember correctly).
>That example explicitly involves .and., and it explicitly
>states that a function need not be evaluate.   It does
>not sya "pure function".  It says "function".

You can stop throwing standard quotes. I do know what the standard says about 
this by now.

Point is: It does not sound very reasonable to me and I agree with Janne that 
the standard's treatment of impure functions is somewhere between careless and 
insane.

If the standard says it's ok to optimize out any kind of function, I would tend 
to take this less as an encouragement to compiler writers to absolutely do this 
under any circumstance, but rather as saying: "Those compiler people are smart 
enough. They can figure out where it makes sense all by themselves."

I happen to hold the opinion that optimizing out a call to a pure function may 
be reasonable if it does not influence the result of an expression, but 
optimizing out an effectively impure function (i.e. one with side effects) is 
not a good idea at any time, since such an 'optimization' can drastically 
change the program flow and all numerical results of a piece of code. (Note the 
the standard does not require this kind of optimization either, and other 
popular compilers, like ifort, do not do it.)

If we can find some kind of agreement that what I'm proposing is a reasonable 
thing to do, I will certainly be happy to provide a patch (however I will not 
be able to get to it before next weekend).

Right now I feel like I'm running into various walls. Janne is basically the 
only one so far who expressed at least partial agreement with some of my 
notions, so I certainly don't feel very encouraged to even start working on a 
patch ...

Cheers,
Janus

Re: [patch, fortran] Handling of .and. and .or. expressions

[Janus Weil]

Am 16. Juni 2018 18:38:40 MESZ schrieb Steve Kargl 
:
>On Sat, Jun 16, 2018 at 01:09:36PM +0200, Janus Weil wrote:
>> 
>> 
>> Am 15. Juni 2018 20:38:17 MESZ schrieb Steve Kargl
>:
>> >> But at least for pure functions, this optimization looks Ok.
>> >> 
>> >
>> >Why is everyone fixated on PURE vs IMPURE functions?
>> 
>> Simply because it makes a difference in this context!
>
>It does not!  A function marked as PURE by a programmer
>must meet certain requirement of the Fortran standard.
>An unmarked function or a function marked as IMPURE can
>still meet those same requirements.  Marking something as 
>IMPURE has only a single requirement in the standard.
>
>   An impure elemental procedure processes array arguments
>   in array element order.
>
>That's it.  Marking a function as IMPURE does not mean 
>the function has side effects.  It does not mean that
>a function must be evaluated for each reference.  Are
>you advocating that gfortran must evaluate ping() 
>twice for
>
>  impure real function ping()
>  end function ping
>  
>  x = ping() + 0 * ping()
>  end

Absolutely, sir. That's what I'm advocating. If someone deliberately declares a 
function as impure, he should be prepared to deal with the consequences. In 
general, though, one can wonder whether the compiler could not warn that the 
impure declaration is not necessary (or, in other words, that the function 
would be better declared as pure).

In any case, the example you're showing is probably not the most problematic 
one. I assume a much more frequent and critical case would be the one where 
people forget to mark a function that is effectively pure with the PURE 
attribute.

However, IIUC, gfortran has the ability to detect that a function meets all 
pureness requirements and mark it as implicit_pure, so that it can be treated 
like a pure procedure, even without any explicit PURE declaration by the 
programmer.

If that's the case, the scheme I proposed should not even have any major 
performance penalty, since the pureness of a function without explicit 
pure/impure declaration can be automatically detected, and appropriate 
optimizations can be chosen.

Therefore the most reasonable strategy I can see is still to apply 
short-circuiting only to pure functions (explicit or implicit) and avoid it for 
impure ones.

Cheers,
Janus

Re: [patch, fortran] Handling of .and. and .or. expressions

[Janus Weil]

Am 15. Juni 2018 20:38:17 MESZ schrieb Steve Kargl 
:
>> But at least for pure functions, this optimization looks Ok.
>> 
>
>Why is everyone fixated on PURE vs IMPURE functions?

Simply because it makes a difference in this context!

That the Fortran standard does not acknowledge this fact is more than 
surprising to me, given that Fortran actually has a way to mark functions as 
pure/impure. Not all languages have that.

What's the use of a PURE keyword, if not to indicate to the compiler that 
certain optimizations can safely be done?

Cheers,
Janus

Re: [patch, fortran] Handling of .and. and .or. expressions

[Janus Weil]

Am 15. Juni 2018 19:49:42 MESZ schrieb Janne Blomqvist 
:
>On Fri, Jun 15, 2018 at 12:22 PM, Janus Weil  wrote:
>
>> Am 14. Juni 2018 12:40:19 MESZ schrieb Janne Blomqvist <
>> blomqvist.ja...@gmail.com>:
>> In Fortran, it still feels like functions as such are second-class
>> citizens. People seriously advise against using them. Doesn't really
>help
>> the attractivity of the language.
>
>
>Yes. Back when I followed c.l.f, several experts did advise people to
>never
>use functions unless they were pure (or more or less effectively so, if
>they didn't fulfill the standard requirements for purity). Considering
>that
>at least some of those same experts were also part of the Fortran
>standards
>committee, I just find it very strange that, to the best of my
>knowledge,
>no effort to fix this has been done.

Very strange indeed.

If someone tells me "don't use functions in Fortran", then what I hear is 
"don't use Fortran, it's broken".

Why would you introduce a language feature that is not meant to be used?


>IMHO it is completely insane to optimize out impure functions, just for
>a
>> little bit of speedup, but sacrificing compiler-independent results.
>>
>> I really don't understand why I'm so alone here with this opinion.
>>
>
>I would agree with you if there were some substantial majority opinion
>among Fortran programmers that all the parts of a logical expression
>are
>always evaluated

Look, in principle I don't really care how many parts of an expression are 
evaluated. If the compiler can prove that a certain part of the expression can 
not change the end result and has no side effects, great, please optimize it 
away. If the compiler can not prove that, the expression should not be altered. 
An 'optimization' that changes results is just not useful. It's broken.

Cheers,
Janus

Re: [patch, fortran] Handling of .and. and .or. expressions

[Janus Weil]

Am 15. Juni 2018 19:10:01 MESZ schrieb Thomas Koenig :
>Am 14.06.2018 um 10:38 schrieb Janus Weil:
>>> Also, there are AFAIU other similar weirdness with impure functions.
>>> The
>>> standard allows a compiler to optimize
>>>
>>> y = f(x) + f(x)
>>>
>>> into
>>>
>>> y = 2 * f(x)
>>>
>>> even if f is impure, which is totally bonkers. Or even not call f at
>>> all,
>>> if the compiler determines that y is not needed.
>> Yes, that is the same kind of craziness. I hope gfortran does not
>actually do this?
>
>I would vote for this, but currently it is not done unless
>-faggressive-function-elimination is specified.

How about putting the short-circuiting of logical expressions with impure 
functions under the same flag?


>By the way, there is a bit of strangeness about this. People use -Ofast
>knowing it will break all sorts of standards for numercial computation

Things are a bit different with -Ofast and -ffast-math. They are not enabled by 
default, their effects are mentioned in the documentation, and they usually 
don't change the results totally.

Short-circuiting is being done by default, is not properly documented and can 
potentially have a huge impact on results (an impure function that is optimized 
away can totally alter the program flow and all numerical results).


>but they balk at using an optimization that is explicitly permitted
>by the standard.

The standard allowing it is really not any consolation, if my results are 
changed by this 'optimization' and I don't have any guarantee that different 
compilers do the same thing with my code.

That's simply a bad idea, no matter how many official approval stamps it gets.

Cheers,
Janus

Re: [patch, fortran] Handling of .and. and .or. expressions

[Janus Weil]

Am 15. Juni 2018 11:22:44 MESZ schrieb Janus Weil :
>>>>but
>>> >> >even b) would be better than leaving it undefined.
>>> >>
>>> >> Agreed.
>>> >>
>>> >> In my opinion the best strategy for gfortran in the current
>>situation
>>> >> would be to apply short-circuiting whenever it can be proven that
>>it
>>> >has no
>>> >> observable consequences. E.g. a function call should not be
>>optimized
>>> >away
>>> >> unless it is pure.
>>> >
>>> >Hmm, why? I don't see why always evaluating everything is less
>wrong
>>> >than
>>> >short-circuiting?
>>>
>>> I'm not saying it is "less wrong", but it can be less efficient. In
>>that
>>> sense my proposed strategy is a compromise that tries to avoid
>>harmful
>>> optimizations but still does optimization where it is safe.
>>>
>>
>>If the user assumes short-circuiting, then evaluating everything is
>>surprising. If the user assumes everything will be evaluated,
>>short-circuiting is surprising. So whatever is done, somebody will
>>think
>>we're doing it wrong.
>
>Strongly disagree here. My proposal is to apply short-circuiting only
>where there are absolutely no surprises. If a user writes
>
>my_variable = .true. and my_pure_function()

Sorry, of course this was meant to read:

my_variable = .false. .and. my_pure_function()

I'm currently travelling with a 5-inch screen only, which somewhat limits my 
patch-review capabilities ...

Cheers,
Janus


>then there is no way at all for him to check if the function was called
>or not (if the function is pure). And in fact it makes zero difference
>for the overall program flow and all results that are produced.
>
>To not call the function here is an absolutely valid optimization to
>make, because it does not change any results, in contrast to the case
>of impure functions.
>
>IMHO it is completely insane to optimize out impure functions, just for
>a little bit of speedup, but sacrificing compiler-independent results.
>
>I really don't understand why I'm so alone here with this opinion.
>
>Cheers,
>Janus

Re: [patch, fortran] Handling of .and. and .or. expressions

[Janus Weil]

Am 14. Juni 2018 12:40:19 MESZ schrieb Janne Blomqvist 
:
>> >> >Either
>> >> >
>> >> >a) short-circuiting in left-to-right order
>> >> >
>> >> >or
>> >> >
>> >> >b) must evaluate all the arguments in left-to-right order
>> >> >
>> >> >My preference would be for a) as that is what most languages are
>> >doing,
>> >>
>> >> Well, C/C++ has two different kinds of operators that give you the
>> >choice
>> >> to specify whether you want short-circuiting or not.
>> >
>> >Presumably you're thinking of "&" and "|". But to be pedantic, those
>> >are
>> >bitwise operators, not non-short-circuiting boolean operators
>>
>> Yes, but they work perfectly fine as logical operators on booleans,
>don't
>> they?
>>
>>
>> >There are however situations where
>> >they are not equivalent to hypothetical non-short-circuiting boolean
>> >operators!
>>
>> Could you give a simple example of such a case?
>>
>
>With the usual representation of integers, for A=1, B=2, both A and B
>by
>themselves will evaluate as true, A& will evaluate to true, but A
>will
>evaluate to false.

Yes, sure, with integers you can play such tricks. But as long as you handle 
only boolean values (in Fortran: .true. and .false.), & and | are proper 
non-short-circuiting boolean operators.

So, to repeat my earlier statement: C/C++ gives you the choice for boolean 
expressions whether you want short-circuiting or not. No ambiguities, no 
confusion, no compiler-dependent code.

In Fortran, it still feels like functions as such are second-class citizens. 
People seriously advise against using them. Doesn't really help the 
attractivity of the language.


>>>but
>> >> >even b) would be better than leaving it undefined.
>> >>
>> >> Agreed.
>> >>
>> >> In my opinion the best strategy for gfortran in the current
>situation
>> >> would be to apply short-circuiting whenever it can be proven that
>it
>> >has no
>> >> observable consequences. E.g. a function call should not be
>optimized
>> >away
>> >> unless it is pure.
>> >
>> >Hmm, why? I don't see why always evaluating everything is less wrong
>> >than
>> >short-circuiting?
>>
>> I'm not saying it is "less wrong", but it can be less efficient. In
>that
>> sense my proposed strategy is a compromise that tries to avoid
>harmful
>> optimizations but still does optimization where it is safe.
>>
>
>If the user assumes short-circuiting, then evaluating everything is
>surprising. If the user assumes everything will be evaluated,
>short-circuiting is surprising. So whatever is done, somebody will
>think
>we're doing it wrong.

Strongly disagree here. My proposal is to apply short-circuiting only where 
there are absolutely no surprises. If a user writes

my_variable = .true. and my_pure_function()

then there is no way at all for him to check if the function was called or not 
(if the function is pure). And in fact it makes zero difference for the overall 
program flow and all results that are produced.

To not call the function here is an absolutely valid optimization to make, 
because it does not change any results, in contrast to the case of impure 
functions.

IMHO it is completely insane to optimize out impure functions, just for a 
little bit of speedup, but sacrificing compiler-independent results.

I really don't understand why I'm so alone here with this opinion.

Cheers,
Janus

Re: [patch, fortran] Handling of .and. and .or. expressions

[Janus Weil]

Am 14. Juni 2018 11:41:03 MESZ schrieb Janne Blomqvist 
:
>On Thu, Jun 14, 2018 at 11:38 AM, Janus Weil 
>wrote:
>
>>
>> Am 14. Juni 2018 10:05:24 MESZ schrieb Janne Blomqvist <
>> blomqvist.ja...@gmail.com>:
>>
>> >Either
>> >
>> >a) short-circuiting in left-to-right order
>> >
>> >or
>> >
>> >b) must evaluate all the arguments in left-to-right order
>> >
>> >My preference would be for a) as that is what most languages are
>doing,
>>
>> Well, C/C++ has two different kinds of operators that give you the
>choice
>> to specify whether you want short-circuiting or not.
>
>Presumably you're thinking of "&" and "|". But to be pedantic, those
>are
>bitwise operators, not non-short-circuiting boolean operators

Yes, but they work perfectly fine as logical operators on booleans, don't they?


>There are however situations where
>they are not equivalent to hypothetical non-short-circuiting boolean
>operators!

Could you give a simple example of such a case?



>>but
>> >even b) would be better than leaving it undefined.
>>
>> Agreed.
>>
>> In my opinion the best strategy for gfortran in the current situation
>> would be to apply short-circuiting whenever it can be proven that it
>has no
>> observable consequences. E.g. a function call should not be optimized
>away
>> unless it is pure.
>
>Hmm, why? I don't see why always evaluating everything is less wrong
>than
>short-circuiting?

I'm not saying it is "less wrong", but it can be less efficient. In that sense 
my proposed strategy is a compromise that tries to avoid harmful optimizations 
but still does optimization where it is safe.

Cheers,
Janus

Re: [patch, fortran] Handling of .and. and .or. expressions

[Janus Weil]

Am 13. Juni 2018 22:39:38 MESZ schrieb Thomas Koenig :
>>> If a logical .and. or .or. expression contains a reference to a
>>> function
>>> which is impure and which also does not behave like a pure function
>>> (i.e. does not have the implicit_pure attribute set), it emits a
>>> warning with -Wsurprising that the function might not be evaluated.
>>> (-Wsurprising is enabled by -Wall).
>> 
>> I think you are warning about too many cases here. Warning about an
>impure function as the *second* operand of the logical operators should
>be enough, I think. AFAICS there is no danger of the first operand not
>being evaluated, right?
>
>Not necessarily.  The middle end, at the moment, may
>lack this optimization; at least nobody has so far come
>up with a test case that demonstrates otherwise.  On the
>other hand, people who know the middle end extremely well have
>indicated that they expect optimiazations to happen with
>TRUTH_AND_EXPR, so I am far from certaint that this behavior
>will continue.

IIUC, TRUTH_AND_EXPR corresponds to C's && operator, which is inherently 
assymetric and guaranteed to short-circuit. So I don't think the middle-end 
would ever switch operands here. I hope your ominous "people who know the 
middle end extremely well" will correct me if I'm wrong :)


>>> It special cases the idiom  if (associated(m) .and. m%t) which
>>> people appear to use.
>> 
>> I don't fully understand why you do this, but in any case it should
>not be necessary if you warn about the second operand only.
>
>Well, it is an easy mistake to make, and it is (as above) liable
>to break at any time.

You still haven't stated clearly the rationale for this warning. As I see it, 
Fortran does not require the compiler to do short-circuiting, but gfortran 
still does it. So AFAICS there is currently no reason to throw a warning on 
this case, right?

I don't think we need warnings for hypothetical situations. In fact, ifort 
might have reason to warn about this case, because it does not do 
short-circuiting.


>>> And, if there is an expression like   func() .and. flag , it
>>> reverses the test as an optimization.
>> 
>> I really don't like this part. It actually introduces more problems
>of the sort that we're trying to warn about ...
>
>If we actually perform this operation, we will have warned about this
>before (with -Wsurprising).

But we do not perform this operation at present, and I really don't think we 
should.


>And of course, the compiler can always reverse the order...

If you're talking about the middle-end here, I don't think it can (see 
discussion above).

Cheers,
Janus

Re: [patch, fortran] Handling of .and. and .or. expressions

[Janus Weil]

Am 14. Juni 2018 10:05:24 MESZ schrieb Janne Blomqvist 
:
>> There is already quite some discussion in the PRs, especially 85599,
>> where not all people were of the same opinion. Let us see where the
>> discussion here leads us.
>
>IMHO it's a mistake that the standard refuses to specify the evaluation
>strategy in the name of some rather minor hypothetical performance
>benefit.

+1

I think it's pretty much the worst strategy the standard can take, and it may 
even harm optimization in the end.

Optimization is always a process that involves both the programmer and the 
compiler. If the programmer does not know what the compiler can and will do 
with his code in the end, it's hard to write 'good' code (in the sense that it 
does what was intended, and with good performance).


>Either
>
>a) short-circuiting in left-to-right order
>
>or
>
>b) must evaluate all the arguments in left-to-right order
>
>My preference would be for a) as that is what most languages are doing,

Well, C/C++ has two different kinds of operators that give you the choice to 
specify whether you want short-circuiting or not.


>but
>even b) would be better than leaving it undefined.

Agreed.

In my opinion the best strategy for gfortran in the current situation would be 
to apply short-circuiting whenever it can be proven that it has no observable 
consequences. E.g. a function call should not be optimized away unless it is 
pure.


>Also, there are AFAIU other similar weirdness with impure functions.
>The
>standard allows a compiler to optimize
>
>y = f(x) + f(x)
>
>into
>
>y = 2 * f(x)
>
>even if f is impure, which is totally bonkers. Or even not call f at
>all,
>if the compiler determines that y is not needed.

Yes, that is the same kind of craziness. I hope gfortran does not actually do 
this?

Cheers,
Janus

Re: [patch, fortran] Handling of .and. and .or. expressions

[Janus Weil]

Hi Thomas,


>the attached patch introduces the following changes:

thanks a lot for working on this! 


>If a logical .and. or .or. expression contains a reference to a
>function
>which is impure and which also does not behave like a pure function
>(i.e. does not have the implicit_pure attribute set), it emits a
>warning with -Wsurprising that the function might not be evaluated.
>(-Wsurprising is enabled by -Wall).

I think you are warning about too many cases here. Warning about an impure 
function as the *second* operand of the logical operators should be enough, I 
think. AFAICS there is no danger of the first operand not being evaluated, 
right?


>It special cases the idiom  if (associated(m) .and. m%t) which
>people appear to use.

I don't fully understand why you do this, but in any case it should not be 
necessary if you warn about the second operand only.


>And, if there is an expression like   func() .and. flag , it
>reverses the test as an optimization.

I really don't like this part. It actually introduces more problems of the sort 
that we're trying to warn about ...

Cheers,
Janus



>2018-06-11  Thomas Koenig  
>
> PR fortran/57160
> PR fortran/85599
> * dump-parse-tree (show_attr): Add handling of implicit_pure.
> * resolve.c (impure_function_callback): New function.
> (resolve_operator): Call it vial gfc_expr_walker. Special-case
> if (associated(m) .and. m%t).  If an .and. or .or. expression
> has a function or a non-function, exchange the operands.
>
>2018-06-11  Thomas Koenig  
>
> PR fortran/57160
> PR fortran/85599
> * gfortran.dg/logical_evaluation_1.f90: New test.
> * gfortran.dg/alloc_comp_default_init_2.f90: Fix code which
> implicitly depends on short-circuiting.

Re: [Patch, Fortran, F08] PR 45521: GENERIC resolution with ALLOCATABLE/POINTER and PROCEDURE

[Janus Weil]

2018-06-11 19:49 GMT+02:00 Steve Kargl :
> On Mon, Jun 11, 2018 at 05:05:17PM +0200, Janus Weil wrote:
>>
>> the attached patch fixes two remaining problems with the resolution of
>> generic functions with POINTER and ALLOCATABLE arguments in F08
>> (coments 16 & 17 in the PR):
>> * it deals with an INTENT(IN) condition that was added in an IR
>> * it deals with polymorphic arguments, which were mistreated previously.
>>
>> The patch regtests cleanly on x86_64-linux-gnu. Ok for trunk?
>>
>
> Yes.  Thanks for the patch.

Thanks for the review, Steve. Committed as r261448.

Cheers,
Janus

[Patch, Fortran, F08] PR 45521: GENERIC resolution with ALLOCATABLE/POINTER and PROCEDURE

[Janus Weil]

Hi all,

the attached patch fixes two remaining problems with the resolution of
generic functions with POINTER and ALLOCATABLE arguments in F08
(coments 16 & 17 in the PR):
* it deals with an INTENT(IN) condition that was added in an IR
* it deals with polymorphic arguments, which were mistreated previously.

The patch regtests cleanly on x86_64-linux-gnu. Ok for trunk?

Cheers,
Janus


2018-06-11  Janus Weil  

PR fortran/45521
* interface.c (compare_ptr_alloc): New function.
(compare_ptr_alloc): Call it.


2018-06-11  Janus Weil  

PR fortran/45521
* gfortran.dg/generic_32.f90: New test.
* gfortran.dg/generic_33.f90: New test.
Index: gcc/fortran/interface.c
===
--- gcc/fortran/interface.c	(revision 261393)
+++ gcc/fortran/interface.c	(working copy)
@@ -1190,6 +1190,24 @@
 }
 
 
+/* Returns true if two dummy arguments are distinguishable due to their POINTER
+   and ALLOCATABLE attributes according to F2018 section 15.4.3.4.5 (3).
+   The function is asymmetric wrt to the arguments s1 and s2 and should always
+   be called twice (with flipped arguments in the second call).  */
+
+static bool
+compare_ptr_alloc(gfc_symbol *s1, gfc_symbol *s2)
+{
+  /* Is s1 allocatable?  */
+  const bool a1 = s1->ts.type == BT_CLASS ?
+		  CLASS_DATA(s1)->attr.allocatable : s1->attr.allocatable;
+  /* Is s2 a pointer?  */
+  const bool p2 = s2->ts.type == BT_CLASS ?
+		  CLASS_DATA(s2)->attr.class_pointer : s2->attr.pointer;
+  return a1 && p2 && (s2->attr.intent != INTENT_IN);
+}
+
+
 /* Perform the correspondence test in rule (3) of F08:C1215.
Returns zero if no argument is found that satisfies this rule,
nonzero otherwise. 'p1' and 'p2' are the PASS arguments of both procedures
@@ -1233,8 +1251,8 @@
   if (f2 != NULL && (compare_type_rank (f1->sym, f2->sym)
 			 || compare_type_rank (f2->sym, f1->sym))
 	  && !((gfc_option.allow_std & GFC_STD_F2008)
-	   && ((f1->sym->attr.allocatable && f2->sym->attr.pointer)
-		   || (f2->sym->attr.allocatable && f1->sym->attr.pointer
+	   && (compare_ptr_alloc(f1->sym, f2->sym)
+		   || compare_ptr_alloc(f2->sym, f1->sym
 	goto next;
 
   /* Now search for a disambiguating keyword argument starting at
@@ -1247,8 +1265,8 @@
 	  sym = find_keyword_arg (g->sym->name, f2_save);
 	  if (sym == NULL || !compare_type_rank (g->sym, sym)
 	  || ((gfc_option.allow_std & GFC_STD_F2008)
-		  && ((sym->attr.allocatable && g->sym->attr.pointer)
-		  || (sym->attr.pointer && g->sym->attr.allocatable
+		  && (compare_ptr_alloc(sym, g->sym)
+		  || compare_ptr_alloc(g->sym, sym
 	return true;
 	}
 
! { dg-do compile }
!
! PR 45521: [F08] GENERIC resolution with ALLOCATABLE/POINTER and PROCEDURE
!
! Contributed by Janus Weil 


  INTERFACE gen
SUBROUTINE suba(a)   ! { dg-error "Ambiguous interfaces" }
  REAL,ALLOCATABLE :: a(:)
END SUBROUTINE
SUBROUTINE subp(p)   ! { dg-error "Ambiguous interfaces" }
  REAL,POINTER,INTENT(IN) :: p(:)
END SUBROUTINE
  END INTERFACE
end
! { dg-do compile }
!
! PR 45521: [F08] GENERIC resolution with ALLOCATABLE/POINTER and PROCEDURE
!
! Contributed by Janus Weil 

  type :: t
  end type

  interface test
procedure testAlloc
procedure testPtr
  end interface

contains

  logical function testAlloc(obj)
class(t), allocatable :: obj
testAlloc = .true.
  end function

  logical function testPtr(obj)
class(t), pointer :: obj
testPtr = .false.
  end function

end

Re: [Patch, Fortran] PR 85088: improve diagnostic for bad INTENT declaration

[Janus Weil]

2018-06-10 9:02 GMT+02:00 Janus Weil :
> Hi Thomas,
>
>> I like what the patch does. However, I have one concern.
>>
>>>  * decl.c (match_attr_spec): Synchronize the DECL_* enum values with
>>> the
>>>  INTENT_* values from the enum 'sym_intent'.
>>
>>
>> This part
>>
>> +  { GFC_DECL_BEGIN = 0, DECL_ALLOCATABLE = GFC_DECL_BEGIN,
>> +DECL_IN = INTENT_IN, DECL_OUT = INTENT_OUT, DECL_INOUT = INTENT_INOUT,
>> +DECL_DIMENSION, DECL_EXTERNAL,
>> +DECL_INTRINSIC, DECL_OPTIONAL,
>>
>> brings a dependency of the values of sym_intent into this enum.
>>
>> I know that the order of sym_intent is not likely to change, but I would
>> still prefer if you could add a comment to the sym_intent definition in
>> gfortran.h noting that INTENT_IN cannot be zero, and add a
>>
>> gcc_assert (INTENT_IN > 0);
>>
>> somewhere (which will be optimized away) with an explanatory comment.
>
> good point. I have added the assert and some comments. Updated patch attached.
>
>
>> OK with that change.
>
> Thanks. I'm doing another regtest now and will commit if that goes well ...

No failures observed. Committed as r261386.

Cheers,
Janus

Re: [Patch, Fortran] PR 85088: improve diagnostic for bad INTENT declaration

[Janus Weil]

Hi Thomas,

> I like what the patch does. However, I have one concern.
>
>>  * decl.c (match_attr_spec): Synchronize the DECL_* enum values with
>> the
>>  INTENT_* values from the enum 'sym_intent'.
>
>
> This part
>
> +  { GFC_DECL_BEGIN = 0, DECL_ALLOCATABLE = GFC_DECL_BEGIN,
> +DECL_IN = INTENT_IN, DECL_OUT = INTENT_OUT, DECL_INOUT = INTENT_INOUT,
> +DECL_DIMENSION, DECL_EXTERNAL,
> +DECL_INTRINSIC, DECL_OPTIONAL,
>
> brings a dependency of the values of sym_intent into this enum.
>
> I know that the order of sym_intent is not likely to change, but I would
> still prefer if you could add a comment to the sym_intent definition in
> gfortran.h noting that INTENT_IN cannot be zero, and add a
>
> gcc_assert (INTENT_IN > 0);
>
> somewhere (which will be optimized away) with an explanatory comment.

good point. I have added the assert and some comments. Updated patch attached.


> OK with that change.

Thanks. I'm doing another regtest now and will commit if that goes well ...

Cheers,
Janus
Index: gcc/fortran/decl.c
===
--- gcc/fortran/decl.c	(revision 261358)
+++ gcc/fortran/decl.c	(working copy)
@@ -4716,9 +4716,10 @@ match_attr_spec (void)
 {
   /* Modifiers that can exist in a type statement.  */
   enum
-  { GFC_DECL_BEGIN = 0,
-DECL_ALLOCATABLE = GFC_DECL_BEGIN, DECL_DIMENSION, DECL_EXTERNAL,
-DECL_IN, DECL_OUT, DECL_INOUT, DECL_INTRINSIC, DECL_OPTIONAL,
+  { GFC_DECL_BEGIN = 0, DECL_ALLOCATABLE = GFC_DECL_BEGIN,
+DECL_IN = INTENT_IN, DECL_OUT = INTENT_OUT, DECL_INOUT = INTENT_INOUT,
+DECL_DIMENSION, DECL_EXTERNAL,
+DECL_INTRINSIC, DECL_OPTIONAL,
 DECL_PARAMETER, DECL_POINTER, DECL_PROTECTED, DECL_PRIVATE,
 DECL_STATIC, DECL_AUTOMATIC,
 DECL_PUBLIC, DECL_SAVE, DECL_TARGET, DECL_VALUE, DECL_VOLATILE,
@@ -4729,6 +4730,9 @@ match_attr_spec (void)
 /* GFC_DECL_END is the sentinel, index starts at 0.  */
 #define NUM_DECL GFC_DECL_END
 
+  /* Make sure that values from sym_intent are safe to be used here.  */
+  gcc_assert (INTENT_IN > 0);
+
   locus start, seen_at[NUM_DECL];
   int seen[NUM_DECL];
   unsigned int d;
@@ -4846,13 +4850,12 @@ match_attr_spec (void)
 		  if (match_string_p ("nt"))
 			{
 			  /* Matched "intent".  */
-			  /* TODO: Call match_intent_spec from here.  */
-			  if (gfc_match (" ( in out )") == MATCH_YES)
-			d = DECL_INOUT;
-			  else if (gfc_match (" ( in )") == MATCH_YES)
-			d = DECL_IN;
-			  else if (gfc_match (" ( out )") == MATCH_YES)
-			d = DECL_OUT;
+			  d = match_intent_spec ();
+			  if (d == INTENT_UNKNOWN)
+			{
+			  m = MATCH_ERROR;
+			  goto cleanup;
+			}
 			}
 		}
 		  else if (ch == 'r')
Index: gcc/fortran/gfortran.h
===
--- gcc/fortran/gfortran.h	(revision 261358)
+++ gcc/fortran/gfortran.h	(working copy)
@@ -291,7 +291,8 @@ enum procedure_type
   PROC_INTRINSIC, PROC_ST_FUNCTION, PROC_EXTERNAL
 };
 
-/* Intent types.  */
+/* Intent types. Note that these values are also used in another enum in
+   decl.c (match_attr_spec).  */
 enum sym_intent
 { INTENT_UNKNOWN = 0, INTENT_IN, INTENT_OUT, INTENT_INOUT
 };

[Patch, Fortran] PR 85088: improve diagnostic for bad INTENT declaration

[Janus Weil]

Hi all,

attached is a small patch that approves some diagnostics for INTENT
declarations. It also takes care of a TODO note in decl.c.

I had regtested a previous version of the patch without problems and
will do another run with this one before checking in. Ok for trunk?

Cheers,
Janus


2018-06-09  Janus Weil  

PR fortran/85088
* decl.c (match_attr_spec): Synchronize the DECL_* enum values with the
INTENT_* values from the enum 'sym_intent'. Call 'match_intent_spec'
and remove a TODO note.

2018-06-09  Janus Weil  

PR fortran/85088
* gfortran.dg/intent_decl_1.f90: New test case.
Index: gcc/fortran/decl.c
===
--- gcc/fortran/decl.c	(revision 261358)
+++ gcc/fortran/decl.c	(working copy)
@@ -4716,9 +4716,10 @@ match_attr_spec (void)
 {
   /* Modifiers that can exist in a type statement.  */
   enum
-  { GFC_DECL_BEGIN = 0,
-DECL_ALLOCATABLE = GFC_DECL_BEGIN, DECL_DIMENSION, DECL_EXTERNAL,
-DECL_IN, DECL_OUT, DECL_INOUT, DECL_INTRINSIC, DECL_OPTIONAL,
+  { GFC_DECL_BEGIN = 0, DECL_ALLOCATABLE = GFC_DECL_BEGIN,
+DECL_IN = INTENT_IN, DECL_OUT = INTENT_OUT, DECL_INOUT = INTENT_INOUT,
+DECL_DIMENSION, DECL_EXTERNAL,
+DECL_INTRINSIC, DECL_OPTIONAL,
 DECL_PARAMETER, DECL_POINTER, DECL_PROTECTED, DECL_PRIVATE,
 DECL_STATIC, DECL_AUTOMATIC,
 DECL_PUBLIC, DECL_SAVE, DECL_TARGET, DECL_VALUE, DECL_VOLATILE,
@@ -4846,13 +4847,12 @@ match_attr_spec (void)
 		  if (match_string_p ("nt"))
 			{
 			  /* Matched "intent".  */
-			  /* TODO: Call match_intent_spec from here.  */
-			  if (gfc_match (" ( in out )") == MATCH_YES)
-			d = DECL_INOUT;
-			  else if (gfc_match (" ( in )") == MATCH_YES)
-			d = DECL_IN;
-			  else if (gfc_match (" ( out )") == MATCH_YES)
-			d = DECL_OUT;
+			  d = match_intent_spec ();
+			  if (d == INTENT_UNKNOWN)
+			{
+			  m = MATCH_ERROR;
+			  goto cleanup;
+			}
 			}
 		}
 		  else if (ch == 'r')
! { dg-do compile }
!
! PR 85088: improve diagnostic for bad INTENT declaration
!
! Contributed by Janus Weil 

subroutine s(x, y, z)
   integer, intent(int) :: x  ! { dg-error "Bad INTENT specification" }
   integer, intent :: y   ! { dg-error "Bad INTENT specification" }
   integer, inten  :: z   ! { dg-error "Invalid character" }
end

Re: [Patch, Fortran] PR 85849: [F2018] warn for obsolescent features

[Janus Weil]

2018-05-24 22:59 GMT+02:00 Steve Kargl <s...@troutmask.apl.washington.edu>:
> On Thu, May 24, 2018 at 09:57:16PM +0200, Janus Weil wrote:
>>
>> just because 2018 seems like a good time to do that, I continue
>> plucking some of the low-hanging Fortran 2018 fruit. The attached
>> patch adds obsolescence warnings for all major F18 obsolescences
>> (COMMON, BLOCK DATA, EQUIVALENCE, FORALL, labeled DO). Those warnings
>> appear only with -std=f2018, but not with gfortran's default -std=gnu.
>> They currently have zero impact on the gfortran test suite, so I'm
>> adding a new test case to check for their existence.
>>
>> Regtests cleanly on x86_64-linux-gnu. Ok for trunk?
>>
>
> Ok.  Thanks for housekeeping type patches.

Thanks! Committed as r260705.

Cheers,
Janus

Re: [PATCH] PR fortan/85779 -- Fix NULL pointer dereference

[Janus Weil]

2018-05-24 2:24 GMT+02:00 Steve Kargl :
> Subject says it all.  OK to commit?

Ok with me.

Thanks,
Janus



> 2018-05-23  Steven G. Kargl  
>
> PR fortran/85779
> *decl.c (gfc_match_derived_decl): Fix NULL point dereference.
>
> 2018-05-23  Steven G. Kargl  
>
> PR fortran/85779
> * gfortran.dg/pr85779_1.f90: New test.
> * gfortran.dg/pr85779_2.f90: Ditto.
> * gfortran.dg/pr85779_3.f90: Ditto.
>
> --
> Steve

Re: [PATCH] PR fortran/85895 -- Check for valid errmsg entity

[Janus Weil]

2018-05-24 1:45 GMT+02:00 Steve Kargl :
> The attach patch fixes PR fortran/85895 and also addresses
> a nearby issue by resolving the expressions associated with
> the STAT= and ERRMSG= tags in SYNC statements.  OK to commit?

Looks good to me. Thanks for the patch!

Cheers,
Janus



> 2018-05-23  Steven G. Kargl  
>
> PR fortran/85895
> * resolve.c (resolve_sync): Resolve expression before checking for
> an error.
>
> 2018-05-23  Steven G. Kargl  
>
> PR fortran/85895
>
> * gfortran.dg/coarray_3.f90: Fix invalid testcase.
> * gfortran.dg/pr85895.f90: New test.
>
> --
> Steve

[Patch, Fortran] PR 85849: [F2018] warn for obsolescent features

[Janus Weil]

Hi all,

just because 2018 seems like a good time to do that, I continue
plucking some of the low-hanging Fortran 2018 fruit. The attached
patch adds obsolescence warnings for all major F18 obsolescences
(COMMON, BLOCK DATA, EQUIVALENCE, FORALL, labeled DO). Those warnings
appear only with -std=f2018, but not with gfortran's default -std=gnu.
They currently have zero impact on the gfortran test suite, so I'm
adding a new test case to check for their existence.

Regtests cleanly on x86_64-linux-gnu. Ok for trunk?

Cheers,
Janus


2018-05-24  Janus Weil  <ja...@gcc.gnu.org>

PR fortran/85839
* match.c (gfc_match_block_data): Call gfc_notify_std to warn about
an obsolescent feature in Fortran 2018.
(gfc_match_equivalence): Ditto.
* resolve.c (resolve_common_blocks): Ditto.
(gfc_resolve_forall): Ditto.
* symbol.c (gfc_define_st_label): Ditto.


2018-05-24  Janus Weil  <ja...@gcc.gnu.org>

PR fortran/85839
* gfortran.dg/f2018_obs.f90: New test case.
Index: gcc/fortran/match.c
===
--- gcc/fortran/match.c	(revision 260623)
+++ gcc/fortran/match.c	(working copy)
@@ -5259,6 +5259,10 @@ gfc_match_block_data (void)
   gfc_symbol *sym;
   match m;
 
+  if (!gfc_notify_std (GFC_STD_F2018_OBS, "BLOCK DATA construct at %L",
+  _current_locus))
+return MATCH_ERROR;
+
   if (gfc_match_eos () == MATCH_YES)
 {
   gfc_new_block = NULL;
@@ -5575,6 +5579,9 @@ gfc_match_equivalence (void)
 	}
 }
 
+  if (!gfc_notify_std (GFC_STD_F2018_OBS, "EQUIVALENCE statement at %C"))
+return MATCH_ERROR;
+
   return MATCH_YES;
 
 syntax:
Index: gcc/fortran/resolve.c
===
--- gcc/fortran/resolve.c	(revision 260623)
+++ gcc/fortran/resolve.c	(working copy)
@@ -997,6 +997,10 @@ resolve_common_blocks (gfc_symtree *common_root)
 
   resolve_common_vars (common_root->n.common, true);
 
+  if (!gfc_notify_std (GFC_STD_F2018_OBS, "COMMON block at %L",
+		   _root->n.common->where))
+return;
+
   /* The common name is a global name - in Fortran 2003 also if it has a
  C binding name, since Fortran 2008 only the C binding name is a global
  identifier.  */
@@ -9928,6 +9932,9 @@ gfc_resolve_forall (gfc_code *code, gfc_namespace
 
   old_nvar = nvar;
 
+  if (!gfc_notify_std (GFC_STD_F2018_OBS, "FORALL construct at %L", >loc))
+return;
+
   /* Start to resolve a FORALL construct   */
   if (forall_save == 0)
 {
Index: gcc/fortran/symbol.c
===
--- gcc/fortran/symbol.c	(revision 260623)
+++ gcc/fortran/symbol.c	(working copy)
@@ -2725,6 +2725,10 @@ gfc_define_st_label (gfc_st_label *lp, gfc_sl_type
   "DO termination statement which is not END DO"
   " or CONTINUE with label %d at %C", labelno))
 	return;
+	  if (type == ST_LABEL_DO_TARGET
+	  && !gfc_notify_std (GFC_STD_F2018_OBS, "Labeled DO statement "
+  "at %L", label_locus))
+	return;
 	  break;
 
 	default:
! { dg-do compile }
! { dg-do options "-std=f2018" }
!
! PR 85839: [F2018] warn for obsolescent features
!
! Contributed by Janus Weil <ja...@gcc.gnu.org>

block data   ! { dg-warning "obsolescent feature" }
  common /a/ y(3)! { dg-warning "obsolescent feature" }
  data y /3*1./
end

program f2018_obs

  implicit none
  integer :: a(10),b(10),j(8),i
  real :: x(3)
  common /c/ x   ! { dg-warning "obsolescent feature" }

  equivalence (a(10),b(1))   ! { dg-warning "obsolescent feature" }

  do 99 i=1,10
99 continue  ! { dg-warning "obsolescent feature" }

  j = (/ 0, 1, 2, 3, 4, 0, 6, 7  /)
  forall (i=1:8, j(i) /= 0)  ! { dg-warning "obsolescent feature" }
j(i) = 0
  end forall
end

Re: [Patch, Fortran] PR 85841: [F2018] reject deleted features

[Janus Weil]

2018-05-22 20:56 GMT+02:00 H.J. Lu <hjl.to...@gmail.com>:
> On Mon, May 21, 2018 at 10:47 PM, Janus Weil <ja...@gcc.gnu.org> wrote:
>> 2018-05-21 18:57 GMT+02:00 Steve Kargl <s...@troutmask.apl.washington.edu>:
>>> On Mon, May 21, 2018 at 12:14:13PM +0200, Janus Weil wrote:
>>>>
>>>> So, here is the promised follow-up patch. It mostly removes
>>>> GFC_STD_F2008_TS and replaces it by GFC_STD_F2018 in a mechanical
>>>> manner. Plus, it fixes the resulting fallout in the testsuite and
>>>> updates the documentation. The non-mechanical parts are libgfortran.h
>>>> and options.c. Regtests cleanly. Ok for trunk with a suitable
>>>> ChangeLog?
>>>
>>> Looks good to me.
>>
>> I have now also committed this follow-up as r260499 (after checking
>> that not only the gfortran testsuite is regression-free, but also the
>> Fortran part of the libgomp testsuite).
>>
>
> Another one:
>
> /export/gnu/import/git/sources/gcc/gcc/testsuite/gfortran.dg/pr30667.f:9:72:
> Warning: Fortran 2018 deleted feature: DO termination statement which
> is not END DO or CONTINUE with label 100 at (1)^M
> FAIL: gfortran.dg/pr30667.f   -O  (test for excess errors)
> Excess errors:
> /export/gnu/import/git/sources/gcc/gcc/testsuite/gfortran.dg/pr30667.f:9:72:
> Warning: Fortran 2018 deleted feature: DO termination statement which
> is not END DO or CONTINUE with label 100 at (1)

I added "-std=legacy" for this case in r260555, which should fix it.

Cheers,
Janus

Re: [Patch, Fortran] PR 85841: [F2018] reject deleted features

[Janus Weil]

2018-05-21 18:57 GMT+02:00 Steve Kargl <s...@troutmask.apl.washington.edu>:
> On Mon, May 21, 2018 at 12:14:13PM +0200, Janus Weil wrote:
>>
>> So, here is the promised follow-up patch. It mostly removes
>> GFC_STD_F2008_TS and replaces it by GFC_STD_F2018 in a mechanical
>> manner. Plus, it fixes the resulting fallout in the testsuite and
>> updates the documentation. The non-mechanical parts are libgfortran.h
>> and options.c. Regtests cleanly. Ok for trunk with a suitable
>> ChangeLog?
>
> Looks good to me.

I have now also committed this follow-up as r260499 (after checking
that not only the gfortran testsuite is regression-free, but also the
Fortran part of the libgomp testsuite).

Cheers,
Janus

Re: [Patch, Fortran] PR 85841: [F2018] reject deleted features

[Janus Weil]

2018-05-21 22:18 GMT+02:00 Janus Weil <ja...@gcc.gnu.org>:
> I'll take care of fixing the remaining libgomp failures.

Should be done with r260487. Please let me know if you observe any
additional problems.

Cheers,
Janus

Re: [Patch, Fortran] PR 85841: [F2018] reject deleted features

[Janus Weil]

2018-05-21 22:16 GMT+02:00 Jakub Jelinek <ja...@redhat.com>:
> On Mon, May 21, 2018 at 01:06:21PM -0700, Steve Kargl wrote:
>> On Mon, May 21, 2018 at 09:39:29PM +0200, Janus Weil wrote:
>> >
>> > Regarding the libgomp.fortran cases: Those are not included in "make
>> > check-fortran", right? How do I actually run them? Is this documented
>> > somewhere?
>>
>> Good question!
>>
>> When I want to do a specific check, I do something of the form
>>
>> % gmake check-fortran RUNTESTFLAGS="dg.exp=\*alloc\*.f90"
>>
>> which runs all tests with 'alloc' in the file name.  I don't
>> know the magic incantation for libgomp.fortran short of a
>> full on 'gmake check'.
>
> E.g. in toplevel directory do
> make check-target-libgomp RUNTESTFLAGS=fortran.exp

Thanks!

I'll take care of fixing the remaining libgomp failures.

Sorry again for the breakage,
Janus

Re: [Patch, Fortran] PR 85841: [F2018] reject deleted features

[Janus Weil]

2018-05-21 21:23 GMT+02:00 Steve Kargl <s...@troutmask.apl.washington.edu>:
> On Mon, May 21, 2018 at 12:10:01PM -0700, Steve Kargl wrote:
>> On Mon, May 21, 2018 at 09:06:40PM +0200, Janus Weil wrote:
>> > Hi Steve,
>> >
>> > > The attached patch fixes a few testcases that were missed
>> > > in the original patch.  Do you have these already in an
>> > > updated patch, or would you like me to commit my patch?
>> >
>> > I saw the message just now and had no time to react yet. Please feel
>> > free to commit your patch.
>> >
>>
>> OK, I'll commit with an appropriate ChangeLog in a few minutes.
>>
>
> Done.

Thank you!

I wonder why I did not see the gfortran.dg/graphite failures. My
testsuite log looks like those should have been run:

Running /home/jweil/gcc/trunk/gcc/testsuite/gfortran.dg/graphite/graphite.exp
...
Running /home/jweil/gcc/trunk/gcc/testsuite/gfortran.dg/graphite/graphite.exp
...
Running /home/jweil/gcc/trunk/gcc/testsuite/gfortran.dg/graphite/graphite.exp
...
Running /home/jweil/gcc/trunk/gcc/testsuite/gfortran.dg/graphite/graphite.exp
...

However no failures were reported!

Regarding the libgomp.fortran cases: Those are not included in "make
check-fortran", right? How do I actually run them? Is this documented
somewhere?

Cheers,
Janus

Re: [Patch, Fortran] PR 85841: [F2018] reject deleted features

[Janus Weil]

Hi Steve,

> The attached patch fixes a few testcases that were missed
> in the original patch.  Do you have these already in an
> updated patch, or would you like me to commit my patch?

I saw the message just now and had no time to react yet. Please feel
free to commit your patch.

Thanks,
Janus

Re: [Patch, Fortran] PR 85841: [F2018] reject deleted features

[Janus Weil]

>> >> Btw, with the arrival of the F2018 standard, I wonder whether it
>> >> actually makes sense to keep the option -std=f2008ts, or to remove it
>> >> in favor of -std=f2018, since the two Technical Specifications covered
>> >> by this flag are now part of F2018 (and pretty much the main part!).
>> >> Any opinions on this?
>> >
>> > I think that f2008ts can go away.  We may need to do this
>> > in two step as some users may have f2008ts hardcoded in
>> > Makefiles.  Probably, issue warning for -std=2008ts and
>> > map it to -std=2018 for a brief period (3 to 6 months?)
>> > and then finally remove it.
>>
>> Yes, I agree that it would be a good idea to keep -std=f2008ts as an
>> alias for f2018 for a while (possibly until the next release, 9.0),
>> and document is as deprecated. I can take care of removing the
>> GFC_STD_F2008_TS macro in a follow-up patch.
>
> Sounds good to me.


So, here is the promised follow-up patch. It mostly removes
GFC_STD_F2008_TS and replaces it by GFC_STD_F2018 in a mechanical
manner. Plus, it fixes the resulting fallout in the testsuite and
updates the documentation. The non-mechanical parts are libgfortran.h
and options.c. Regtests cleanly. Ok for trunk with a suitable
ChangeLog?

Cheers,
Janus
Index: gcc/fortran/array.c
===
--- gcc/fortran/array.c	(revision 260433)
+++ gcc/fortran/array.c	(working copy)
@@ -533,7 +533,7 @@ gfc_match_array_spec (gfc_array_spec **asp, bool m
   as->type = AS_ASSUMED_RANK;
   as->rank = -1;
 
-  if (!gfc_notify_std (GFC_STD_F2008_TS, "Assumed-rank array at %C"))
+  if (!gfc_notify_std (GFC_STD_F2018, "Assumed-rank array at %C"))
 	goto cleanup;
 
   if (!match_codim)
Index: gcc/fortran/check.c
===
--- gcc/fortran/check.c	(revision 260433)
+++ gcc/fortran/check.c	(working copy)
@@ -1136,7 +1136,7 @@ gfc_check_atomic (gfc_expr *atom, int atom_no, gfc
   if (!kind_value_check (stat, stat_no, gfc_default_integer_kind))
 	return false;
 
-  if (!gfc_notify_std (GFC_STD_F2008_TS, "STAT= argument to %s at %L",
+  if (!gfc_notify_std (GFC_STD_F2018, "STAT= argument to %s at %L",
 			   gfc_current_intrinsic, >where))
 	return false;
 }
@@ -1349,7 +1349,7 @@ gfc_check_event_query (gfc_expr *event, gfc_expr *
   if (!variable_check (stat, 2, false))
 	return false;
 
-  if (!gfc_notify_std (GFC_STD_F2008_TS, "STAT= argument to %s at %L",
+  if (!gfc_notify_std (GFC_STD_F2018, "STAT= argument to %s at %L",
 			   gfc_current_intrinsic, >where))
 	return false;
 }
@@ -4745,7 +4745,7 @@ gfc_check_c_f_pointer (gfc_expr *cptr, gfc_expr *f
 }
 
   if (fptr->rank > 0 && !is_c_interoperable (fptr, , false, true))
-return gfc_notify_std (GFC_STD_F2008_TS, "Noninteroperable array FPTR "
+return gfc_notify_std (GFC_STD_F2018, "Noninteroperable array FPTR "
 			   "at %L to C_F_POINTER: %s", >where, msg);
 
   return true;
@@ -4786,7 +4786,7 @@ gfc_check_c_f_procpointer (gfc_expr *cptr, gfc_exp
 }
 
   if (!attr.is_bind_c)
-return gfc_notify_std (GFC_STD_F2008_TS, "Noninteroperable procedure "
+return gfc_notify_std (GFC_STD_F2018, "Noninteroperable procedure "
 			   "pointer at %L to C_F_PROCPOINTER", >where);
 
   return true;
@@ -4829,7 +4829,7 @@ gfc_check_c_funloc (gfc_expr *x)
 }
 
   if (!attr.is_bind_c)
-return gfc_notify_std (GFC_STD_F2008_TS, "Noninteroperable procedure "
+return gfc_notify_std (GFC_STD_F2018, "Noninteroperable procedure "
 			   "at %L to C_FUNLOC", >where);
   return true;
 }
@@ -4883,7 +4883,7 @@ gfc_check_c_loc (gfc_expr *x)
 	}
 
   if (x->rank
-	  && !gfc_notify_std (GFC_STD_F2008_TS,
+	  && !gfc_notify_std (GFC_STD_F2018,
 			  "Noninteroperable array at %L as"
 			  " argument to C_LOC: %s", >where, msg))
 	  return false;
@@ -5267,7 +5267,7 @@ gfc_check_num_images (gfc_expr *distance, gfc_expr
   if (!scalar_check (distance, 0))
 	return false;
 
-  if (!gfc_notify_std (GFC_STD_F2008_TS, "DISTANCE= argument to "
+  if (!gfc_notify_std (GFC_STD_F2018, "DISTANCE= argument to "
 			   "NUM_IMAGES at %L", >where))
 	return false;
 }
@@ -5280,7 +5280,7 @@ gfc_check_num_images (gfc_expr *distance, gfc_expr
   if (!scalar_check (failed, 1))
 	return false;
 
-  if (!gfc_notify_std (GFC_STD_F2008_TS, "FAILED= argument to "
+  if (!gfc_notify_std (GFC_STD_F2018, "FAILED= argument to "
 			   "NUM_IMAGES at %L", >where))
 	return false;
 }
@@ -5366,7 +5366,7 @@ gfc_check_this_image (gfc_expr *coarray, gfc_expr
   if (!scalar_check (distance, 2))
 	return false;
 
-  if (!gfc_notify_std (GFC_STD_F2008_TS, "DISTANCE= argument to "
+  if (!gfc_notify_std (GFC_STD_F2018, "DISTANCE= argument to "
 			   "THIS_IMAGE at %L", >where))
 	return false;
 
Index: gcc/fortran/decl.c
===
--- gcc/fortran/decl.c	(revision 

Re: [Patch, Fortran] PR 85841: [F2018] reject deleted features

[Janus Weil]

2018-05-21 0:19 GMT+02:00 Steve Kargl <s...@troutmask.apl.washington.edu>:
> On Sun, May 20, 2018 at 09:44:47PM +0200, Janus Weil wrote:
>>
>> >> The patch still regtests cleanly. Ok for trunk?
>> >
>> > Patch looks good to me.  The only thing that worries me is
>> > whether the patch will cause the SPEC benchmark to throw
>> > an error or warning that it did not before.  As I don't have
>> > SPEC benchmark and it cost $$$, I'm not going to let it
>> > bother too much.
>>
>> Unfortunately I don't have access to SPEC either. Is anyone in a
>> position to check this?
>
> We'll find out evidently as one of the C/C++ developers run SPEC.

Ok, we have plenty of time to fix up things before the next release anyway.


>> If it is indeed a problem, I could remove the warning for the F2018
>> deleted features with default flags (-std=gnu). However, I'd only like
>> to do that if it's really necessary.
>
> I would rather see the warnings to encourage users to fix
> their codes.

Me too. Good that we can agree on something for once ;)


>> >> Btw, with the arrival of the F2018 standard, I wonder whether it
>> >> actually makes sense to keep the option -std=f2008ts, or to remove it
>> >> in favor of -std=f2018, since the two Technical Specifications covered
>> >> by this flag are now part of F2018 (and pretty much the main part!).
>> >> Any opinions on this?
>> >
>> > I think that f2008ts can go away.  We may need to do this
>> > in two step as some users may have f2008ts hardcoded in
>> > Makefiles.  Probably, issue warning for -std=2008ts and
>> > map it to -std=2018 for a brief period (3 to 6 months?)
>> > and then finally remove it.
>>
>> Yes, I agree that it would be a good idea to keep -std=f2008ts as an
>> alias for f2018 for a while (possibly until the next release, 9.0),
>> and document is as deprecated. I can take care of removing the
>> GFC_STD_F2008_TS macro in a follow-up patch.
>
> Sounds good to me.
>
>> Actually there is one other thing I do not like about my previous
>> patch: The duplication of GFC_STD_* flags in gfc_match_stopcode. To
>> get rid of this, I'm introducing additional macros in libgfortran.h,
>> see new patch in the attachment. I hope you agree that this is an
>> improvement. (Again: Regtests cleanly.)
>
> Again, looks good.

Thanks. I have committed this version of the patch as r260433.


> Welcome back to gfortran development.

Well, I do have a continued interest in improving gfortran. As usual,
I cannot make any promises (too many boundary conditions), but I will
certainly try to make the occasional contribution if I find the time.

It would be amazing to finally have complete Fortran 2003 support with
gfortran-9. I guess we're really only a handful of PRs away from that
goal now. Many of the small F2018 improvements should not be too hard
to implement either.

Cheers,
Janus

Re: [Patch, Fortran] PR 85841: [F2018] reject deleted features

[Janus Weil]

Hi Steve,

thanks for your comments!

>> The patch still regtests cleanly. Ok for trunk?
>
> Patch looks good to me.  The only thing that worries me is
> whether the patch will cause the SPEC benchmark to throw
> an error or warning that it did not before.  As I don't have
> SPEC benchmark and it cost $$$, I'm not going to let it
> bother too much.

Unfortunately I don't have access to SPEC either. Is anyone in a
position to check this?

If it is indeed a problem, I could remove the warning for the F2018
deleted features with default flags (-std=gnu). However, I'd only like
to do that if it's really necessary.


>> Btw, with the arrival of the F2018 standard, I wonder whether it
>> actually makes sense to keep the option -std=f2008ts, or to remove it
>> in favor of -std=f2018, since the two Technical Specifications covered
>> by this flag are now part of F2018 (and pretty much the main part!).
>> Any opinions on this?
>
> I think that f2008ts can go away.  We may need to do this
> in two step as some users may have f2008ts hardcoded in
> Makefiles.  Probably, issue warning for -std=2008ts and
> map it to -std=2018 for a brief period (3 to 6 months?)
> and then finally remove it.

Yes, I agree that it would be a good idea to keep -std=f2008ts as an
alias for f2018 for a while (possibly until the next release, 9.0),
and document is as deprecated. I can take care of removing the
GFC_STD_F2008_TS macro in a follow-up patch.


Actually there is one other thing I do not like about my previous
patch: The duplication of GFC_STD_* flags in gfc_match_stopcode. To
get rid of this, I'm introducing additional macros in libgfortran.h,
see new patch in the attachment. I hope you agree that this is an
improvement. (Again: Regtests cleanly.)

Cheers,
Janus
Index: gcc/fortran/error.c
===
--- gcc/fortran/error.c	(revision 260420)
+++ gcc/fortran/error.c	(working copy)
@@ -842,6 +842,40 @@ gfc_notification_std (int std)
 }
 
 
+/* Return a string describing the nature of a standard violation
+ * and/or the relevant version of the standard.  */
+
+char const*
+notify_std_msg(int std)
+{
+
+  if (std & GFC_STD_F2018_DEL)
+return _("Fortran 2018 deleted feature:");
+  else if (std & GFC_STD_F2018_OBS)
+return _("Fortran 2018 obsolescent feature:");
+  else if (std & GFC_STD_F2018)
+return _("Fortran 2018:");
+  else if (std & GFC_STD_F2008_TS)
+return "TS 29113/TS 18508:";
+  else if (std & GFC_STD_F2008_OBS)
+return _("Fortran 2008 obsolescent feature:");
+  else if (std & GFC_STD_F2008)
+return "Fortran 2008:";
+  else if (std & GFC_STD_F2003)
+return "Fortran 2003:";
+  else if (std & GFC_STD_GNU)
+return _("GNU Extension:");
+  else if (std & GFC_STD_LEGACY)
+return _("Legacy Extension:");
+  else if (std & GFC_STD_F95_OBS)
+return _("Obsolescent feature:");
+  else if (std & GFC_STD_F95_DEL)
+return _("Deleted feature:");
+  else
+gcc_unreachable ();
+}
+
+
 /* Possibly issue a warning/error about use of a nonstandard (or deleted)
feature.  An error/warning will be issued if the currently selected
standard does not contain the requested bits.  Return false if
@@ -851,55 +885,24 @@ bool
 gfc_notify_std (int std, const char *gmsgid, ...)
 {
   va_list argp;
-  bool warning;
   const char *msg, *msg2;
   char *buffer;
 
-  warning = ((gfc_option.warn_std & std) != 0) && !inhibit_warnings;
-  if ((gfc_option.allow_std & std) != 0 && !warning)
+  /* Determine whether an error or a warning is needed.  */
+  const int wstd = std & gfc_option.warn_std;/* Standard to warn about.  */
+  const int estd = std & ~gfc_option.allow_std;  /* Standard to error about.  */
+  const bool warning = (wstd != 0) && !inhibit_warnings;
+  const bool error = (estd != 0);
+
+  if (!error && !warning)
 return true;
-
   if (suppress_errors)
-return warning ? true : false;
+return !error;
 
-  switch (std)
-  {
-case GFC_STD_F2018_DEL:
-  msg = _("Fortran 2018 deleted feature:");
-  break;
-case GFC_STD_F2018_OBS:
-  msg = _("Fortran 2018 obsolescent feature:");
-  break;
-case GFC_STD_F2018:
-  msg = _("Fortran 2018:");
-  break;
-case GFC_STD_F2008_TS:
-  msg = "TS 29113/TS 18508:";
-  break;
-case GFC_STD_F2008_OBS:
-  msg = _("Fortran 2008 obsolescent feature:");
-  break;
-case GFC_STD_F2008:
-  msg = "Fortran 2008:";
-  break;
-case GFC_STD_F2003:
-  msg = "Fortran 2003:";
-  break;
-case GFC_STD_GNU:
-  msg = _("GNU Extension:");
-  break;
-case GFC_STD_LEGACY:
-  msg = _("Legacy Extension:");
-  break;
-case GFC_STD_F95_OBS:
-  msg = _("Obsolescent feature:");
-  break;
-case GFC_STD_F95_DEL:
-  msg = _("Deleted feature:");
-  break;
-default:
-  gcc_unreachable ();
-  }
+  if (error)
+msg = notify_std_msg (estd);
+  else
+msg = notify_std_msg (wstd);
 
   msg2 = 

Re: [Patch, Fortran] PR 85841: [F2018] reject deleted features

[Janus Weil]

Hi all,

> the attached patch deals with the fact that the Fortran 2018 standard
> marks two features as "deleted" (i.e. no longer supported), namely
> arithmetic IFs and nonblock DO constructs. Both have been obsolescent
> since the 90s (and have been warned about by gfortran with appropriate
> flags).
>
> Here's what the patch does:
> 1) It rejects those features with -std=f2018, giving a hard error.
> 2) It gives a warning with default flags (previously this happened
> only with -std=f2008 etc, but not with -std=gnu).

here is a slightly updated version of the patch. While the previous
one had to duplicate every invocation of gfc_notify_std like this:

if (!gfc_notify_std (GFC_STD_F2018_DEL, ...)) ...
if (!gfc_notify_std (GFC_STD_F95_OBS, ...)) ...

the new version extends gfc_notify_std, such that it works not only
with a single GFC_STD_* flag, but can handle combinations of those.
For such a situation, where a single feature can belong to several
GFC_STD_* classes, one potentially needs to throw different error or
warning messages, depending on the -std= option. The invocation of
gfc_notify_std then simplifies to:

if (!gfc_notify_std (GFC_STD_F95_OBS | GFC_STD_F2018_DEL, ...)) ...

The patch still regtests cleanly. Ok for trunk?

Btw, with the arrival of the F2018 standard, I wonder whether it
actually makes sense to keep the option -std=f2008ts, or to remove it
in favor of -std=f2018, since the two Technical Specifications covered
by this flag are now part of F2018 (and pretty much the main part!).
Any opinions on this?

Cheers,
Janus



2018-05-20  Janus Weil  <ja...@gcc.gnu.org>

PR fortran/85841
* error.c (notify_std_msg): New function.
(gfc_notify_std): Adjust such that it can handle combinations of
GFC_STD_* flags in the 'std' argument, not just a single one.
* match.c (match_arithmetic_if, gfc_match_if): Reject arithmetic if
in Fortran 2018.
(gfc_match_stopcode): Adjust checks for standard version.
* options.c (set_default_std_flags): Warn for F2018 deleted features
by default.
(gfc_handle_option): F2018 deleted features are allowed in earlier
standards.
* symbol.c (gfc_define_st_label, gfc_reference_st_label): Reject
nonblock do constructs in Fortran 2018.


2018-05-20  Janus Weil  <ja...@gcc.gnu.org>

PR fortran/85841
* gfortran.dg/g77/19990826-3.f: Add option "-std=legacy".
* gfortran.dg/g77/20020307-1.f: Ditto.
* gfortran.dg/g77/980310-3.f: Ditto.
* gfortran.dg/goacc/loop-1-2.f95: Ditto.
* gfortran.dg/goacc/loop-1.f95: Ditto.
* gfortran.dg/gomp/appendix-a/a.6.1.f90: Ditto.
* gfortran.dg/gomp/appendix-a/a.6.2.f90: Ditto.
* gfortran.dg/gomp/do-1.f90: Ditto.
* gfortran.dg/gomp/omp_do1.f90: Ditto.
* gfortran.dg/pr17229.f: Ditto.
* gfortran.dg/pr37243.f: Ditto.
* gfortran.dg/pr49721-1.f: Ditto.
* gfortran.dg/pr58484.f: Ditto.
* gfortran.dg/pr81175.f: Ditto.
* gfortran.dg/pr81723.f: Ditto.
* gfortran.dg/predcom-2.f: Ditto.
* gfortran.dg/vect/Ofast-pr50414.f90: Ditto.
* gfortran.dg/vect/cost-model-pr34445a.f: Ditto.
* gfortran.dg/vect/fast-math-mgrid-resid.f: Ditto.
* gfortran.dg/vect/pr52580.f: Ditto.
Index: gcc/fortran/error.c
===
--- gcc/fortran/error.c	(revision 260420)
+++ gcc/fortran/error.c	(working copy)
@@ -842,6 +842,40 @@ gfc_notification_std (int std)
 }
 
 
+/* Return a string describing the nature of a standard violation
+ * and/or the relevant version of the standard.  */
+
+char const*
+notify_std_msg(int std)
+{
+
+  if (std & GFC_STD_F2018_DEL)
+return _("Fortran 2018 deleted feature:");
+  else if (std & GFC_STD_F2018_OBS)
+return _("Fortran 2018 obsolescent feature:");
+  else if (std & GFC_STD_F2018)
+return _("Fortran 2018:");
+  else if (std & GFC_STD_F2008_TS)
+return "TS 29113/TS 18508:";
+  else if (std & GFC_STD_F2008_OBS)
+return _("Fortran 2008 obsolescent feature:");
+  else if (std & GFC_STD_F2008)
+return "Fortran 2008:";
+  else if (std & GFC_STD_F2003)
+return "Fortran 2003:";
+  else if (std & GFC_STD_GNU)
+return _("GNU Extension:");
+  else if (std & GFC_STD_LEGACY)
+return _("Legacy Extension:");
+  else if (std & GFC_STD_F95_OBS)
+return _("Obsolescent feature:");
+  else if (std & GFC_STD_F95_DEL)
+return _("Deleted feature:");
+  else
+gcc_unreachable ();
+}
+
+
 /* Possibly issue a warning/error about use of a nonstandard (or deleted)
feature.  An error/warning will be issued if the currently selected
standard does not contain the requested bits.  Return false if
@@ -851,55 +885,24 @@ bool
 gfc_notify_std (int std, const char *gmsgid, ...)
 {
   va_list argp;
-  bool warning;
   const