* [Patch, fortran] PR65792 - allocation of scalar elemental function with structure constructor fails
@ 2015-04-26 18:35 Paul Richard Thomas
2015-04-26 18:53 ` Steve Kargl
0 siblings, 1 reply; 8+ messages in thread
From: Paul Richard Thomas @ 2015-04-26 18:35 UTC (permalink / raw)
To: Mikael Morin, Andre Vehreschild, fortran, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 932 bytes --]
Dear All,
This patch exploits Mikael's identification of the location of the
problem and his testcase. It requires Andre's patch for PR59678 as a
prerequisite. It is almost certainly a bit over-conservative but it
seems to be free of any memory leakage. The ChangeLog and the comments
describe it sufficiently for review.
OK for trunk and 5.x?
Paul
2015-04-26 Mikael Morin <mikael@gcc.gnu.org
Paul Thomas <pault@gcc.gnu.org>
PR fortran/65792
* trans-expr.c (gfc_trans_subcomponent_assign): Always assign
the expression component to the destination. In addition, if
the component has allocatable components, copy them and
deallocate those of the expression, if it is not a variable.
The expression is fixed if not a variable to prevent multiple
evaluations.
2015-04-26 Mikael Morin <mikael@gcc.gnu.org>
PR fortran/65792
* gfortran.dg/derived_constructor_components_5: New test
[-- Attachment #2: submit.diff --]
[-- Type: text/plain, Size: 1944 bytes --]
Index: gcc/fortran/trans-expr.c
===================================================================
*** gcc/fortran/trans-expr.c (revision 222439)
--- gcc/fortran/trans-expr.c (working copy)
*************** gfc_trans_subcomponent_assign (tree dest
*** 7062,7080 ****
{
if (expr->expr_type != EXPR_STRUCTURE)
{
gfc_init_se (&se, NULL);
gfc_conv_expr (&se, expr);
gfc_add_block_to_block (&block, &se.pre);
if (cm->ts.u.derived->attr.alloc_comp
! && expr->expr_type == EXPR_VARIABLE)
{
tmp = gfc_copy_alloc_comp (cm->ts.u.derived, se.expr,
dest, expr->rank);
gfc_add_expr_to_block (&block, tmp);
}
- else
- gfc_add_modify (&block, dest,
- fold_convert (TREE_TYPE (dest), se.expr));
gfc_add_block_to_block (&block, &se.post);
}
else
--- 7062,7091 ----
{
if (expr->expr_type != EXPR_STRUCTURE)
{
+ tree dealloc = NULL_TREE;
gfc_init_se (&se, NULL);
gfc_conv_expr (&se, expr);
gfc_add_block_to_block (&block, &se.pre);
+ /* Prevent repeat evaluations in gfc_copy_alloc_comp by fixing the
+ expression in a temporary variable and deallocate is allocatable
+ components the copy to the result. */
if (cm->ts.u.derived->attr.alloc_comp
! && expr->expr_type != EXPR_VARIABLE)
! {
! se.expr = gfc_evaluate_now (se.expr, &block);
! dealloc = gfc_deallocate_alloc_comp (cm->ts.u.derived, se.expr,
! expr->rank);
! }
! gfc_add_modify (&block, dest,
! fold_convert (TREE_TYPE (dest), se.expr));
! if (cm->ts.u.derived->attr.alloc_comp)
{
tmp = gfc_copy_alloc_comp (cm->ts.u.derived, se.expr,
dest, expr->rank);
gfc_add_expr_to_block (&block, tmp);
+ if (dealloc != NULL_TREE)
+ gfc_add_expr_to_block (&block, dealloc);
}
gfc_add_block_to_block (&block, &se.post);
}
else
[-- Attachment #3: derived_constructor_comps_5.f90 --]
[-- Type: text/plain, Size: 1647 bytes --]
! { dg-do run }
!
! PR fortran/65792
! The evaluation of the argument in the call to new_prt_spec2
! failed to properly initialize the comp component.
! While the array contents were properly copied, the array bounds remained
! uninitialized.
!
! Contributed by Dominique D'Humieres <dominiq@lps.ens.fr>
program main
implicit none
integer, parameter :: n = 2
type :: string_t
character(LEN=1), dimension(:), allocatable :: chars
end type string_t
type :: string_container_t
type(string_t) :: comp
end type string_container_t
type(string_t) :: prt_in, tmp, tmpa(n)
type(string_container_t) :: tmpc, tmpca(n)
integer :: i, j, k
do i=1,2
! scalar elemental function with structure constructor
prt_in = string_t(["D"])
tmpc = new_prt_spec2 (string_container_t(prt_in))
if (any(tmpc%comp%chars .ne. ["D"])) call abort
deallocate (prt_in%chars)
deallocate(tmpc%comp%chars)
! Check that function arguments are OK too
tmpc = new_prt_spec2 (string_container_t(new_str_t(["h","e","l","l","o"])))
if (any(tmpc%comp%chars .ne. ["h","e","l","l","o"])) call abort
deallocate(tmpc%comp%chars)
end do
contains
impure elemental function new_prt_spec2 (name) result (prt_spec)
type(string_container_t), intent(in) :: name
type(string_container_t) :: prt_spec
prt_spec = name
end function new_prt_spec2
function new_str_t (name) result (prt_spec)
character (*), intent(in), dimension (:) :: name
type(string_t) :: prt_spec
prt_spec = string_t(name)
end function new_str_t
end program main
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Patch, fortran] PR65792 - allocation of scalar elemental function with structure constructor fails
2015-04-26 18:35 [Patch, fortran] PR65792 - allocation of scalar elemental function with structure constructor fails Paul Richard Thomas
@ 2015-04-26 18:53 ` Steve Kargl
2015-04-27 20:42 ` Paul Richard Thomas
0 siblings, 1 reply; 8+ messages in thread
From: Steve Kargl @ 2015-04-26 18:53 UTC (permalink / raw)
To: Paul Richard Thomas; +Cc: Mikael Morin, Andre Vehreschild, fortran, gcc-patches
On Sun, Apr 26, 2015 at 08:35:06PM +0200, Paul Richard Thomas wrote:
>
> --- 7062,7091 ----
> {
> if (expr->expr_type != EXPR_STRUCTURE)
> {
> + tree dealloc = NULL_TREE;
> gfc_init_se (&se, NULL);
> gfc_conv_expr (&se, expr);
> gfc_add_block_to_block (&block, &se.pre);
> + /* Prevent repeat evaluations in gfc_copy_alloc_comp by fixing the
> + expression in a temporary variable and deallocate is allocatable
> + components the copy to the result. */
Can you take a second shot at this comment? The "and ..." portions
seems to be a little muddled.
OK with after comment fix.
--
Steve
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Patch, fortran] PR65792 - allocation of scalar elemental function with structure constructor fails
2015-04-26 18:53 ` Steve Kargl
@ 2015-04-27 20:42 ` Paul Richard Thomas
2015-05-01 18:25 ` Paul Richard Thomas
0 siblings, 1 reply; 8+ messages in thread
From: Paul Richard Thomas @ 2015-04-27 20:42 UTC (permalink / raw)
To: Steve Kargl; +Cc: Mikael Morin, Andre Vehreschild, fortran, gcc-patches
Dear Steve,
Thanks for the review. I THINK that I know what I meant in the comment
:-) I will commit tomorrow night.
Cheers
Paul
On 26 April 2015 at 20:53, Steve Kargl <sgk@troutmask.apl.washington.edu> wrote:
> On Sun, Apr 26, 2015 at 08:35:06PM +0200, Paul Richard Thomas wrote:
>>
>> --- 7062,7091 ----
>> {
>> if (expr->expr_type != EXPR_STRUCTURE)
>> {
>> + tree dealloc = NULL_TREE;
>> gfc_init_se (&se, NULL);
>> gfc_conv_expr (&se, expr);
>> gfc_add_block_to_block (&block, &se.pre);
>> + /* Prevent repeat evaluations in gfc_copy_alloc_comp by fixing the
>> + expression in a temporary variable and deallocate is allocatable
>> + components the copy to the result. */
>
> Can you take a second shot at this comment? The "and ..." portions
> seems to be a little muddled.
>
> OK with after comment fix.
>
> --
> Steve
--
Outside of a dog, a book is a man's best friend. Inside of a dog it's
too dark to read.
Groucho Marx
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Patch, fortran] PR65792 - allocation of scalar elemental function with structure constructor fails
2015-04-27 20:42 ` Paul Richard Thomas
@ 2015-05-01 18:25 ` Paul Richard Thomas
2015-05-09 13:13 ` Mikael Morin
0 siblings, 1 reply; 8+ messages in thread
From: Paul Richard Thomas @ 2015-05-01 18:25 UTC (permalink / raw)
To: Steve Kargl; +Cc: Mikael Morin, Andre Vehreschild, fortran, gcc-patches
Dear All,
By the time I went to commit, something had changed and the patch
caused a regression. I presume that the version that I had of Andre's
patch was not the same as the one committed. I'll cast an eye over it
this weekend and see if I can understand what gives.
Cheers
paul
On 27 April 2015 at 22:42, Paul Richard Thomas
<paul.richard.thomas@gmail.com> wrote:
> Dear Steve,
>
> Thanks for the review. I THINK that I know what I meant in the comment
> :-) I will commit tomorrow night.
>
> Cheers
>
> Paul
>
> On 26 April 2015 at 20:53, Steve Kargl <sgk@troutmask.apl.washington.edu> wrote:
>> On Sun, Apr 26, 2015 at 08:35:06PM +0200, Paul Richard Thomas wrote:
>>>
>>> --- 7062,7091 ----
>>> {
>>> if (expr->expr_type != EXPR_STRUCTURE)
>>> {
>>> + tree dealloc = NULL_TREE;
>>> gfc_init_se (&se, NULL);
>>> gfc_conv_expr (&se, expr);
>>> gfc_add_block_to_block (&block, &se.pre);
>>> + /* Prevent repeat evaluations in gfc_copy_alloc_comp by fixing the
>>> + expression in a temporary variable and deallocate is allocatable
>>> + components the copy to the result. */
>>
>> Can you take a second shot at this comment? The "and ..." portions
>> seems to be a little muddled.
>>
>> OK with after comment fix.
>>
>> --
>> Steve
>
>
>
> --
> Outside of a dog, a book is a man's best friend. Inside of a dog it's
> too dark to read.
>
> Groucho Marx
--
Outside of a dog, a book is a man's best friend. Inside of a dog it's
too dark to read.
Groucho Marx
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Patch, fortran] PR65792 - allocation of scalar elemental function with structure constructor fails
2015-05-01 18:25 ` Paul Richard Thomas
@ 2015-05-09 13:13 ` Mikael Morin
2015-05-15 17:02 ` *ping* " Mikael Morin
0 siblings, 1 reply; 8+ messages in thread
From: Mikael Morin @ 2015-05-09 13:13 UTC (permalink / raw)
To: Paul Richard Thomas, Steve Kargl; +Cc: Andre Vehreschild, fortran, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 478 bytes --]
Le 01/05/2015 20:25, Paul Richard Thomas a écrit :
> Dear All,
>
> By the time I went to commit, something had changed and the patch
> caused a regression. I presume that the version that I had of Andre's
> patch was not the same as the one committed. I'll cast an eye over it
> this weekend and see if I can understand what gives.
>
Hello Paul,
to get things moving again, I propose the attached fix to your patch.
Tested on alloc_comp_deep_copy_1 only for now.
Mikael
[-- Attachment #2: pr65792_fix.diff --]
[-- Type: text/x-patch, Size: 572 bytes --]
diff --git a/gcc/fortran/trans-expr.c b/gcc/fortran/trans-expr.c
index 6cf5fe2..532f4b7 100644
--- a/gcc/fortran/trans-expr.c
+++ b/gcc/fortran/trans-expr.c
@@ -7078,7 +7078,8 @@ gfc_trans_subcomponent_assign (tree dest, gfc_component * cm, gfc_expr * expr,
}
gfc_add_modify (&block, dest,
fold_convert (TREE_TYPE (dest), se.expr));
- if (cm->ts.u.derived->attr.alloc_comp)
+ if (cm->ts.u.derived->attr.alloc_comp
+ && expr->expr_type != EXPR_NULL)
{
tmp = gfc_copy_alloc_comp (cm->ts.u.derived, se.expr,
dest, expr->rank);
^ permalink raw reply [flat|nested] 8+ messages in thread
* *ping* Re: [Patch, fortran] PR65792 - allocation of scalar elemental function with structure constructor fails
2015-05-09 13:13 ` Mikael Morin
@ 2015-05-15 17:02 ` Mikael Morin
2015-05-15 18:07 ` Paul Richard Thomas
2015-05-16 9:39 ` Paul Richard Thomas
0 siblings, 2 replies; 8+ messages in thread
From: Mikael Morin @ 2015-05-15 17:02 UTC (permalink / raw)
To: Paul Richard Thomas, Steve Kargl; +Cc: Andre Vehreschild, fortran, gcc-patches
Le 09/05/2015 15:12, Mikael Morin a écrit :
> Le 01/05/2015 20:25, Paul Richard Thomas a écrit :
>> Dear All,
>>
>> By the time I went to commit, something had changed and the patch
>> caused a regression. I presume that the version that I had of Andre's
>> patch was not the same as the one committed. I'll cast an eye over it
>> this weekend and see if I can understand what gives.
>>
> Hello Paul,
>
> to get things moving again, I propose the attached fix to your patch.
> Tested on alloc_comp_deep_copy_1 only for now.
Hello Paul,
any news?
Mikael
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: *ping* Re: [Patch, fortran] PR65792 - allocation of scalar elemental function with structure constructor fails
2015-05-15 17:02 ` *ping* " Mikael Morin
@ 2015-05-15 18:07 ` Paul Richard Thomas
2015-05-16 9:39 ` Paul Richard Thomas
1 sibling, 0 replies; 8+ messages in thread
From: Paul Richard Thomas @ 2015-05-15 18:07 UTC (permalink / raw)
To: Mikael Morin; +Cc: Steve Kargl, Andre Vehreschild, fortran, gcc-patches
Dear Mikael, dear all,
I am struggling to find time for gfortran at present because of the
change in my personal circumstances. I am still working full time as a
consultant, whilst trying to sell the house and fix up all those
things that l'Administration Francaise makes so "interesting" (Having
had quasi-diplomatic status, there are all sorts of issues involved in
cars, pensions and... yes, the 2014 tax return). Things will improve
on a timescale of three months because I am expecting an acceptable
offer for the house, next week.
In the meantime, to show willing, I will attend to this right now :-)
Cheers
Paul
On 15 May 2015 at 19:01, Mikael Morin <mikael.morin@sfr.fr> wrote:
> Le 09/05/2015 15:12, Mikael Morin a écrit :
>> Le 01/05/2015 20:25, Paul Richard Thomas a écrit :
>>> Dear All,
>>>
>>> By the time I went to commit, something had changed and the patch
>>> caused a regression. I presume that the version that I had of Andre's
>>> patch was not the same as the one committed. I'll cast an eye over it
>>> this weekend and see if I can understand what gives.
>>>
>> Hello Paul,
>>
>> to get things moving again, I propose the attached fix to your patch.
>> Tested on alloc_comp_deep_copy_1 only for now.
>
> Hello Paul,
>
> any news?
>
> Mikael
--
Outside of a dog, a book is a man's best friend. Inside of a dog it's
too dark to read.
Groucho Marx
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: *ping* Re: [Patch, fortran] PR65792 - allocation of scalar elemental function with structure constructor fails
2015-05-15 17:02 ` *ping* " Mikael Morin
2015-05-15 18:07 ` Paul Richard Thomas
@ 2015-05-16 9:39 ` Paul Richard Thomas
1 sibling, 0 replies; 8+ messages in thread
From: Paul Richard Thomas @ 2015-05-16 9:39 UTC (permalink / raw)
To: Mikael Morin; +Cc: Steve Kargl, Andre Vehreschild, fortran, gcc-patches
Dear Mikael,
With your tweak it bootstrapped and regtested OK. Also changed the
comment after Steve's pointing out that the last sentence was
incomprehensible:-)
Committed to trunk as revision 223234.
Cheers
Paul
2015-05-16 Mikael Morin <mikael@gcc.gnu.org
Paul Thomas <pault@gcc.gnu.org>
PR fortran/65792
* trans-expr.c (gfc_trans_subcomponent_assign): Always assign
the expression component to the destination. In addition, if
the component has allocatable components, copy them and
deallocate those of the expression, if it is not a variable.
The expression is fixed if not a variable to prevent multiple
evaluations.
2015-05-16 Mikael Morin <mikael@gcc.gnu.org>
PR fortran/65792
* gfortran.dg/derived_constructor_components_5: New test
On 15 May 2015 at 19:01, Mikael Morin <mikael.morin@sfr.fr> wrote:
> Le 09/05/2015 15:12, Mikael Morin a écrit :
>> Le 01/05/2015 20:25, Paul Richard Thomas a écrit :
>>> Dear All,
>>>
>>> By the time I went to commit, something had changed and the patch
>>> caused a regression. I presume that the version that I had of Andre's
>>> patch was not the same as the one committed. I'll cast an eye over it
>>> this weekend and see if I can understand what gives.
>>>
>> Hello Paul,
>>
>> to get things moving again, I propose the attached fix to your patch.
>> Tested on alloc_comp_deep_copy_1 only for now.
>
> Hello Paul,
>
> any news?
>
> Mikael
--
Outside of a dog, a book is a man's best friend. Inside of a dog it's
too dark to read.
Groucho Marx
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2015-05-16 8:13 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-26 18:35 [Patch, fortran] PR65792 - allocation of scalar elemental function with structure constructor fails Paul Richard Thomas
2015-04-26 18:53 ` Steve Kargl
2015-04-27 20:42 ` Paul Richard Thomas
2015-05-01 18:25 ` Paul Richard Thomas
2015-05-09 13:13 ` Mikael Morin
2015-05-15 17:02 ` *ping* " Mikael Morin
2015-05-15 18:07 ` Paul Richard Thomas
2015-05-16 9:39 ` Paul Richard Thomas
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).