Re: [Patch, Fortran] PR57508 - Fix ICE/Reject-valid issue with get_temp_from_expr (intrinsic assignment with defined assignment)
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
Re: [Patch, Fortran] PR57508 - Fix ICE/Reject-valid issue with get_temp_from_expr (intrinsic assignment with defined assignment)
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
Re: [Patch, Fortran] PR57508 - Fix ICE/Reject-valid issue with get_temp_from_expr (intrinsic assignment with defined assignment)
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
Re: [Patch, Fortran] PR57508 - Fix ICE/Reject-valid issue with get_temp_from_expr (intrinsic assignment with defined assignment)
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.
Re: [Patch, Fortran] PR57508 - Fix ICE/Reject-valid issue with get_temp_from_expr (intrinsic assignment with defined assignment)
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
*ping* / Re: [Patch, Fortran] PR57508 - Fix ICE/Reject-valid issue with get_temp_from_expr (intrinsic assignment with defined assignment)
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