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
[Patch, Fortran] PR57508 - Fix ICE/Reject-valid issue with get_temp_from_expr (intrinsic assignment with defined assignment)
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
2013-06-03 Tobias Burnus bur...@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 bur...@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
