Re: [Patch, fortran] PR54107: ICE on recursive interfaces and PR54195: symbol bogusly inserted twice in the interface.

2013-02-04 Thread Janus Weil
Hi Mikael,

 The following patches fix both PR54107 and PR54195.

good stuff, thank you!


 - In PR54107(comment 26), the procedure result is a procedure pointer
 whose interface is the procedure itself, which leads to an infinite
 recursion during resolution.
 - In PR54195, a type's type bound procedures are resolved twice, leading
 to a symbol being added twice in an interface and rejected.

What about PR 54107 comment 4? This also still fails, right? You had
already posted a patch for this in this PR, which however was quite
different from the one you propose here. (I was assuming that the
problems of comment 4 and comment 26 were quite similar.)

Can your 'resolved' attribute also help to fix comment 4, or are you
going to post the other patch later?


 The fix, as discussed in PR54195, adds a flag to mark a symbol as
 resolved.

Why not add this flag directly to gfc_symbol instead of
symbol_attribute? It seems we do not need the attribute for components
(or do we?).


 This leads to two regressions.  For class_20, a check to skip
 result symbols had to be removed (which was there to avoid duplicated
 resolution).  For initialization_27 (among a few others) the code adding
 the default initialization code was guarded by a check against
 gfc_current_ns, which always ended triggering when there was more than
 one resolution but may not anymore.  The fix removes it; I checked that
 gfc_current_ns wasn't used in the following code.

Ok, this makes sense.


 The second fix makes the recursion through resolve_symbol, so that the
 flag just added triggers and PR54195 is fixed.

 Regression tested on x86_64-unknown-linux-gnu. OK for trunk?

Yes, I think it's basically ok, except for the points mentioned above.

Thanks,
Janus


Re: [Patch, fortran] PR54107: ICE on recursive interfaces and PR54195: symbol bogusly inserted twice in the interface.

2013-02-04 Thread Mikael Morin
Le 04/02/2013 09:37, Janus Weil a écrit :
 - In PR54107(comment 26), the procedure result is a procedure pointer
 whose interface is the procedure itself, which leads to an infinite
 recursion during resolution.
 - In PR54195, a type's type bound procedures are resolved twice, leading
 to a symbol being added twice in an interface and rejected.
 
 What about PR 54107 comment 4?
Ah, yes, this patch fixes comment26 only.

 This also still fails, right? You had
 already posted a patch for this in this PR, which however was quite
 different from the one you propose here. (I was assuming that the
 problems of comment 4 and comment 26 were quite similar.)
Well, that's what I thought and I even developed a patch limited to
trans-types.c and supposed to fix both.  But it turned out that it
didn't, as comment26 is a recursion during resolution while comment4 is
a recursion during translation.  I didn't investigate the reasons why
comment26 doesn't recurse during translation; I suppose it's caused by
code like this (several instances):
  if (sym-attr.proc_pointer  /* whatever  */)
{
  sym-attr.proc_pointer = 0;
  type = build_pointer_type (gfc_get_function_type (sym));
  sym-attr.proc_pointer = 1;
}


 
 Can your 'resolved' attribute also help to fix comment 4, or are you
 going to post the other patch later?
It can't really help for the reasons above; at the time we start
translation, all the symbols are resolved, so if we reuse that flag, we
have to clear it somewhere, so that it doesn't really mean 'resolved'
any more.  I'll post the other patch later.

 
 
 The fix, as discussed in PR54195, adds a flag to mark a symbol as
 resolved.
 
 Why not add this flag directly to gfc_symbol instead of
 symbol_attribute? It seems we do not need the attribute for components
 (or do we?).
Hum, indeed.  symbol_attribute, despite its name, also applies to
components :-/.
OK, I'll move the flag to gfc_symbol.

Thanks for the review.
Mikael


Re: [Patch, fortran] PR54107: ICE on recursive interfaces and PR54195: symbol bogusly inserted twice in the interface.

2013-02-04 Thread Mikael Morin
Le 04/02/2013 14:02, Mikael Morin a écrit :
 The fix, as discussed in PR54195, adds a flag to mark a symbol as
 resolved.

 Why not add this flag directly to gfc_symbol instead of
 symbol_attribute? It seems we do not need the attribute for components
 (or do we?).
 Hum, indeed.  symbol_attribute, despite its name, also applies to
 components :-/.
 OK, I'll move the flag to gfc_symbol.
 
Committed as follows. revision 195729.
The other patch will follow.


Index: testsuite/gfortran.dg/recursive_interface_1.f90
===
--- testsuite/gfortran.dg/recursive_interface_1.f90 (révision 0)
+++ testsuite/gfortran.dg/recursive_interface_1.f90 (révision 195729)
@@ -0,0 +1,20 @@
+! { dg-do compile }
+!
+! PR fortran/54107
+! The compiler used to ICE on recursive interfaces.
+
+module m
+ contains
+  function foo() result(r1)
+procedure(foo), pointer :: r1 
+  end function foo
+
+  function bar() result(r2)
+procedure(baz), pointer :: r2
+  end function bar
+
+  function baz() result(r3)
+procedure(bar), pointer :: r3
+  end function baz
+end module m
+
Index: testsuite/ChangeLog
===
--- testsuite/ChangeLog (révision 195728)
+++ testsuite/ChangeLog (révision 195729)
@@ -1,3 +1,8 @@
+2013-02-04  Mikael Morin  mik...@gcc.gnu.org
+
+   PR fortran/54107
+   * gfortran.dg/recursive_interface_1.f90: New test.
+
 2013-02-04  Richard Guenther  rguent...@suse.de
 
PR lto/56168
@@ -97,7 +102,7 @@
* lib/target-supports-dg.exp (dg-process-target): Use expr to
evaluate the end index in string range.
 
-2012-01-30  Tobias Burnus  bur...@net-b.de
+2013-01-30  Tobias Burnus  bur...@net-b.de
 
PR fortran/56138
* gfortran.dg/allocatable_function_6.f90: New.
Index: fortran/gfortran.h
===
--- fortran/gfortran.h  (révision 195728)
+++ fortran/gfortran.h  (révision 195729)
@@ -1248,6 +1248,9 @@ typedef struct gfc_symbol
   unsigned equiv_built:1;
   /* Set if this variable is used as an index name in a FORALL.  */
   unsigned forall_index:1;
+  /* Used to avoid multiple resolutions of a single symbol.  */
+  unsigned resolved:1;
+
   int refs;
   struct gfc_namespace *ns;/* namespace containing this symbol */
 
Index: fortran/ChangeLog
===
--- fortran/ChangeLog   (révision 195728)
+++ fortran/ChangeLog   (révision 195729)
@@ -1,3 +1,12 @@
+2013-02-04  Mikael Morin  mik...@gcc.gnu.org
+
+   PR fortran/54107
+   PR fortran/54195
+   * gfortran.h (struct gfc_symbol): New field 'resolved'.
+   * resolve.c (resolve_fl_var_and_proc): Don't skip result symbols.
+   (resolve_symbol): Skip duplicate calls.  Don't check the current
+   namespace.
+
 2013-02-02  Thomas Koenig  tkoe...@gcc.gnu.org
 
PR fortran/50627
@@ -7,7 +16,7 @@
* parse.c (parse_module):  Do not put namespace into
gsymbol on error.
 
-2012-01-30  Tobias Burnus  bur...@net-b.de
+2013-01-30  Tobias Burnus  bur...@net-b.de
 
PR fortran/56138
* trans-decl.c (gfc_trans_deferred_vars): Fix deferred-length
@@ -214,7 +223,7 @@
finalizer_insert_packed_call, generate_finalization_wrapper):
Clean up by using gfc_build_intrinsic_call.
 
-2012-01-07  Tobias Burnus  bur...@net-b.de
+2013-01-07  Tobias Burnus  bur...@net-b.de
 
PR fortran/55763
* resolve.c (resolve_select_type): Reject intrinsic types for
Index: fortran/resolve.c
===
--- fortran/resolve.c   (révision 195728)
+++ fortran/resolve.c   (révision 195729)
@@ -11051,11 +11051,6 @@ resolve_fl_var_and_proc (gfc_symbol *sym, int mp_f
 {
   gfc_array_spec *as;
 
-  /* Avoid double diagnostics for function result symbols.  */
-  if ((sym-result || sym-attr.result)  !sym-attr.dummy
-   (sym-ns != gfc_current_ns))
-return SUCCESS;
-
   if (sym-ts.type == BT_CLASS  sym-attr.class_ok)
 as = CLASS_DATA (sym)-as;
   else
@@ -13170,6 +13165,10 @@ resolve_symbol (gfc_symbol *sym)
   gfc_array_spec *as;
   bool saved_specification_expr;
 
+  if (sym-resolved)
+return;
+  sym-resolved = 1;
+
   if (sym-attr.artificial)
 return;
 
@@ -13779,7 +13778,6 @@ resolve_symbol (gfc_symbol *sym)
  described in 14.7.5, to those variables that have not already
  been assigned one.  */
   if (sym-ts.type == BT_DERIVED
-   sym-ns == gfc_current_ns
!sym-value
!sym-attr.allocatable
!sym-attr.alloc_comp)