Message ID | 510ED7FA.2050900@sfr.fr |
---|---|
State | New |
Headers | show |
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
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 --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)