| Message ID | 51ACA2CB.6010601@net-b.de |
|---|---|
| State | New |
| Headers | show |
Early *ping*. http://gcc.gnu.org/ml/fortran/2013-06/msg00027.html Tobias Burnus wrote: > Dear all, > > Due to copying the attributes, the temporary variable could get marked > as function (attr.function, attr.flavor == FL_PROCEDURE). This either > lead to leaking those attributes into the assembler file - or to cause > an error due to the call to gfc_add_flavor. With this patch, I now > explicitly unset those attribues. (Fund when building ForTrilinos.) > > Build and > OK for the trunk and GCC 4.8? > > Tobias
Hello, Le 03/06/2013 16:06, Tobias Burnus a écrit : > Dear all, > > Due to copying the attributes, the temporary variable could get marked > as function (attr.function, attr.flavor == FL_PROCEDURE). This either > lead to leaking those attributes into the assembler file - or to cause > an error due to the call to gfc_add_flavor. With this patch, I now > explicitly unset those attribues. (Fund when building ForTrilinos.) > > diff --git a/gcc/fortran/resolve.c b/gcc/fortran/resolve.c > index b2e8fdc..655d3c1 100644 > --- a/gcc/fortran/resolve.c > +++ b/gcc/fortran/resolve.c > @@ -9293,8 +9293,12 @@ get_temp_from_expr (gfc_expr *e, gfc_namespace *ns) > } > } > > /* Add the attributes and the arrayspec to the temporary. */ > /* Add the attributes and the arrayspec to the temporary. */ > tmp->n.sym->attr = gfc_expr_attr (e); > + tmp->n.sym->attr.function = 0; > + tmp->n.sym->attr.result = 0; > + tmp->n.sym->attr.flavor = FL_VARIABLE; > + > if (as) > { > tmp->n.sym->as = gfc_copy_array_spec (as); This fixes the problem, but shouldn't the fix be in gfc_expr_attr instead? It seems to me that most symbol attributes don't make sense in any case for non-variables, except for some of the standard ones (allocatable,...) and possibly a couple more. Mikael
Hello Mikael, Mikael Morin wrote: > Le 03/06/2013 16:06, Tobias Burnus a écrit : >> diff --git a/gcc/fortran/resolve.c b/gcc/fortran/resolve.c >> index b2e8fdc..655d3c1 100644 >> --- a/gcc/fortran/resolve.c >> +++ b/gcc/fortran/resolve.c >> @@ -9293,8 +9293,12 @@ get_temp_from_expr (gfc_expr *e, gfc_namespace *ns) >> } >> } >> >> /* Add the attributes and the arrayspec to the temporary. */ >> /* Add the attributes and the arrayspec to the temporary. */ >> tmp->n.sym->attr = gfc_expr_attr (e); >> + tmp->n.sym->attr.function = 0; >> + tmp->n.sym->attr.result = 0; >> + tmp->n.sym->attr.flavor = FL_VARIABLE; >> + >> if (as) >> { >> tmp->n.sym->as = gfc_copy_array_spec (as); > This fixes the problem, but shouldn't the fix be in gfc_expr_attr instead? > It seems to me that most symbol attributes don't make sense in any case > for non-variables, except for some of the standard ones > (allocatable,...) and possibly a couple more. I will audit the use of gfc_expr_attr and send an updated patch later. Is the current patch okay for GCC 4.8? I prefer simpler patches for the branch. Tobias PS: Still pending review: [Patch, Fortran] PR57535 - Fix class-array handling for function result variables, http://gcc.gnu.org/ml/fortran/2013-06/msg00053.html
Le 11/06/2013 12:00, Tobias Burnus a écrit : > Hello Mikael, > > Mikael Morin wrote: >> Le 03/06/2013 16:06, Tobias Burnus a écrit : >>> diff --git a/gcc/fortran/resolve.c b/gcc/fortran/resolve.c >>> index b2e8fdc..655d3c1 100644 >>> --- a/gcc/fortran/resolve.c >>> +++ b/gcc/fortran/resolve.c >>> @@ -9293,8 +9293,12 @@ get_temp_from_expr (gfc_expr *e, gfc_namespace >>> *ns) >>> } >>> } >>> /* Add the attributes and the arrayspec to the temporary. */ >>> /* Add the attributes and the arrayspec to the temporary. */ >>> tmp->n.sym->attr = gfc_expr_attr (e); >>> + tmp->n.sym->attr.function = 0; >>> + tmp->n.sym->attr.result = 0; >>> + tmp->n.sym->attr.flavor = FL_VARIABLE; >>> + >>> if (as) >>> { >>> tmp->n.sym->as = gfc_copy_array_spec (as); >> This fixes the problem, but shouldn't the fix be in gfc_expr_attr >> instead? >> It seems to me that most symbol attributes don't make sense in any case >> for non-variables, except for some of the standard ones >> (allocatable,...) and possibly a couple more. > > I will audit the use of gfc_expr_attr and send an updated patch later. > > Is the current patch okay for GCC 4.8? I prefer simpler patches for the > branch. > Yes.
Mikael Morin wrote:
> This fixes the problem, but shouldn't the fix be in gfc_expr_attr instead?
I tried it - but it does not work: In many case, one actually needs a
function, e.g. for procedure pointers or for C_FUNLOC. Thus, I had to
add an additional flag to tell whether the function or the function
result it needed. But instead of adding a Boolean flag to 55 calls,
which can be false in 54 case and true in 1, I think that the original
patch is better. It's the only case where not an attribute it checked -
but where attributes are copied.
Thus, is the original patch okay? Or do you have a better
proposal?http://gcc.gnu.org/ml/fortran/2013-06/msg00027.html
Tobias
PS: Other pending patches:
* Unreviewed: Print exception status at STOP,
http://gcc.gnu.org/ml/fortran/2013-06/msg00077.html
* PR57596 - Fix OPTIONAL handling of deferred-length strings,
http://gcc.gnu.org/ml/fortran/2013-06/msg00082.html
Le 13/06/2013 19:56, Tobias Burnus a écrit : > Mikael Morin wrote: >> This fixes the problem, but shouldn't the fix be in gfc_expr_attr >> instead? > > I tried it - but it does not work: In many case, one actually needs a > function, e.g. for procedure pointers or for C_FUNLOC. Thus, I had to > add an additional flag to tell whether the function or the function > result it needed. But instead of adding a Boolean flag to 55 calls, > which can be false in 54 case and true in 1, I think that the original > patch is better. It's the only case where not an attribute it checked - > but where attributes are copied. > > Thus, is the original patch okay? Or do you have a better > proposal?http://gcc.gnu.org/ml/fortran/2013-06/msg00027.html > Sorry, I didn't see what the problem really was. Yes, the original patch is OK. Mikael
2013-06-03 Tobias Burnus <burnus@net-b.de> PR fortran/57508 * resolve.c (get_temp_from_expr): Don't copy function result attributes to temporary. 2013-06-03 Tobias Burnus <burnus@net-b.de> PR fortran/57508 * gfortran.dg/defined_assignment_7.f90: New. diff --git a/gcc/fortran/resolve.c b/gcc/fortran/resolve.c index b2e8fdc..655d3c1 100644 --- a/gcc/fortran/resolve.c +++ b/gcc/fortran/resolve.c @@ -9293,8 +9293,12 @@ get_temp_from_expr (gfc_expr *e, gfc_namespace *ns) } } /* Add the attributes and the arrayspec to the temporary. */ /* Add the attributes and the arrayspec to the temporary. */ tmp->n.sym->attr = gfc_expr_attr (e); + tmp->n.sym->attr.function = 0; + tmp->n.sym->attr.result = 0; + tmp->n.sym->attr.flavor = FL_VARIABLE; + if (as) { tmp->n.sym->as = gfc_copy_array_spec (as); @@ -9307,7 +9311,6 @@ get_temp_from_expr (gfc_expr *e, gfc_namespace *ns) tmp->n.sym->attr.dimension = 0; gfc_set_sym_referenced (tmp->n.sym); - gfc_add_flavor (&tmp->n.sym->attr, FL_VARIABLE, name, NULL); gfc_commit_symbol (tmp->n.sym); e = gfc_lval_expr_from_sym (tmp->n.sym); --- /dev/null 2013-06-03 08:35:13.011105509 +0200 +++ gcc/gcc/testsuite/gfortran.dg/defined_assignment_7.f90 2013-06-03 15:58:17.227408173 +0200 @@ -0,0 +1,29 @@ +! { dg-compile } +! +! PR fortran/57508 +! +module ForTrilinos_ref_counter + type ref_counter + contains + procedure :: assign + generic :: assignment(=) => assign + end type +contains + subroutine assign (lhs, rhs) + class (ref_counter), intent(inout) :: lhs + class (ref_counter), intent(in) :: rhs + end subroutine +end module +module FEpetra_BlockMap + use ForTrilinos_ref_counter, only : ref_counter + type :: Epetra_BlockMap + type(ref_counter) :: counter + end type +contains + function from_struct() result(new_Epetra_BlockMap) + type(Epetra_BlockMap) :: new_Epetra_BlockMap + end function + type(Epetra_BlockMap) function create_arbitrary() + create_arbitrary = from_struct() + end function +end module