Re: [Patch, fortran, PR44672, v6] [F08] ALLOCATE with SOURCE and no array-spec
Hi,
no, I lost patience with this. Attached is the last version of this patch I will
provide. I have changed some things to fix the issues needed for
allocate_with_source_3.f90 (now called allocate_with_source_8.f08) to run. The
fix implies that source=[type_constructor(...)] generate a zero-bound array,
which is not correct according to the standard, but I can't do anything about
it you will like.
I have thought some more about the code not distinguishing source vs mold.
It seems to me that it makes sense to _not_ distinguish, and what you do
with e3_is == E3_MOLD seems bogus to me. For example:
@@ -5391,6 +5398,12 @@ gfc_trans_allocate (gfc_code * code)
}
gcc_assert (expr3_esize);
expr3_esize = fold_convert (sizetype, expr3_esize);
+ if (e3_is == E3_MOLD)
+ {
+ /* The expr3 is no longer valid after this point. */
+ expr3 = NULL_TREE;
+ e3_is = E3_UNSET;
+ }
}
else if (code-ext.alloc.ts.type != BT_UNKNOWN)
{
You forget about the descriptor you have just created?!?
+ e3_is = expr3 != NULL_TREE ?
+ (code-ext.alloc.arr_spec_from_expr3 ?
+ E3_DESC
+: (code-expr3-mold ? E3_MOLD : E3_SOURCE))
+ : E3_UNSET;
No, a E3_DESC is set before a E3_MOLD, therefore the reset in the chunk above
is not triggered.
I can't spend more resources on this. When you see the need of changes, you are
welcome to add them.
- Andre
--
Andre Vehreschild * Email: vehre ad gmx dot de
diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h
index 905d47c..211c781 100644
--- a/gcc/fortran/gfortran.h
+++ b/gcc/fortran/gfortran.h
@@ -2396,6 +2396,9 @@ typedef struct gfc_code
{
gfc_typespec ts;
gfc_alloc *list;
+ /* Take the array specification from expr3 to allocate arrays
+ without an explicit array specification. */
+ unsigned arr_spec_from_expr3:1;
}
alloc;
diff --git a/gcc/fortran/resolve.c b/gcc/fortran/resolve.c
index e615cc6..315170a 100644
--- a/gcc/fortran/resolve.c
+++ b/gcc/fortran/resolve.c
@@ -6805,7 +6805,7 @@ conformable_arrays (gfc_expr *e1, gfc_expr *e2)
have a trailing array reference that gives the size of the array. */
static bool
-resolve_allocate_expr (gfc_expr *e, gfc_code *code)
+resolve_allocate_expr (gfc_expr *e, gfc_code *code, bool *array_alloc_wo_spec)
{
int i, pointer, allocatable, dimension, is_abstract;
int codimension;
@@ -7104,13 +7104,24 @@ resolve_allocate_expr (gfc_expr *e, gfc_code *code)
if (!ref2 || ref2-type != REF_ARRAY || ref2-u.ar.type == AR_FULL
|| (dimension ref2-u.ar.dimen == 0))
{
- gfc_error (Array specification required in ALLOCATE statement
- at %L, e-where);
- goto failure;
+ /* F08:C633. */
+ if (code-expr3)
+ {
+ if (!gfc_notify_std (GFC_STD_F2008, Array specification required
+ in ALLOCATE statement at %L, e-where))
+ goto failure;
+ *array_alloc_wo_spec = true;
+ }
+ else
+ {
+ gfc_error (Array specification required in ALLOCATE statement
+ at %L, e-where);
+ goto failure;
+ }
}
/* Make sure that the array section reference makes sense in the
-context of an ALLOCATE specification. */
+ context of an ALLOCATE specification. */
ar = ref2-u.ar;
@@ -7125,7 +7136,7 @@ resolve_allocate_expr (gfc_expr *e, gfc_code *code)
for (i = 0; i ar-dimen; i++)
{
- if (ref2-u.ar.type == AR_ELEMENT)
+ if (ar-type == AR_ELEMENT || ar-type == AR_FULL)
goto check_symbols;
switch (ar-dimen_type[i])
@@ -7202,6 +7213,7 @@ failure:
return false;
}
+
static void
resolve_allocate_deallocate (gfc_code *code, const char *fcn)
{
@@ -7376,8 +7388,16 @@ resolve_allocate_deallocate (gfc_code *code, const char *fcn)
if (strcmp (fcn, ALLOCATE) == 0)
{
+ bool arr_alloc_wo_spec = false;
for (a = code-ext.alloc.list; a; a = a-next)
- resolve_allocate_expr (a-expr, code);
+ resolve_allocate_expr (a-expr, code, arr_alloc_wo_spec);
+
+ if (arr_alloc_wo_spec code-expr3)
+ {
+ /* Mark the allocate to have to take the array specification
+ from the expr3. */
+ code-ext.alloc.arr_spec_from_expr3 = 1;
+ }
}
else
{
diff --git a/gcc/fortran/trans-array.c b/gcc/fortran/trans-array.c
index c8fab45..9767e9d 100644
--- a/gcc/fortran/trans-array.c
+++ b/gcc/fortran/trans-array.c
@@ -5005,7 +5005,8 @@ static tree
gfc_array_init_size (tree descriptor, int rank, int corank, tree * poffset,
gfc_expr ** lower, gfc_expr ** upper, stmtblock_t * pblock,
stmtblock_t * descriptor_block, tree * overflow,
- tree expr3_elem_size, tree *nelems, gfc_expr *expr3)
+ tree expr3_elem_size, tree *nelems, gfc_expr *expr3,
+ tree expr3_desc)
{
tree type;
tree tmp;
@@ -5020,7 +5021,7 @@ gfc_array_init_size (tree descriptor, int rank, int corank, tree * poffset,
tree var;
stmtblock_t thenblock;
stmtblock_t elseblock;
-
Re: [Patch, fortran, PR44672, v6] [F08] ALLOCATE with SOURCE and no array-spec
Hello Andre,
comments below (out of order, sorry).
Le 29/05/2015 13:46, Andre Vehreschild a écrit :
Hi Mikael,
comments inline below:
On Thu, 28 May 2015 20:06:57 +0200
Mikael Morin mikael.mo...@sfr.fr wrote:
Le 28/05/2015 17:29, Andre Vehreschild a écrit :
*** resolve_allocate_expr (gfc_expr *e, gfc_
*** 7103,7112
--- 7103,7123
if (!ref2 || ref2-type != REF_ARRAY || ref2-u.ar.type == AR_FULL
|| (dimension ref2-u.ar.dimen == 0))
{
+ /* F08:C633. */
+ if (code-expr3)
+ {
+ if (!gfc_notify_std (GFC_STD_F2008, Array specification
required
+ in ALLOCATE statement at %L, e-where))
+ goto failure;
+ *array_alloc_wo_spec = true;
+ }
+ else
+ {
gfc_error (Array specification required in ALLOCATE statement
at %L, e-where);
goto failure;
}
+ }
/* Make sure that the array section reference makes sense in the
context of an ALLOCATE specification. */
I think we can be a little be more user friendly with the gfc_notify_std
error message.
Something like:
ALLOCATE without array spec at %L
ALLOCATE with array bounds determined from SOURCE or MOLD at %L
I didn't want to mess with the error messages to prevent issues for
translations. So how is the policy on this?
I'm not aware of any policy regarding translations.
With a message like:
fortran 2008: array specification required ...
I don't see how the user can understand that the array specification is
_not_ required with fortran 2008, regardless of translations.
I'm rather in favour of not having misleading diagnostic, even if
correctly translated.
*** gfc_array_init_size (tree descriptor, in
*** 5076,5085
/* Set upper bound. */
gfc_init_se (se, NULL);
gcc_assert (ubound);
gfc_conv_expr_type (se, ubound, gfc_array_index_type);
gfc_add_block_to_block (pblock, se.pre);
!
gfc_conv_descriptor_ubound_set (descriptor_block, descriptor,
gfc_rank_cst[n], se.expr);
conv_ubound = se.expr;
--- 5087,5111
/* Set upper bound. */
gfc_init_se (se, NULL);
+ if (expr3_desc != NULL_TREE)
+ {
+ /* Set the upper bound to be (desc.ubound - desc.lbound)+ 1. */
+ tmp = fold_build2_loc (input_location, MINUS_EXPR,
+gfc_array_index_type,
+gfc_conv_descriptor_ubound_get (
+ expr3_desc, gfc_rank_cst[n]),
+gfc_conv_descriptor_lbound_get (
+ expr3_desc, gfc_rank_cst[n]));
+ se.expr = fold_build2_loc (input_location, PLUS_EXPR,
+gfc_array_index_type, tmp,
+gfc_index_one_node);
+ }
+ else
+ {
gcc_assert (ubound);
gfc_conv_expr_type (se, ubound, gfc_array_index_type);
gfc_add_block_to_block (pblock, se.pre);
! }
gfc_conv_descriptor_ubound_set (descriptor_block, descriptor,
gfc_rank_cst[n], se.expr);
conv_ubound = se.expr;
Your one-based-ness problem was here, wasn't it?
Correct.
I would rather copy directly lbound and ubound from expr3_desc to
descriptor.
It was that way in the previous version of the patch, which does *not* work
any
longer. When gfc_trans_allocate () is responsible for the creating a temporary
variable for the source=-expression, then it does so using zero based
expressions.
If the source has non-one-based bounds, the above would produce wrong
bounds.
Counterexample? Note, the expr3_desc is guaranteed to be an artificial
variable
created by conv_expr_descriptor, aka zero-based.
here is a counterexample.
integer, dimension(:), allocatable :: a, b
allocate (a(0:3))
allocate (b, source = a)
print *, lbound(a, 1), ubound(a, 1)
print *, lbound(b, 1), ubound(b, 1)
end
output:
0 3
1 4
I think that if you set se.expr with
ubound with gfc_conv_descriptor_ubound_get(...) instead of what you do
above, and se.expr with gfc_conv_descriptor_lbound_get(...) instead of
gfc_index_one_node in the hunk before, it should work.
snipp
*** gfc_trans_allocate (gfc_code * code)
*** 5229,5235
}
else
tmp = se.expr;
! if (!code-expr3-mold)
expr3 = tmp;
else
expr3_tmp = tmp;
--- 5240,5248
}
else
tmp = se.expr;
! if (code-ext.alloc.arr_spec_from_expr3)
! expr3_desc = tmp;
! else if (!code-expr3-mold)
expr3 = tmp;
else
expr3_tmp = tmp;
Couldn't expr3 be reused?
We had code-expr3, expr3, expr3rhs, and now this is adding expr3_desc,
and (below)
Re: [Patch, fortran, PR44672, v6] [F08] ALLOCATE with SOURCE and no array-spec
Hi Mikael,
comments inline below:
On Thu, 28 May 2015 20:06:57 +0200
Mikael Morin mikael.mo...@sfr.fr wrote:
Le 28/05/2015 17:29, Andre Vehreschild a écrit :
*** resolve_allocate_expr (gfc_expr *e, gfc_
*** 7103,7112
--- 7103,7123
if (!ref2 || ref2-type != REF_ARRAY || ref2-u.ar.type == AR_FULL
|| (dimension ref2-u.ar.dimen == 0))
{
+ /* F08:C633. */
+ if (code-expr3)
+ {
+ if (!gfc_notify_std (GFC_STD_F2008, Array specification
required
+ in ALLOCATE statement at %L, e-where))
+ goto failure;
+ *array_alloc_wo_spec = true;
+ }
+ else
+ {
gfc_error (Array specification required in ALLOCATE statement
at %L, e-where);
goto failure;
}
+ }
/* Make sure that the array section reference makes sense in the
context of an ALLOCATE specification. */
I think we can be a little be more user friendly with the gfc_notify_std
error message.
Something like:
ALLOCATE without array spec at %L
ALLOCATE with array bounds determined from SOURCE or MOLD at %L
I didn't want to mess with the error messages to prevent issues for
translations. So how is the policy on this?
*** gfc_array_init_size (tree descriptor, in
*** 5044,5053
lower == NULL= lbound = 1, ubound = upper[n]
upper[n] = NULL = lbound = 1, ubound = lower[n]
upper[n] != NULL = lbound = lower[n], ubound = upper[n] */
- ubound = upper[n];
/* Set lower bound. */
gfc_init_se (se, NULL);
if (lower == NULL)
se.expr = gfc_index_one_node;
else
--- 5050,5063
lower == NULL= lbound = 1, ubound = upper[n]
upper[n] = NULL = lbound = 1, ubound = lower[n]
upper[n] != NULL = lbound = lower[n], ubound = upper[n] */
/* Set lower bound. */
gfc_init_se (se, NULL);
+ if (expr3_desc != NULL_TREE)
+ se.expr = gfc_index_one_node;
+ else
+ {
+ ubound = upper[n];
if (lower == NULL)
se.expr = gfc_index_one_node;
else
*** gfc_array_init_size (tree descriptor, in
*** 5064,5069
--- 5074,5080
ubound = lower[n];
}
}
+ }
gfc_conv_descriptor_lbound_set (descriptor_block, descriptor,
gfc_rank_cst[n], se.expr);
conv_lbound = se.expr;
You can avoid reindenting if the ubound = upper[n] statement is kept at
its original place.
Fixed.
*** gfc_array_init_size (tree descriptor, in
*** 5076,5085
/* Set upper bound. */
gfc_init_se (se, NULL);
gcc_assert (ubound);
gfc_conv_expr_type (se, ubound, gfc_array_index_type);
gfc_add_block_to_block (pblock, se.pre);
!
gfc_conv_descriptor_ubound_set (descriptor_block, descriptor,
gfc_rank_cst[n], se.expr);
conv_ubound = se.expr;
--- 5087,5111
/* Set upper bound. */
gfc_init_se (se, NULL);
+ if (expr3_desc != NULL_TREE)
+ {
+ /* Set the upper bound to be (desc.ubound - desc.lbound)+ 1. */
+ tmp = fold_build2_loc (input_location, MINUS_EXPR,
+gfc_array_index_type,
+gfc_conv_descriptor_ubound_get (
+ expr3_desc, gfc_rank_cst[n]),
+gfc_conv_descriptor_lbound_get (
+ expr3_desc, gfc_rank_cst[n]));
+ se.expr = fold_build2_loc (input_location, PLUS_EXPR,
+gfc_array_index_type, tmp,
+gfc_index_one_node);
+ }
+ else
+ {
gcc_assert (ubound);
gfc_conv_expr_type (se, ubound, gfc_array_index_type);
gfc_add_block_to_block (pblock, se.pre);
! }
gfc_conv_descriptor_ubound_set (descriptor_block, descriptor,
gfc_rank_cst[n], se.expr);
conv_ubound = se.expr;
Your one-based-ness problem was here, wasn't it?
Correct.
I would rather copy directly lbound and ubound from expr3_desc to
descriptor.
It was that way in the previous version of the patch, which does *not* work any
longer. When gfc_trans_allocate () is responsible for the creating a temporary
variable for the source=-expression, then it does so using zero based
expressions.
If the source has non-one-based bounds, the above would produce wrong
bounds.
Counterexample? Note, the expr3_desc is guaranteed to be an artificial variable
created by conv_expr_descriptor, aka zero-based.
snipp
*** gfc_trans_allocate (gfc_code * code)
*** 5229,5235
}
else
tmp = se.expr;
! if (!code-expr3-mold)
Re: [Patch, fortran, PR44672, v6] [F08] ALLOCATE with SOURCE and no array-spec
Hi Andre, just a couple of remarks. You are adding significant new code to an existing test case, allocate_with_source_3.f90. As discussed previously, it would be better to put the new code into an extra test case. The following test case segfaults with your patch with an invalid free: module foo contains integer function f() f = 2 end function f end module foo program main use foo integer :: n n = 42 block real, dimension(0:n) :: a real, dimension(:), allocatable :: c call random_number(a) allocate(c,source=a(:f())) end block end program main You could also add n = n - 1 allocate(c,source=a) if (size(a,1) /= size(c,1)) call abort to the test case above to make sure that changing a variable that was used to declare an array bound does not lead to wrong code. Regards Thomas
Re: [Patch, fortran, PR44672, v6] [F08] ALLOCATE with SOURCE and no array-spec
Le 28/05/2015 17:29, Andre Vehreschild a écrit :
*** resolve_allocate_expr (gfc_expr *e, gfc_
*** 7103,7112
--- 7103,7123
if (!ref2 || ref2-type != REF_ARRAY || ref2-u.ar.type == AR_FULL
|| (dimension ref2-u.ar.dimen == 0))
{
+ /* F08:C633. */
+ if (code-expr3)
+ {
+ if (!gfc_notify_std (GFC_STD_F2008, Array specification required
+in ALLOCATE statement at %L, e-where))
+ goto failure;
+ *array_alloc_wo_spec = true;
+ }
+ else
+ {
gfc_error (Array specification required in ALLOCATE statement
at %L, e-where);
goto failure;
}
+ }
/* Make sure that the array section reference makes sense in the
context of an ALLOCATE specification. */
I think we can be a little be more user friendly with the gfc_notify_std
error message.
Something like:
ALLOCATE without array spec at %L
ALLOCATE with array bounds determined from SOURCE or MOLD at %L
*** gfc_array_init_size (tree descriptor, in
*** 5044,5053
lower == NULL= lbound = 1, ubound = upper[n]
upper[n] = NULL = lbound = 1, ubound = lower[n]
upper[n] != NULL = lbound = lower[n], ubound = upper[n] */
- ubound = upper[n];
/* Set lower bound. */
gfc_init_se (se, NULL);
if (lower == NULL)
se.expr = gfc_index_one_node;
else
--- 5050,5063
lower == NULL= lbound = 1, ubound = upper[n]
upper[n] = NULL = lbound = 1, ubound = lower[n]
upper[n] != NULL = lbound = lower[n], ubound = upper[n] */
/* Set lower bound. */
gfc_init_se (se, NULL);
+ if (expr3_desc != NULL_TREE)
+ se.expr = gfc_index_one_node;
+ else
+ {
+ ubound = upper[n];
if (lower == NULL)
se.expr = gfc_index_one_node;
else
*** gfc_array_init_size (tree descriptor, in
*** 5064,5069
--- 5074,5080
ubound = lower[n];
}
}
+ }
gfc_conv_descriptor_lbound_set (descriptor_block, descriptor,
gfc_rank_cst[n], se.expr);
conv_lbound = se.expr;
You can avoid reindenting if the ubound = upper[n] statement is kept at
its original place.
*** gfc_array_init_size (tree descriptor, in
*** 5076,5085
/* Set upper bound. */
gfc_init_se (se, NULL);
gcc_assert (ubound);
gfc_conv_expr_type (se, ubound, gfc_array_index_type);
gfc_add_block_to_block (pblock, se.pre);
!
gfc_conv_descriptor_ubound_set (descriptor_block, descriptor,
gfc_rank_cst[n], se.expr);
conv_ubound = se.expr;
--- 5087,5111
/* Set upper bound. */
gfc_init_se (se, NULL);
+ if (expr3_desc != NULL_TREE)
+ {
+ /* Set the upper bound to be (desc.ubound - desc.lbound)+ 1. */
+ tmp = fold_build2_loc (input_location, MINUS_EXPR,
+ gfc_array_index_type,
+ gfc_conv_descriptor_ubound_get (
+expr3_desc, gfc_rank_cst[n]),
+ gfc_conv_descriptor_lbound_get (
+expr3_desc, gfc_rank_cst[n]));
+ se.expr = fold_build2_loc (input_location, PLUS_EXPR,
+ gfc_array_index_type, tmp,
+ gfc_index_one_node);
+ }
+ else
+ {
gcc_assert (ubound);
gfc_conv_expr_type (se, ubound, gfc_array_index_type);
gfc_add_block_to_block (pblock, se.pre);
! }
gfc_conv_descriptor_ubound_set (descriptor_block, descriptor,
gfc_rank_cst[n], se.expr);
conv_ubound = se.expr;
Your one-based-ness problem was here, wasn't it?
I would rather copy directly lbound and ubound from expr3_desc to
descriptor.
If the source has non-one-based bounds, the above would produce wrong
bounds.
*** gfc_trans_allocate (gfc_code * code)
*** 5174,5185
{
if (!code-expr3-mold
|| code-expr3-ts.type == BT_CHARACTER
! || vtab_needed)
{
/* Convert expr3 to a tree. */
gfc_init_se (se, NULL);
! /* For all simple expression just get the descriptor or the
! reference, respectively, depending on the rank of the expr. */
if (code-expr3-rank != 0)
gfc_conv_expr_descriptor (se, code-expr3);
else
--- 5175,5195
{
if (!code-expr3-mold
|| code-expr3-ts.type == BT_CHARACTER
! || vtab_needed
! || code-ext.alloc.arr_spec_from_expr3)
{
/* Convert expr3 to a tree. */
Re: [Patch, fortran, PR44672, v6] [F08] ALLOCATE with SOURCE and no array-spec
Hi Mikael,
thanks for the comments so far.
I don't understand why one of your previous patches was factoring the
source expression evaluation to a temporary in gfc_trans_allocate, and
now with this patch you do the same thing in gfc_resolve_allocate, not
reusing the part in gfc_trans_allocate.
When I remember correctly, then at the time of writing this patch the one
factoring out the temporary in gfc_trans_allocate() was not doing that yet. At
least it was not doing it always as needed. Therefore we are looking at a kind
of history here already.
*** failure:
*** 7201,7212
--- 7212,7229
return false;
}
+
static void
resolve_allocate_deallocate (gfc_code *code, const char *fcn)
{
gfc_expr *stat, *errmsg, *pe, *qe;
gfc_alloc *a, *p, *q;
+ /* When this flag is set already, then this allocate has already been
+ resolved. Doing so again, would result in an endless loop. */
+ if (code-ext.alloc.arr_spec_from_expr3)
+ return;
+
I expect you'll miss some error messages by doing this.
Where is the endless loop?
This has been removed. The endless loop was triggered by gfc_resolve_code () in
line 179 of the patch, which is now in chunk that is mostly removed.
*** resolve_allocate_deallocate (gfc_code *c
*** 7375,7382
--- 7392,7500
if (strcmp (fcn, ALLOCATE) == 0)
{
+ bool arr_alloc_wo_spec = false;
for (a = code-ext.alloc.list; a; a = a-next)
! resolve_allocate_expr (a-expr, code, arr_alloc_wo_spec);
!
! if (arr_alloc_wo_spec code-expr3)
! {
[...]
!
! ass = gfc_get_code (EXEC_ASSIGN);
This memory is not freed as far as I know.
I think you can use a local variable for it.
Complete block removed. Therefore fixed.
*** /tmp/PRaWHc_trans-expr.c 2015-05-25 19:54:35.056309429 +0200
--- /tmp/7e82nd_trans-expr.c 2015-05-25 19:54:35.058309429 +0200
*** gfc_conv_procedure_call (gfc_se * se, gf
*** 5328,5334
if (e (e-ts.type == BT_DERIVED || e-ts.type == BT_CLASS)
e-ts.u.derived-attr.alloc_comp
!(e-symtree e-symtree-n.sym-attr.pointer)
! (e-expr_type != EXPR_VARIABLE !e-rank))
{
int parm_rank;
tmp = build_fold_indirect_ref_loc (input_location,
--- 5328,5335
if (e (e-ts.type == BT_DERIVED || e-ts.type == BT_CLASS)
e-ts.u.derived-attr.alloc_comp
!(e-symtree e-symtree-n.sym-attr.pointer)
! e-expr_type != EXPR_VARIABLE !e-rank
! e-expr_type != EXPR_STRUCTURE)
{
int parm_rank;
tmp = build_fold_indirect_ref_loc (input_location,
Can't you remove this? It's undone by the PR58586 patch.
Removed, looks like an artefact of a long forgotten need.
*** gfc_trans_allocate (gfc_code * code)
*** 5733,5746
if (dataref dataref-u.c.component-as)
{
! int dim;
gfc_expr *temp;
gfc_ref *ref = dataref-next;
ref-u.ar.type = AR_SECTION;
/* We have to set up the array reference to give ranges
in all dimensions and ensure that the end and stride
are set so that the copy can be scalarized. */
- dim = 0;
for (; dim dataref-u.c.component-as-rank; dim++)
{
ref-u.ar.dimen_type[dim] = DIMEN_RANGE;
--- 5758,5815
if (dataref dataref-u.c.component-as)
{
! int dim = 0;
gfc_expr *temp;
gfc_ref *ref = dataref-next;
ref-u.ar.type = AR_SECTION;
+ if (code-ext.alloc.arr_spec_from_expr3)
+ {
+ /* Take the array dimensions from the
+source=-expression. */
+ gfc_array_ref *source_ref =
+ gfc_find_array_ref (code-expr3);
Does this work? code-expr3 is not always a variable.
The block removed from resolve_allocate() ensured, that this was always a
variable. Therefore, yes, it had to work then. Now, we of course have far more
trouble.
+ if (source_ref-type == AR_FULL)
+ {
+ /* For full array refs copy the bounds. */
+ for (; dim dataref-u.c.component-as-rank;
dim++)
+ {
+ ref-u.ar.dimen_type[dim] = DIMEN_RANGE;
+ ref-u.ar.start[dim] =
+ gfc_copy_expr
(source_ref-as-lower[dim]);
+ ref-u.ar.end[dim] =
+ gfc_copy_expr
(source_ref-as-upper[dim]);
+ }
This won't work. Consider this:
block
integer :: a(n)
n = n+1
Re: [Patch, fortran, PR44672, v6] [F08] ALLOCATE with SOURCE and no array-spec
Le 19/05/2015 12:26, Andre Vehreschild a écrit :
Hi all,
update based on latest 65548 (v5) patch and current trunk. Description and
issue addressed unchanged (see cite below).
Bootstrapped and regtested on x86_64-linux-gnu/f21.
Any volunteers to review? The initial version dates back to March 30. 2015.
Not
a single comment so far!
Let's start now. ;-)
I don't understand why one of your previous patches was factoring the
source expression evaluation to a temporary in gfc_trans_allocate, and
now with this patch you do the same thing in gfc_resolve_allocate, not
reusing the part in gfc_trans_allocate.
*** failure:
*** 7201,7212
--- 7212,7229
return false;
}
+
static void
resolve_allocate_deallocate (gfc_code *code, const char *fcn)
{
gfc_expr *stat, *errmsg, *pe, *qe;
gfc_alloc *a, *p, *q;
+ /* When this flag is set already, then this allocate has already been
+ resolved. Doing so again, would result in an endless loop. */
+ if (code-ext.alloc.arr_spec_from_expr3)
+ return;
+
I expect you'll miss some error messages by doing this.
Where is the endless loop?
*** resolve_allocate_deallocate (gfc_code *c
*** 7375,7382
--- 7392,7500
if (strcmp (fcn, ALLOCATE) == 0)
{
+ bool arr_alloc_wo_spec = false;
for (a = code-ext.alloc.list; a; a = a-next)
! resolve_allocate_expr (a-expr, code, arr_alloc_wo_spec);
!
! if (arr_alloc_wo_spec code-expr3)
! {
[...]
!
! ass = gfc_get_code (EXEC_ASSIGN);
This memory is not freed as far as I know.
I think you can use a local variable for it.
*** /tmp/PRaWHc_trans-expr.c2015-05-25 19:54:35.056309429 +0200
--- /tmp/7e82nd_trans-expr.c2015-05-25 19:54:35.058309429 +0200
*** gfc_conv_procedure_call (gfc_se * se, gf
*** 5328,5334
if (e (e-ts.type == BT_DERIVED || e-ts.type == BT_CLASS)
e-ts.u.derived-attr.alloc_comp
!(e-symtree e-symtree-n.sym-attr.pointer)
!(e-expr_type != EXPR_VARIABLE !e-rank))
{
int parm_rank;
tmp = build_fold_indirect_ref_loc (input_location,
--- 5328,5335
if (e (e-ts.type == BT_DERIVED || e-ts.type == BT_CLASS)
e-ts.u.derived-attr.alloc_comp
!(e-symtree e-symtree-n.sym-attr.pointer)
!e-expr_type != EXPR_VARIABLE !e-rank
!e-expr_type != EXPR_STRUCTURE)
{
int parm_rank;
tmp = build_fold_indirect_ref_loc (input_location,
Can't you remove this? It's undone by the PR58586 patch.
*** gfc_trans_allocate (gfc_code * code)
*** 5733,5746
if (dataref dataref-u.c.component-as)
{
! int dim;
gfc_expr *temp;
gfc_ref *ref = dataref-next;
ref-u.ar.type = AR_SECTION;
/* We have to set up the array reference to give ranges
in all dimensions and ensure that the end and stride
are set so that the copy can be scalarized. */
- dim = 0;
for (; dim dataref-u.c.component-as-rank; dim++)
{
ref-u.ar.dimen_type[dim] = DIMEN_RANGE;
--- 5758,5815
if (dataref dataref-u.c.component-as)
{
! int dim = 0;
gfc_expr *temp;
gfc_ref *ref = dataref-next;
ref-u.ar.type = AR_SECTION;
+ if (code-ext.alloc.arr_spec_from_expr3)
+ {
+ /* Take the array dimensions from the
+ source=-expression. */
+ gfc_array_ref *source_ref =
+ gfc_find_array_ref (code-expr3);
Does this work? code-expr3 is not always a variable.
+ if (source_ref-type == AR_FULL)
+ {
+ /* For full array refs copy the bounds. */
+ for (; dim dataref-u.c.component-as-rank; dim++)
+ {
+ ref-u.ar.dimen_type[dim] = DIMEN_RANGE;
+ ref-u.ar.start[dim] =
+ gfc_copy_expr (source_ref-as-lower[dim]);
+ ref-u.ar.end[dim] =
+ gfc_copy_expr (source_ref-as-upper[dim]);
+ }
This won't work. Consider this:
block
integer :: a(n)
n = n+1
allocate(b, source=a)
end block
You have to use a full array ref. In fact you can use a full array ref
everywhere, I think.
That's all for now.
Mikael
Ping: [Patch, fortran, PR44672, v6] [F08] ALLOCATE with SOURCE and no array-spec
Hi, the patch (65548) this one depends on is in trunk now. Still bootstraps ok and regtests with the issue in gfortran.dg/alloc_comp_constructor_1.f90 (which is addressed by the patch for pr58586 already) on x86_64-linux-gnu/f21. Ok for trunk? - Andre On Tue, 19 May 2015 12:26:02 +0200 Andre Vehreschild ve...@gmx.de wrote: Hi all, update based on latest 65548 (v5) patch and current trunk. Description and issue addressed unchanged (see cite below). Bootstrapped and regtested on x86_64-linux-gnu/f21. Any volunteers to review? The initial version dates back to March 30. 2015. Not a single comment so far! - Andre On Thu, 30 Apr 2015 16:17:42 +0200 Andre Vehreschild ve...@gmx.de wrote: Hi all, and also for this bug, I like to present an updated patch. It was brought to my attention, that the previous patch did not fix statements like: allocate(m, source=[(I, I=1, n)]) where n is a variable and type p class(*), allocatable :: m(:,:) end type real mat(2,3) type(P) :: o allocate(o%m, source=mat) The new version of the patch fixes those issue now also and furthermore addresses some issues (most probably not all) where the rank of the source=-variable and the rank of the array to allocate differ. For example, when one is do: real v(:) allocate(v, source= arr(1,2:3)) where arr has a rank of 2 and only the source=-expression a rank of one, which is then compatible with v. Nevertheless did this need addressing, when setting up the descriptor of the v and during data copy. Bootstrap ok on x86_64-linux-gnu/f21. Regtests with one regression in gfortran.dg/alloc_comp_constructor_1.f90, which is addressed in the patch for pr58586, whose final version is in preparation. Ok for trunk in combination with 58586 once both are reviewed? Regards, Andre On Wed, 29 Apr 2015 17:23:58 +0200 Andre Vehreschild ve...@gmx.de wrote: Hi all, this is the fourth version of the patch, adapting to the current state of trunk. This patch is based on my patch for 65584 version 2 and needs that patch applied beforehand to apply cleanly. The patch for 65548 is available from: https://gcc.gnu.org/ml/fortran/2015-04/msg00121.html Scope: Allow allocate of arrays w/o having to give an array-spec as specified in F2008:C633. An example is: integer, dimension(:) :: arr allocate(arr, source = [1,2,3]) Solution: While resolving an allocate, the objects to allocate are analyzed whether they carry an array-spec, if not the array-spec of the source=-expression is transferred. Unfortunately some source=-expressions are not easy to handle and have to be assigned to a temporary variable first. Only with the temporary variable the gfc_trans_allocate() is then able to compute the array descriptor correctly and allocate with correct array bounds. Side notes: This patch creates a regression in alloc_comp_constructor_1.f90 where two free()'s are gone missing. This will be fixed by the patch for pr58586 and therefore not repeated here. Bootstraps and regtests ok on x86_64-linux-gnu/f21. Ok for trunk? Regards, Andre -- Andre Vehreschild * Email: vehre ad gmx dot de
Re: [Patch, fortran, PR44672, v6] [F08] ALLOCATE with SOURCE and no array-spec
Hi all,
update based on latest 65548 (v5) patch and current trunk. Description and
issue addressed unchanged (see cite below).
Bootstrapped and regtested on x86_64-linux-gnu/f21.
Any volunteers to review? The initial version dates back to March 30. 2015. Not
a single comment so far!
- Andre
On Thu, 30 Apr 2015 16:17:42 +0200
Andre Vehreschild ve...@gmx.de wrote:
Hi all,
and also for this bug, I like to present an updated patch. It was brought to
my attention, that the previous patch did not fix statements like:
allocate(m, source=[(I, I=1, n)])
where n is a variable and
type p
class(*), allocatable :: m(:,:)
end type
real mat(2,3)
type(P) :: o
allocate(o%m, source=mat)
The new version of the patch fixes those issue now also and furthermore
addresses some issues (most probably not all) where the rank of the
source=-variable and the rank of the array to allocate differ. For example,
when one is do:
real v(:)
allocate(v, source= arr(1,2:3))
where arr has a rank of 2 and only the source=-expression a rank of one, which
is then compatible with v. Nevertheless did this need addressing, when setting
up the descriptor of the v and during data copy.
Bootstrap ok on x86_64-linux-gnu/f21.
Regtests with one regression in gfortran.dg/alloc_comp_constructor_1.f90,
which is addressed in the patch for pr58586, whose final version is in
preparation.
Ok for trunk in combination with 58586 once both are reviewed?
Regards,
Andre
On Wed, 29 Apr 2015 17:23:58 +0200
Andre Vehreschild ve...@gmx.de wrote:
Hi all,
this is the fourth version of the patch, adapting to the current state of
trunk. This patch is based on my patch for 65584 version 2 and needs that
patch applied beforehand to apply cleanly. The patch for 65548 is available
from:
https://gcc.gnu.org/ml/fortran/2015-04/msg00121.html
Scope:
Allow allocate of arrays w/o having to give an array-spec as specified in
F2008:C633. An example is:
integer, dimension(:) :: arr
allocate(arr, source = [1,2,3])
Solution:
While resolving an allocate, the objects to allocate are analyzed whether
they carry an array-spec, if not the array-spec of the source=-expression is
transferred. Unfortunately some source=-expressions are not easy to handle
and have to be assigned to a temporary variable first. Only with the
temporary variable the gfc_trans_allocate() is then able to compute the
array descriptor correctly and allocate with correct array bounds.
Side notes:
This patch creates a regression in alloc_comp_constructor_1.f90 where two
free()'s are gone missing. This will be fixed by the patch for pr58586 and
therefore not repeated here.
Bootstraps and regtests ok on x86_64-linux-gnu/f21.
Ok for trunk?
Regards,
Andre
--
Andre Vehreschild * Email: vehre ad gmx dot de
pr44672_6.clog
Description: Binary data
diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h
index aaa4e89..a7d862b 100644
--- a/gcc/fortran/gfortran.h
+++ b/gcc/fortran/gfortran.h
@@ -2396,6 +2396,9 @@ typedef struct gfc_code
{
gfc_typespec ts;
gfc_alloc *list;
+ /* Take the array specification from expr3 to allocate arrays
+ without an explicit array specification. */
+ unsigned arr_spec_from_expr3:1;
}
alloc;
diff --git a/gcc/fortran/resolve.c b/gcc/fortran/resolve.c
index fbf260f..6678138 100644
--- a/gcc/fortran/resolve.c
+++ b/gcc/fortran/resolve.c
@@ -6804,7 +6804,7 @@ conformable_arrays (gfc_expr *e1, gfc_expr *e2)
have a trailing array reference that gives the size of the array. */
static bool
-resolve_allocate_expr (gfc_expr *e, gfc_code *code)
+resolve_allocate_expr (gfc_expr *e, gfc_code *code, bool *array_alloc_wo_spec)
{
int i, pointer, allocatable, dimension, is_abstract;
int codimension;
@@ -7103,13 +7103,24 @@ resolve_allocate_expr (gfc_expr *e, gfc_code *code)
if (!ref2 || ref2-type != REF_ARRAY || ref2-u.ar.type == AR_FULL
|| (dimension ref2-u.ar.dimen == 0))
{
- gfc_error (Array specification required in ALLOCATE statement
- at %L, e-where);
- goto failure;
+ /* F08:C633. */
+ if (code-expr3)
+ {
+ if (!gfc_notify_std (GFC_STD_F2008, Array specification required
+ in ALLOCATE statement at %L, e-where))
+ goto failure;
+ *array_alloc_wo_spec = true;
+ }
+ else
+ {
+ gfc_error (Array specification required in ALLOCATE statement
+ at %L, e-where);
+ goto failure;
+ }
}
/* Make sure that the array section reference makes sense in the
-context of an ALLOCATE specification. */
+ context of an ALLOCATE specification. */
ar = ref2-u.ar;
@@ -7124,7 +7135,7 @@ resolve_allocate_expr (gfc_expr *e, gfc_code *code)
for (i = 0; i ar-dimen; i++)
{
- if (ref2-u.ar.type == AR_ELEMENT)
+ if (ar-type == AR_ELEMENT || ar-type == AR_FULL)
goto check_symbols;
