Re: [Patch, fortran] [4.7/4.8 Regression] [OOP] ICE on invalid: gfc_variable_attr(): Bad array reference
Dear Tobias, Committed as revision 194916. I am leaving the PR open to deal with 4.7 just as soon as I have downloaded it! Thanks again for the careful review. Cheers Paul On 2 January 2013 23:48, Tobias Burnus wrote: > Dear Paul, > > First, the new patch is fine from my side. (Although, I think the test case > should also include the vector-section example.) Thanks for working on that > regression. > > > > Paul Richard Thomas wrote: >> >> First of all, thanks for the review! I still owe you my comments on >> FINAL; I got lost in trying to fix these various regressions :-) I >> promise that I'll come back to you first thing tomorrow. > > > I am looking forward to them - and in particular to a patch review of the > FINAL patches. I am also interested in your comment to my LOGICAL in BIND(C) > procedures patch, namely http://gcc.gnu.org/ml/fortran/2012-12/msg00200.html > > >>> It looks mostly okay; however, you do not handle vector sections >>> correctly, >>> which leads to an ICE. Without your patch, one gets: >>> Error: CLASS selector at (1) needs a temporary which is not yet >>> implemented >>> >>> With your patch, it fails as one has: >> >> This was fairly easily fixed - see attached. > > > Thanks. By the way, I have filled a PR to track this "not yet implemented" > issue: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55849 > > >>> I am not quite sure whether the following ICE has the same cause or a >>> different one, but it also ICEs with your patch applied: >>> select type (component => self%cb[4]) >> >> This co-array example was never OK, as far as I can tell. The error >> >> is similar to that of the PR. However, co-arrays were just never >> handled at all again, as far as I can tell. I'll have a go at it >> tomorrow night. > > > I recall that we did add some coarray support for polymorphic variables. At > least with coarray arrays (contrary to coarray scalars) it seems to compile. > But it is very likely that select type never worked with coarrays or coarray > scalars. > > Note that the coindexd > > > select type (component => self%cb[4]) > is invalid (C803; PR55850 (a)). However, the same failure occurs for > noncoindexed valid selector: > select type (component => self%cb) > > > (See also PR 55850 for some other SELECT TYPE issues I found.) > > Tobias -- The knack of flying is learning how to throw yourself at the ground and miss. --Hitchhikers Guide to the Galaxy
Re: [Patch, fortran] [4.7/4.8 Regression] [OOP] ICE on invalid: gfc_variable_attr(): Bad array reference
Dear Tobias, Thanks for the review. The patched trunk is just now bootstrapping and regtesting prior to commitment. I have added a check for co-indexed selectors in resolve.c and have added tests in select_type_31.f03 for co-indexing and vector indices. I am now turning my attention to FINAL :-) Cheers Paul PS BTW co-arrays fail similarly as selectors in SELECT_TYPE and in ASSOCIATE. Do I assume correctly that the associate variable should be a co-array? On 2 January 2013 23:48, Tobias Burnus wrote: > Dear Paul, > > First, the new patch is fine from my side. (Although, I think the test case > should also include the vector-section example.) Thanks for working on that > regression. > > > > Paul Richard Thomas wrote: >> >> First of all, thanks for the review! I still owe you my comments on >> FINAL; I got lost in trying to fix these various regressions :-) I >> promise that I'll come back to you first thing tomorrow. > > > I am looking forward to them - and in particular to a patch review of the > FINAL patches. I am also interested in your comment to my LOGICAL in BIND(C) > procedures patch, namely http://gcc.gnu.org/ml/fortran/2012-12/msg00200.html > > >>> It looks mostly okay; however, you do not handle vector sections >>> correctly, >>> which leads to an ICE. Without your patch, one gets: >>> Error: CLASS selector at (1) needs a temporary which is not yet >>> implemented >>> >>> With your patch, it fails as one has: >> >> This was fairly easily fixed - see attached. > > > Thanks. By the way, I have filled a PR to track this "not yet implemented" > issue: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55849 > > >>> I am not quite sure whether the following ICE has the same cause or a >>> different one, but it also ICEs with your patch applied: >>> select type (component => self%cb[4]) >> >> This co-array example was never OK, as far as I can tell. The error >> >> is similar to that of the PR. However, co-arrays were just never >> handled at all again, as far as I can tell. I'll have a go at it >> tomorrow night. > > > I recall that we did add some coarray support for polymorphic variables. At > least with coarray arrays (contrary to coarray scalars) it seems to compile. > But it is very likely that select type never worked with coarrays or coarray > scalars. > > Note that the coindexd > > > select type (component => self%cb[4]) > is invalid (C803; PR55850 (a)). However, the same failure occurs for > noncoindexed valid selector: > select type (component => self%cb) > > > (See also PR 55850 for some other SELECT TYPE issues I found.) > > Tobias -- The knack of flying is learning how to throw yourself at the ground and miss. --Hitchhikers Guide to the Galaxy
Re: [Patch, fortran] [4.7/4.8 Regression] [OOP] ICE on invalid: gfc_variable_attr(): Bad array reference
Dear Paul, First, the new patch is fine from my side. (Although, I think the test case should also include the vector-section example.) Thanks for working on that regression. Paul Richard Thomas wrote: First of all, thanks for the review! I still owe you my comments on FINAL; I got lost in trying to fix these various regressions :-) I promise that I'll come back to you first thing tomorrow. I am looking forward to them - and in particular to a patch review of the FINAL patches. I am also interested in your comment to my LOGICAL in BIND(C) procedures patch, namely http://gcc.gnu.org/ml/fortran/2012-12/msg00200.html It looks mostly okay; however, you do not handle vector sections correctly, which leads to an ICE. Without your patch, one gets: Error: CLASS selector at (1) needs a temporary which is not yet implemented With your patch, it fails as one has: This was fairly easily fixed - see attached. Thanks. By the way, I have filled a PR to track this "not yet implemented" issue: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55849 I am not quite sure whether the following ICE has the same cause or a different one, but it also ICEs with your patch applied: select type (component => self%cb[4]) This co-array example was never OK, as far as I can tell. The error is similar to that of the PR. However, co-arrays were just never handled at all again, as far as I can tell. I'll have a go at it tomorrow night. I recall that we did add some coarray support for polymorphic variables. At least with coarray arrays (contrary to coarray scalars) it seems to compile. But it is very likely that select type never worked with coarrays or coarray scalars. Note that the coindexd select type (component => self%cb[4]) is invalid (C803; PR55850 (a)). However, the same failure occurs for noncoindexed valid selector: select type (component => self%cb) (See also PR 55850 for some other SELECT TYPE issues I found.) Tobias
Re: [Patch, fortran] [4.7/4.8 Regression] [OOP] ICE on invalid: gfc_variable_attr(): Bad array reference
Dear Tobias, First of all, thanks for the review! I still owe you my comments on FINAL; I got lost in trying to fix these various regressions :-) I promise that I'll come back to you first thing tomorrow. > > It looks mostly okay; however, you do not handle vector sections correctly, > which leads to an ICE. Without your patch, one gets: >Error: CLASS selector at (1) needs a temporary which is not yet > implemented > > With your patch, it fails as one has: This was fairly easily fixed - see attached. snip... > I am not quite sure whether the following ICE has the same cause or a > different one, but it also ICEs with your patch applied: > > module gn > type :: ncb > end type ncb > type, public :: tn > class(ncb), allocatable :: cb[:] > end type tn > contains > integer function name(self) > implicit none > class (tn), intent(in) :: self > select type (component => self%cb[4]) > ! ALSO FAILS: "(component => self%cb)" > end select > end function name > end module gn This co-array example was never OK, as far as I can tell. The error is similar to that of the PR. However, co-arrays were just never handled at all again, as far as I can tell. I'll have a go at it tomorrow night. With best regards Paul pr55172a.diff Description: Binary data
Re: [Patch, fortran] [4.7/4.8 Regression] [OOP] ICE on invalid: gfc_variable_attr(): Bad array reference
Dear Paul,
Paul Richard Thomas wrote:
As noted by Janus in comment #2 of the PR, "I think the function
'copy_ts_from_selector_to_associate' comes too early (namely during
parsing). It tries to resolve the target expr, which should rather
wait until resolution stage!?!"
It turned out that the function of the call to gfc_resolve_expr was to
fix up the selector array reference type. This has been done
explicitly in this patch.
Bootstraps and regtests on FC17/x86_64 - OK for trunk and 4.7?
It looks mostly okay; however, you do not handle vector sections
correctly, which leads to an ICE. Without your patch, one gets:
Error: CLASS selector at (1) needs a temporary which is not yet
implemented
With your patch, it fails as one has:
(gdb) p ref->next->u.ar.type
$7 = AR_ELEMENT
(gdb) p ref->next->u.ar.dimen_type
$8 = {DIMEN_VECTOR, 0, 0, 0, 0, 0, 0}
Please fix the DIMEN_VECTOR handling and add a test case for it (e.g.
the one below). Could you also check whether we have a PR for that "not
yet implemented" error?
module gn
type :: ncb
end type ncb
type, public :: tn
class(ncb), allocatable, dimension(:) :: cb
end type tn
contains
integer function name(self)
implicit none
class (tn), intent(in) :: self
select type (component => self%cb([4,7+1]))
end select
end function name
end module gn
I am not quite sure whether the following ICE has the same cause or a
different one, but it also ICEs with your patch applied:
module gn
type :: ncb
end type ncb
type, public :: tn
class(ncb), allocatable :: cb[:]
end type tn
contains
integer function name(self)
implicit none
class (tn), intent(in) :: self
select type (component => self%cb[4])
! ALSO FAILS: "(component => self%cb)"
end select
end function name
end module gn
Tobias
[Patch, fortran] [4.7/4.8 Regression] [OOP] ICE on invalid: gfc_variable_attr(): Bad array reference
Dear All, As noted by Janus in comment #2 of the PR, "I think the function 'copy_ts_from_selector_to_associate' comes too early (namely during parsing). It tries to resolve the target expr, which should rather wait until resolution stage!?!" It turned out that the function of the call to gfc_resolve_expr was to fix up the selector array reference type. This has been done explicitly in this patch. Bootstraps and regtests on FC17/x86_64 - OK for trunk and 4.7? A fix for PRs 53876, 54990 and 54992 is on its way, as soon as I am back from getting some groceries :-) Cheers Paul 2013-01-02 Paul Thomas PR fortran/55172 * match.c (copy_ts_from_selector_to_associate): Remove call to gfc_resolve_expr and replace it with explicit setting of the array reference type. 2013-01-02 Paul Thomas PR fortran/55172 * gfortran.dg/select_type_31.f03: New test. pr55172.diff Description: Binary data
