diff mbox

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

Message ID 510ED7FA.2050900@sfr.fr
State New
Headers show

Commit Message

Mikael Morin Feb. 3, 2013, 9:34 p.m. UTC
Hello,

The following patches fix both PR54107 and PR54195.
- 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.

The fix, as discussed in PR54195, adds a flag to mark a symbol as
resolved.  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.

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?

Mikael
2013-02-03  Mikael Morin  <mikael@gcc.gnu.org>

	PR fortran/54107
	PR fortran/54195
	* gfortran.h (struct symbol_attribute): New field 'resolved'.
	* resolve.c (resolve_fl_var_and_proc): Don't skip result symbols.
	(resolve_symbol): Skip duplicate calls.  Don' check the current
	namespace.

2013-02-03  Mikael Morin  <mikael@gcc.gnu.org>

	PR fortran/54107
	* gfortran.dg/recursive_interface_1.f90: New test.

2013-02-03  Mikael Morin  <mikael@gcc.gnu.org>

	PR fortran/54195
	* resolve.c (resolve_typebound_procedures): Recurse through
	resolve_symbol.

2013-02-03  Mikael Morin  <mikael@gcc.gnu.org>

	PR fortran/54195
	* gfortran.dg/defined_assignment_4.f90: New test.
	* gfortran.dg/defined_assignment_5.f90: New test.
diff --git a/resolve.c b/resolve.c
index 3b74c6f..6bec662 100644
--- a/resolve.c
+++ b/resolve.c
@@ -12344,7 +12344,7 @@ resolve_typebound_procedures (gfc_symbol* derived)
 
   super_type = gfc_get_derived_super_type (derived);
   if (super_type)
-    resolve_typebound_procedures (super_type);
+    resolve_symbol (super_type);
 
   resolve_bindings_derived = derived;
   resolve_bindings_result = SUCCESS;

Comments

Janus Weil Feb. 4, 2013, 8:37 a.m. UTC | #1
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
Mikael Morin Feb. 4, 2013, 1:02 p.m. UTC | #2
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
diff mbox

Patch

diff --git a/gfortran.h b/gfortran.h
index 16751b4..af2b45a 100644
--- a/gfortran.h
+++ b/gfortran.h
@@ -810,6 +810,9 @@  typedef struct
   /* Attributes set by compiler extensions (!GCC$ ATTRIBUTES).  */
   unsigned ext_attr:EXT_ATTR_NUM;
 
+  /* Used to avoid multiple resolutions of a single symbol.  */
+  unsigned resolved:1;
+
   /* The namespace where the attribute has been set.  */
   struct gfc_namespace *volatile_ns, *asynchronous_ns;
 }
diff --git a/resolve.c b/resolve.c
index d6bae43..3b74c6f 100644
--- a/resolve.c
+++ b/resolve.c
@@ -11051,11 +11051,6 @@  resolve_fl_var_and_proc (gfc_symbol *sym, int mp_flag)
 {
   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->attr.resolved)
+    return;
+  sym->attr.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)