Re: [Fortran, Patch, PR{43366, 57117, 61337, 61376}, v1] Assign to polymorphic objects.
Dump sent privately. Yes, I meant "x". AIX defaults to 32 bit. - David On Thu, Dec 1, 2016 at 1:31 PM, Andre Vehreschild wrote: > Hi all, > > I am sorry, but the initial mail as well as Dominique answer puzzles me: > > David: I do expect to > > write (*,*) any > > not being compilable at all, because "any" is an intrinsic function and I > suppose that gfortran is not able to print it. At best it gives an address. So > am I right to assume that it should have been: > > write (*,*) x > > ? > > Which is a bit strange. Furthermore is it difficult for me to debug, because I > do not have access to an AIX machine. What address size does the machine have > 32/48/64-bit? Is there a chance you send me the file that is generated > additionally by gfortran when called with -fdump-tree-original ? The file is > named alloc_comp_class_5.f03.003t.original usually. > > Dominique: How did you get that? Do you have access to an AIX machine? What > kind of instrumentation was active in the compiler you mentioned? > > - Andre > > On Wed, 30 Nov 2016 21:51:30 +0100 > Dominique d'Humières wrote: > >> If I compile the test with an instrumented gfortran , I get >> >> ../../work/gcc/fortran/interface.c:2948:33: runtime error: load of value >> 1818451807, which is not a valid value for type ‘expr_t' >> >> Dominique >> >> > Le 30 nov. 2016 à 21:06, David Edelsohn a écrit : >> > >> > Hi, Andre >> > >> > I have noticed that the alloc_comp_class_5.f03 testcase fails on AIX. >> > Annotating the testcase a little, shows that the failure is at >> > >> > if (any(x /= ["foo", "bar", "baz"])) call abort() >> > >> > write (*,*) any >> > >> > at the point of failure produces >> > >> > "foobarba" >> > >> > - David >> > > > -- > Andre Vehreschild * Email: vehre ad gmx dot de
Re: [Fortran, Patch, PR{43366, 57117, 61337, 61376}, v1] Assign to polymorphic objects.
Hi all, I am sorry, but the initial mail as well as Dominique answer puzzles me: David: I do expect to write (*,*) any not being compilable at all, because "any" is an intrinsic function and I suppose that gfortran is not able to print it. At best it gives an address. So am I right to assume that it should have been: write (*,*) x ? Which is a bit strange. Furthermore is it difficult for me to debug, because I do not have access to an AIX machine. What address size does the machine have 32/48/64-bit? Is there a chance you send me the file that is generated additionally by gfortran when called with -fdump-tree-original ? The file is named alloc_comp_class_5.f03.003t.original usually. Dominique: How did you get that? Do you have access to an AIX machine? What kind of instrumentation was active in the compiler you mentioned? - Andre On Wed, 30 Nov 2016 21:51:30 +0100 Dominique d'Humières wrote: > If I compile the test with an instrumented gfortran , I get > > ../../work/gcc/fortran/interface.c:2948:33: runtime error: load of value > 1818451807, which is not a valid value for type ‘expr_t' > > Dominique > > > Le 30 nov. 2016 à 21:06, David Edelsohn a écrit : > > > > Hi, Andre > > > > I have noticed that the alloc_comp_class_5.f03 testcase fails on AIX. > > Annotating the testcase a little, shows that the failure is at > > > > if (any(x /= ["foo", "bar", "baz"])) call abort() > > > > write (*,*) any > > > > at the point of failure produces > > > > "foobarba" > > > > - David > -- Andre Vehreschild * Email: vehre ad gmx dot de
Re: [Fortran, Patch, PR{43366, 57117, 61337, 61376}, v1] Assign to polymorphic objects.
If I compile the test with an instrumented gfortran , I get ../../work/gcc/fortran/interface.c:2948:33: runtime error: load of value 1818451807, which is not a valid value for type ‘expr_t' Dominique > Le 30 nov. 2016 à 21:06, David Edelsohn a écrit : > > Hi, Andre > > I have noticed that the alloc_comp_class_5.f03 testcase fails on AIX. > Annotating the testcase a little, shows that the failure is at > > if (any(x /= ["foo", "bar", "baz"])) call abort() > > write (*,*) any > > at the point of failure produces > > "foobarba" > > - David
Re: [Fortran, Patch, PR{43366, 57117, 61337, 61376}, v1] Assign to polymorphic objects.
Hi, Andre I have noticed that the alloc_comp_class_5.f03 testcase fails on AIX. Annotating the testcase a little, shows that the failure is at if (any(x /= ["foo", "bar", "baz"])) call abort() write (*,*) any at the point of failure produces "foobarba" - David
Re: [Fortran, Patch, PR{43366, 57117, 61337, 61376}, v1] Assign to polymorphic objects.
Hi Paul, thanks for the review. Committed as r241439. The first nit has gone to the patch for pr78053 as agreed upon. The second nit: > + class(r), allocatable :: foo ! Need this declared of copy_R is not > generated. has magically disappeared. I assume that it was necessary on an intermediate stage of the patch only. I now have stripped the above line from the commit and everything works fine. Thanks again for the review. Regards, Andre On Sat, 22 Oct 2016 12:41:19 +0200 Paul Richard Thomas wrote: > Dear Andre, > > For the bulk of the patch, I have no comments. However, for the > testcase alloc_comp_class_5.f03, please eliminate the commented out > lines and the TODO, as discussed on #gfortran. Add them to the > testcase for for PR78053, as we agreed. > > In realloc_on_assign_27.f08, you have the following lines: > + class(t), allocatable :: x > + class(r), allocatable :: foo ! Need this declared of copy_R is not > generated. > + type(r) :: y = r (3, 42) > + > + x = y > > Surely, if you test for the existence of the vtable and create it if > necessary for the rhs type in gfc_trans_class_assign, that would > remove the need for 'foo'? > > The patch applies cleanly and regtests OK. Apart from the above nits, > OK for trunk. > > Best regards > > Paul > > On 22 October 2016 at 12:19, Andre Vehreschild wrote: > > Hi Paul, > > > > here is the patch for pr78053 so far. It is based on the one for pr43366. > > Compilation of the also attached testcase now works. Unfortunately produces > > the patch a lot of regressions because the length of a char array is not > > stored any longer in the vtab *and* in the _len component for deferred > > length char arrays. That still has to be fixed. Given that you have > > modified a lot on how SELECT TYPE works I fear, that when I change there, > > too, we get a lot of conflicts. So when you have a version of your patch > > for pr69834 I am happy to review it and continue work on pr78053 > > afterwards. I think this makes the most sense to avoid duplicate or > > colliding work. > > > > Regards, > > Andre > > -- Andre Vehreschild * Email: vehre ad gmx dot de
Re: [Fortran, Patch, PR{43366, 57117, 61337, 61376}, v1] Assign to polymorphic objects.
Dear Andre, For the bulk of the patch, I have no comments. However, for the testcase alloc_comp_class_5.f03, please eliminate the commented out lines and the TODO, as discussed on #gfortran. Add them to the testcase for for PR78053, as we agreed. In realloc_on_assign_27.f08, you have the following lines: + class(t), allocatable :: x + class(r), allocatable :: foo ! Need this declared of copy_R is not generated. + type(r) :: y = r (3, 42) + + x = y Surely, if you test for the existence of the vtable and create it if necessary for the rhs type in gfc_trans_class_assign, that would remove the need for 'foo'? The patch applies cleanly and regtests OK. Apart from the above nits, OK for trunk. Best regards Paul On 22 October 2016 at 12:19, Andre Vehreschild wrote: > Hi Paul, > > here is the patch for pr78053 so far. It is based on the one for pr43366. > Compilation of the also attached testcase now works. Unfortunately produces > the > patch a lot of regressions because the length of a char array is not stored > any > longer in the vtab *and* in the _len component for deferred length char > arrays. > That still has to be fixed. Given that you have modified a lot on how SELECT > TYPE works I fear, that when I change there, too, we get a lot of conflicts. > So > when you have a version of your patch for pr69834 I am happy to review it and > continue work on pr78053 afterwards. I think this makes the most sense to > avoid > duplicate or colliding work. > > Regards, > Andre >
Re: [Fortran, Patch, PR{43366, 57117, 61337, 61376}, v1] Assign to polymorphic objects.
Hi all,
attached is an enhanced version of the patch, which now catches all of the
testcases in the comments of pr61337. Thanks for reporting that I missed them,
Dominique.
For a detailed description see below. PR61337 needed just some more pre-code
joining and correct handling of class-typed array constructors.
Bootstraps and regtests ok on x86_64-linux/F23. Ok for trunk?
Regards,
Andre
On Thu, 13 Oct 2016 14:42:00 +0200
Andre Vehreschild wrote:
> Hi all,
>
> attached patch fixes the PRs (as to my knowledge):
>
> PR43366 - [OOP][F08] Intrinsic assign to polymorphic variable
> PR57117 - [OOP] ICE for sourced allocation of a polymorphic entity using
> TRANSPOSE
> PR61337 - Wrong indexing and runtime crash with unlimited polymorphic array.
> PR61378 - Error using private statement in polymorphic derived type
>
> The latter two are more or less fixed by accident or have been fixed by
> previous patches, but have not been identified as such. Anyway, they are fixed
> now and will be closed once the patch hits trunk.
>
> As for PR43366: I did not indent to fix this one, but when going for PR57117 I
> once again stumbled over the deficiencies of gfc_trans_assigment's handling of
> class objects. Therefore I figured what would be needed to complete PR43366
> and this is it now.
>
> As for PR57117: The issue was that ALLOCATE () used gfc_copy_class_to_class ()
> when a class object was allocated. The function gfc_copy_class_to_class ()
> does not use the scalarizer correctly. I.e., a transpose of the source=
> expression would not be respected. I therefore decided to remove all this
> special casing for class objects in ALLOCATE () and let gfc_trans_assignment
> do the trick. This way ensuring, that any improvements of the scalarizer will
> benefit class objects, too. Unfortunately did this mean to add more logic to
> gfc_trans_assignment. While doing so, I learned that existing wrappers for
> class assignments were obsoleted by the work I did, so I removed them.
>
> I tried to get rid of the malicious copy_class_to_class, too, but at the
> moment it is still used at one location where components of derived types are
> assigned. I was not bold enough to replace this occurrence with
> trans_assignment yet.
>
> This patch shall make our lives easier, because now there is one routine to
> assign all sorts of objects and no special casing for class objects is needed
> anymore. I expect that some other parts of gfortran's code base may benefit
> from the changes and have their complexity reduced.
>
> Bootstrapped and regtested ok on x86_64-linux/F23. Ok for trunk?
>
> Regards,
> Andre
--
Andre Vehreschild * Email: vehre ad gmx dot de
pr43366_v2.clog
Description: Binary data
diff --git a/gcc/fortran/primary.c b/gcc/fortran/primary.c
index 85589ee..3803b88 100644
--- a/gcc/fortran/primary.c
+++ b/gcc/fortran/primary.c
@@ -2359,6 +2359,10 @@ gfc_expr_attr (gfc_expr *e)
attr.allocatable = CLASS_DATA (sym)->attr.allocatable;
}
}
+ else if (e->value.function.isym
+ && e->value.function.isym->transformational
+ && e->ts.type == BT_CLASS)
+ attr = CLASS_DATA (e)->attr;
else
attr = gfc_variable_attr (e, NULL);
diff --git a/gcc/fortran/resolve.c b/gcc/fortran/resolve.c
index 87178a4..3bb057d 100644
--- a/gcc/fortran/resolve.c
+++ b/gcc/fortran/resolve.c
@@ -9834,10 +9834,6 @@ resolve_ordinary_assign (gfc_code *code, gfc_namespace *ns)
"requires %<-frealloc-lhs%>", &lhs->where);
return false;
}
- /* See PR 43366. */
- gfc_error ("Assignment to an allocatable polymorphic variable at %L "
- "is not yet supported", &lhs->where);
- return false;
}
else if (lhs->ts.type == BT_CLASS)
{
@@ -10740,6 +10736,19 @@ start:
break;
gfc_check_pointer_assign (code->expr1, code->expr2);
+
+ /* Assigning a class object always is a regular assign. */
+ if (code->expr2->ts.type == BT_CLASS
+ && !CLASS_DATA (code->expr2)->attr.dimension
+ && !(UNLIMITED_POLY (code->expr2)
+ && code->expr1->ts.type == BT_DERIVED
+ && (code->expr1->ts.u.derived->attr.sequence
+ || code->expr1->ts.u.derived->attr.is_bind_c))
+ && !(gfc_expr_attr (code->expr1).proc_pointer
+ && code->expr2->expr_type == EXPR_VARIABLE
+ && code->expr2->symtree->n.sym->attr.flavor
+ == FL_PROCEDURE))
+ code->op = EXEC_ASSIGN;
break;
}
diff --git a/gcc/fortran/trans-array.c b/gcc/fortran/trans-array.c
index 37cca79..c59e872 100644
--- a/gcc/fortran/trans-array.c
+++ b/gcc/fortran/trans-array.c
@@ -2292,7 +2292,8 @@ trans_array_constructor (gfc_ss * ss, locus * where)
type = build_pointer_type (type);
}
else
-type = gfc_typenode_for_spec (&expr->ts);
+type = gfc_typenode_for_spec (expr->ts.type == BT_CLASS
+ ? &CLASS_DATA (expr)->ts : &expr->ts);
/* See if the constructor determines the loop bounds. */
dynamic = false;
@@ -3036,50 +3037,57 @@ build_class
[Fortran, Patch, PR{43366, 57117, 61337, 61376}, v1] Assign to polymorphic objects.
Hi all,
attached patch fixes the PRs (as to my knowledge):
PR43366 - [OOP][F08] Intrinsic assign to polymorphic variable
PR57117 - [OOP] ICE for sourced allocation of a polymorphic entity using
TRANSPOSE
PR61337 - Wrong indexing and runtime crash with unlimited polymorphic array.
PR61378 - Error using private statement in polymorphic derived type
The latter two are more or less fixed by accident or have been fixed by
previous patches, but have not been identified as such. Anyway, they are fixed
now and will be closed once the patch hits trunk.
As for PR43366: I did not indent to fix this one, but when going for PR57117 I
once again stumbled over the deficiencies of gfc_trans_assigment's handling of
class objects. Therefore I figured what would be needed to complete PR43366 and
this is it now.
As for PR57117: The issue was that ALLOCATE () used gfc_copy_class_to_class ()
when a class object was allocated. The function gfc_copy_class_to_class () does
not use the scalarizer correctly. I.e., a transpose of the source= expression
would not be respected. I therefore decided to remove all this special casing
for class objects in ALLOCATE () and let gfc_trans_assignment do the trick.
This way ensuring, that any improvements of the scalarizer will benefit class
objects, too. Unfortunately did this mean to add more logic to
gfc_trans_assignment. While doing so, I learned that existing wrappers for
class assignments were obsoleted by the work I did, so I removed them.
I tried to get rid of the malicious copy_class_to_class, too, but at the moment
it is still used at one location where components of derived types are
assigned. I was not bold enough to replace this occurrence with
trans_assignment yet.
This patch shall make our lives easier, because now there is one routine to
assign all sorts of objects and no special casing for class objects is needed
anymore. I expect that some other parts of gfortran's code base may benefit from
the changes and have their complexity reduced.
Bootstrapped and regtested ok on x86_64-linux/F23. Ok for trunk?
Regards,
Andre
--
Andre Vehreschild * Email: vehre ad gmx dot de
pr43366_1.clog
Description: Binary data
diff --git a/gcc/fortran/primary.c b/gcc/fortran/primary.c
index 85589ee..3803b88 100644
--- a/gcc/fortran/primary.c
+++ b/gcc/fortran/primary.c
@@ -2359,6 +2359,10 @@ gfc_expr_attr (gfc_expr *e)
attr.allocatable = CLASS_DATA (sym)->attr.allocatable;
}
}
+ else if (e->value.function.isym
+ && e->value.function.isym->transformational
+ && e->ts.type == BT_CLASS)
+ attr = CLASS_DATA (e)->attr;
else
attr = gfc_variable_attr (e, NULL);
diff --git a/gcc/fortran/resolve.c b/gcc/fortran/resolve.c
index 4645b57..42e3421 100644
--- a/gcc/fortran/resolve.c
+++ b/gcc/fortran/resolve.c
@@ -9829,10 +9829,6 @@ resolve_ordinary_assign (gfc_code *code, gfc_namespace *ns)
"requires %<-frealloc-lhs%>", &lhs->where);
return false;
}
- /* See PR 43366. */
- gfc_error ("Assignment to an allocatable polymorphic variable at %L "
- "is not yet supported", &lhs->where);
- return false;
}
else if (lhs->ts.type == BT_CLASS)
{
@@ -10735,6 +10731,19 @@ start:
break;
gfc_check_pointer_assign (code->expr1, code->expr2);
+
+ /* Assigning a class object always is a regular assign. */
+ if (code->expr2->ts.type == BT_CLASS
+ && !CLASS_DATA (code->expr2)->attr.dimension
+ && !(UNLIMITED_POLY (code->expr2)
+ && code->expr1->ts.type == BT_DERIVED
+ && (code->expr1->ts.u.derived->attr.sequence
+ || code->expr1->ts.u.derived->attr.is_bind_c))
+ && !(gfc_expr_attr (code->expr1).proc_pointer
+ && code->expr2->expr_type == EXPR_VARIABLE
+ && code->expr2->symtree->n.sym->attr.flavor
+ == FL_PROCEDURE))
+ code->op = EXEC_ASSIGN;
break;
}
diff --git a/gcc/fortran/trans-array.c b/gcc/fortran/trans-array.c
index 37cca79..4db55c1 100644
--- a/gcc/fortran/trans-array.c
+++ b/gcc/fortran/trans-array.c
@@ -2292,7 +2292,8 @@ trans_array_constructor (gfc_ss * ss, locus * where)
type = build_pointer_type (type);
}
else
-type = gfc_typenode_for_spec (&expr->ts);
+type = gfc_typenode_for_spec (expr->ts.type == BT_CLASS
+ ? &CLASS_DATA (expr)->ts : &expr->ts);
/* See if the constructor determines the loop bounds. */
dynamic = false;
@@ -3036,50 +3037,57 @@ build_class_array_ref (gfc_se *se, tree base, tree index)
tree type;
tree size;
tree offset;
- tree decl;
+ tree decl = NULL_TREE;
tree tmp;
gfc_expr *expr = se->ss->info->expr;
gfc_ref *ref;
- gfc_ref *class_ref;
+ gfc_ref *class_ref = NULL;
gfc_typespec *ts;
- if (expr == NULL
- || (expr->ts.type != BT_CLASS
- && !gfc_is_alloc_class_array_function (expr)))
-return false;
-
- if (expr->symtree && expr->symtree->n.sym->ts.type == BT_CLASS)
-ts = &expr->symtree->n.sym->ts;
+ if (se->expr && DECL_P (se->expr) && D
