Hi all, I have worked on the comments Mikael gave me. I am now checking for class_pointer in the way he pointed out. Furthermore did I *join the two parts* of the patch into this one, because keeping both in sync was no benefit but only tedious and did not prove to be reviewed faster. Paul, Dominique: I have addressed the LOC issue that came up lately. Or rather the patch addressed it already. I feel like this is not tested very well, not the loc() call nor the sizeof() call as given in the 57305 second's download. Unfortunately, is that download not runable. I would love to see a test similar to that download, but couldn't come up with one, that satisfied me. Given that the patch's review will last some days, I still have enough time to come up with something beautiful which I will add then. Bootstraps and regtests ok on x86_64-linux-gnu/F20. Regards, Andre On Tue, 24 Mar 2015 11:13:27 +0100 Paul Richard Thomas wrote: > Dear Andre, > > Dominique pointed out to me that the 'loc' patch causes a ICE in the > testsuite. It seems that 'loc' should provide the address of the class > container in some places and the address of the data in others. I will > put my thinking cap on tonight :-) > > Cheers > > Paul > > On 23 March 2015 at 13:43, Andre Vehreschild wrote: > > 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? > > > > Regards, > > Andre > > -- > > Andre Vehreschild * Email: vehre ad gmx dot de > > > > > > ---------- Forwarded message ---------- > > From: Paul Richard Thomas > > To: Andre Vehreschild , Dominique Dhumieres > > Cc: > > Date: Sun, 22 Mar 2015 21:20:20 +0100 > > Subject: Bug in intrinsic LOC for scalar class objects > > Dear Andre and Dominique, > > > > I have found that LOC is returning the address of the class container > > rather than the _data component for class scalars. See the source > > below, which you will recognise! A fix is attached. > > > > Note that the scalar allocate fails with MOLD= and so I substituted SOURCE=. > > > > Cheers > > > > Paul > > > > class(*), allocatable :: a(:), e ! Change 'e' to an array and > > second memcpy works correctly > > ! Problem is with loc(e), which > > returns the address of the > > ! class container. > > allocate (e, source = 99.0) > > allocate (a(2), source = [1.0, 2.0]) > > call add_element_poly (a,e) > > select type (a) > > type is (real) > > print *, a > > end select > > > > contains > > > > subroutine add_element_poly(a,e) > > use iso_c_binding > > class(*),allocatable,intent(inout),target :: a(:) > > class(*),intent(in),target :: e > > class(*),allocatable,target :: tmp(:) > > type(c_ptr) :: dummy > > > > interface > > function memcpy(dest,src,n) bind(C,name="memcpy") result(res) > > import > > type(c_ptr) :: res > > integer(c_intptr_t),value :: dest > > integer(c_intptr_t),value :: src > > integer(c_size_t),value :: n > > end function > > end interface > > > > if (.not.allocated(a)) then > > allocate(a(1), source=e) > > else > > allocate(tmp(size(a)),source=a) > > deallocate(a) > > allocate(a(size(tmp)+1),source=e) ! mold gives a segfault > > dummy = memcpy(loc(a(1)),loc(tmp),sizeof(tmp)) > > dummy = memcpy(loc(a(size(tmp)+1)),loc(e),sizeof(e)) > > end if > > end subroutine > > end > > > > > -- Andre Vehreschild * Email: vehre ad gmx dot de