Le 23/03/2015 13:43, Andre Vehreschild a écrit : > Hi Mikael, > > thanks for looking at the patch. Please note, that Paul has sent an addendum > to > the patches for 60322, which I deliberately have attached. > >> 26/02/2015 18:17, Andre Vehreschild a écrit : >>> This first patch is only preparatory and does not change any of the >>> semantics of gfortran at all. >> Sure? > > With the counterexample you found below, this of course is a wrong statement. > >>> diff --git a/gcc/fortran/expr.c b/gcc/fortran/expr.c >>> index ab6f7a5..d28cf77 100644 >>> --- a/gcc/fortran/expr.c >>> +++ b/gcc/fortran/expr.c >>> @@ -4059,10 +4060,10 @@ gfc_lval_expr_from_sym (gfc_symbol *sym) >>> lval->symtree = gfc_find_symtree (sym->ns->sym_root, sym->name); >>> >>> /* It will always be a full array. */ >>> - lval->rank = sym->as ? sym->as->rank : 0; >>> + as = sym->as; >>> + lval->rank = as ? as->rank : 0; >>> if (lval->rank) >>> - gfc_add_full_array_ref (lval, sym->ts.type == BT_CLASS ? >>> - CLASS_DATA (sym)->as : sym->as); >>> + gfc_add_full_array_ref (lval, as); >> >> This is a change of semantics. Or do you know that sym->ts.type != >> BT_CLASS? > > You are completely right. I have made a mistake here. I have to tell the > truth, > I never ran a regtest with only part 1 of the patches applied. The second part > of the patch will correct this, by setting the variable as depending on > whether > type == BT_CLASS or not. Sorry for the mistake. > >>> diff --git a/gcc/fortran/trans-decl.c b/gcc/fortran/trans-decl.c >>> index 3664824..e571a17 100644 >>> --- a/gcc/fortran/trans-decl.c >>> +++ b/gcc/fortran/trans-decl.c >>> @@ -1013,16 +1017,24 @@ gfc_build_dummy_array_decl (gfc_symbol * sym, tree >>> dummy) tree decl; >>> tree type; >>> gfc_array_spec *as; >>> + symbol_attribute *array_attr; >>> char *name; >>> gfc_packed packed; >>> int n; >>> bool known_size; >>> >>> - if (sym->attr.pointer || sym->attr.allocatable >>> - || (sym->as && sym->as->type == AS_ASSUMED_RANK)) >>> + /* Use the array as and attr. */ >>> + as = sym->as; >>> + array_attr = &sym->attr; >>> + >>> + /* The pointer attribute is always set on a _data component, therefore >>> check >>> + the sym's attribute only. */ >>> + if (sym->attr.pointer || array_attr->allocatable >>> + || (as && as->type == AS_ASSUMED_RANK)) >>> return dummy; >>> >> Any reason to sometimes use array_attr, sometimes not, like here? >> By the way, the comment is misleading: for classes, there is the >> class_pointer attribute (and it is a pain, I know). > > Yes, and a good one. Array_attr is sometimes sym->attr and sometimes > CLASS_DATA(sym)->attr aka sym->ts.u.derived->components->attr. In the later > case .pointer is always set to 1 in the _data component's attr. I.e., the > above > if, would always yield true for a class_array, which is not intended, but > rather > destructive. I know about the class_pointer attribute, but I figured, that it > is not relevant here. Any idea how to formulate the comment better, to reflect > what I just explained? > This pointer stuff is very difficult to swallow to me. I understand that for classes, the CLASS_DATA (sym)->pointer is always set, but almost everywhere the checks for pointerness are like (sym->ts.type != BT_CLASS && sym->attr.pointer) || (sym->ts.type == BT_CLASS && CLASS_DATA (sym)->attr.class_pointer) and I don't see a convincing reason to have it different here.
At least gfc_is_nodesc_array should return 0 if sym->ts.type == BT_CLASS which solves the problem there; for the other cases, I think that class_pointer should be looked at. gfc_build_class_symbol clears the sym->attr.pointer flag for class containers so it doesn't make sense to test that flag. Mikael