Hi Sandra,
On 20.10.21 22:03, Sandra Loosemore wrote:
The one that was most concerning was an ICE when calling the SHAPE
intrinsic with an assumed-rank class type argument ... In this case,
SHAPE was calling a library function and trying to copy the array
contents to a temporary, which is really stupid because SHAPE only
needs to look at the descriptor and not the array contents. I thought
we could handle this inline the same as UBOUND and LBOUND, by
extending gfc_trans_intrinsic_bound, and avoid the library function
entirely.
Then, I found some other existing problems in
gfc_trans_intrinsic_bound; the conditional it was building to test for
the extent-zero special cases for LBOUND and UBOUND was completely
wrong, and the compile-time test for the assumed-rank/assumed-size
case was wrong too. So I ended up rewriting large parts of that
function.
I also fixed a bug in the SIZE intrinsic where it was not taking the
class types into account. (SIZE is already being handled inline in a
separate place, otherwise I might've merged it into
gfc_trans_intrinsic_bound as well.)
Thanks for your efforts!
LGTM with the changelog path fix, without the gfc_tree_array_size
attribute change and with the indentation fix.
Namely:
gcc/testsuite/gfortran.dg/
PR fortran/94070
* c-interop/shape-bindc.f90: New test.
The ChangeLog file is in testsuite/ not in testsuite/gfortran.dg/.
@@ -8054,9 +8060,18 @@ gfc_tree_array_size (stmtblock_t *block, tree desc,
gfc_expr *expr, tree dim)
return GFC_TYPE_ARRAY_SIZE (TREE_TYPE (desc));
}
tree size, tmp, rank = NULL_TREE, cond = NULL_TREE;
- symbol_attribute attr = gfc_expr_attr (expr);
+ symbol_attribute attr;
gfc_array_spec *as = gfc_get_full_arrayspec_from_expr (expr);
gcc_assert (GFC_DESCRIPTOR_TYPE_P (TREE_TYPE (desc)));
+
+ if (expr->ts.type == BT_CLASS)
+ {
+ attr = CLASS_DATA (expr->symtree->n.sym)->attr;
+ attr.pointer = attr.class_pointer;
+ }
+ else
+ attr = gfc_expr_attr (expr);
Short version: Can you undo this change and verify that it still
fails? – Because I think that it works now due to the patch mentioned below.
At least it did pass here and an assert only pointed to testcases,
where I expected an issue.
* * *
Long version:
I stumbled over this while reading the patch as I think it is wrong in the
general case. However, I belief it is fine for the particular use
(only allocatable + pointer with assumed-rank arrays). It does mishandle
"nonclass%class_comp" – but that is inaccessible if 'nonclass' is
assumed-rank (and the attributes are only used in that case).
Nonetheless, I fail to see when this fails - because I think that gfc_expr_attr
should yield the proper result (since Tue Oct 12, my
eb92cd57a1ebe7cd7589bdbec34d9ae337752ead)
I think before before that patch, the problem was that expr was not 'var'
but 'var%_data' and gfc_expr then returned the attributes for gfc_expr,
which always had attr.pointer == 1 as the true 'pointer' attribute is in
attr.class_pointer and not in attr.pointer.
My bet is that you modified this before doing the "git pull" which pulled
in my patch above ...
For testing, I did turn your change into an assert and it only failed for
class_48.f90 and pr93792.f90. Those expose the issue I was concerned about:
(gdb) p gfc_debug_expr(expr)
test4:one % a % _data(FULL)
(gdb) p gfc_debug_expr(expr)
copy:self % x % _data(FULL)
(As said: nonissue in this case, but still feels wrong – and as the
workaround is longer be needed, it can also be removed.)
--- a/gcc/fortran/trans-intrinsic.c
+++ b/gcc/fortran/trans-intrinsic.c
...
+ /* Descriptors for assumed-size arrays have ubound = -1
+ in the last dimension. */
+ cond1 = fold_build2_loc (input_location, EQ_EXPR,
+ logical_type_node, ubound, minus_one);
Here, indentation goes wrong.
Thanks,
Tobias
-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634
München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas
Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht
München, HRB 106955
