Re: [Patch, fortran] PR40196 - [F03] [F08] Type parameter inquiry (str%len, a%kind) and Complex parts (z%re, z%im)
Hi Thomas, I tried failing cases of that kind; or assignment to len/kind part refs and returned correct errors. Must check where I was going wrong. Paul from a chilly Garching-bei-Muenchen On Sun, 28 Oct 2018, 13:38 Thomas Koenig Hi Paul, > > > >> inq would be easier to understand and unambiguous imho. > > > > Why? inquiry_type seems fine to me. > > I think Bernhard means the name of the member, i. > > I think it makes sense to leave as it is - gfc_ref is a > struct that occurs a lot in complicated expressions, and the other > members are one and two letters, too. > > > snip > >> Is the switch really worth it? I'd have used a plain chain of strcmp, > >> fwiw. > > > > I have done it. However, I might revert in order to combine the switch > > block where I set the typespec for the primary expression. > > Whatever suits you best. > > > I haven't added testcases for errors. Does anybody think that this is > necessary? > > Might not be a bad idea to run through at least each new error message > again. > > There is one illwfL test case which ICEs: > > $ cat b.f90 > program main >character(len=:), allocatable :: a >allocate(a,source="abc") >a%len = 2 >print *,a > end > $ gfortran b.f90 > gimplification failed: > (integer(kind=4)) .a type size > unit-size > align:32 warn_if_not_align:0 symtab:0 alias-set -1 > canonical-type 0x7f138acd15e8 precision:32 min 0x7f138acbcd68 -2147483648> max > pointer_to_this > > > arg:0 type size > unit-size > align:64 warn_if_not_align:0 symtab:0 alias-set -1 > canonical-type 0x7f138acd1738 precision:64 min 0x7f138acbcdf8 -9223372036854775808> max 9223372036854775807> > pointer_to_this > > used DI b.f90:1:0 size > unit-size > align:64 warn_if_not_align:0 context 0x7f138ae83200 MAIN__> > chain 0x7f138ae82540> > used unsigned DI b.f90:2:0 size 64> unit-size > align:64 warn_if_not_align:0 context 0x7f138ae83200 MAIN__ > b.f90:4:0: > > 4 | a%len = 2 >| > internal compiler error: gimplification failed > 0xb45602 gimplify_expr(tree_node**, gimple**, gimple**, bool > (*)(tree_node*), int) > ../../trunk/gcc/gimplify.c:12568 > > Regards > > Thomas >
Re: [Patch, fortran] PR40196 - [F03] [F08] Type parameter inquiry (str%len, a%kind) and Complex parts (z%re, z%im)
Hi Thomas, Thanks for finding the assignment a%len = 2 that escapes the check for lvalues. I am back home tomorrow night and will investigate why this one evades the trap. I think that an error test is needed in expr.c(gfc_check_assign). Cheers Paul On Sun, 28 Oct 2018 at 13:38, Thomas Koenig wrote: > > Hi Paul, > > > >> inq would be easier to understand and unambiguous imho. > > > > Why? inquiry_type seems fine to me. > > I think Bernhard means the name of the member, i. > > I think it makes sense to leave as it is - gfc_ref is a > struct that occurs a lot in complicated expressions, and the other > members are one and two letters, too. > > > snip > >> Is the switch really worth it? I'd have used a plain chain of strcmp, > >> fwiw. > > > > I have done it. However, I might revert in order to combine the switch > > block where I set the typespec for the primary expression. > > Whatever suits you best. > > > I haven't added testcases for errors. Does anybody think that this is > > necessary? > > Might not be a bad idea to run through at least each new error message > again. > > There is one illwfL test case which ICEs: > > $ cat b.f90 > program main >character(len=:), allocatable :: a >allocate(a,source="abc") >a%len = 2 >print *,a > end > $ gfortran b.f90 > gimplification failed: > (integer(kind=4)) .a type size > unit-size > align:32 warn_if_not_align:0 symtab:0 alias-set -1 > canonical-type 0x7f138acd15e8 precision:32 min 0x7f138acbcd68 -2147483648> max > pointer_to_this > > > arg:0 type size > unit-size > align:64 warn_if_not_align:0 symtab:0 alias-set -1 > canonical-type 0x7f138acd1738 precision:64 min 0x7f138acbcdf8 -9223372036854775808> max 9223372036854775807> > pointer_to_this > > used DI b.f90:1:0 size > unit-size > align:64 warn_if_not_align:0 context 0x7f138ae83200 MAIN__> > chain > used unsigned DI b.f90:2:0 size 64> unit-size > align:64 warn_if_not_align:0 context 0x7f138ae83200 MAIN__ > b.f90:4:0: > > 4 | a%len = 2 >| > internal compiler error: gimplification failed > 0xb45602 gimplify_expr(tree_node**, gimple**, gimple**, bool > (*)(tree_node*), int) > ../../trunk/gcc/gimplify.c:12568 > > Regards > > Thomas -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
Re: [Patch, fortran] PR40196 - [F03] [F08] Type parameter inquiry (str%len, a%kind) and Complex parts (z%re, z%im)
Hi Paul, inq would be easier to understand and unambiguous imho. Why? inquiry_type seems fine to me. I think Bernhard means the name of the member, i. I think it makes sense to leave as it is - gfc_ref is a struct that occurs a lot in complicated expressions, and the other members are one and two letters, too. snip Is the switch really worth it? I'd have used a plain chain of strcmp, fwiw. I have done it. However, I might revert in order to combine the switch block where I set the typespec for the primary expression. Whatever suits you best. I haven't added testcases for errors. Does anybody think that this is necessary? Might not be a bad idea to run through at least each new error message again. There is one illwfL test case which ICEs: $ cat b.f90 program main character(len=:), allocatable :: a allocate(a,source="abc") a%len = 2 print *,a end $ gfortran b.f90 gimplification failed: (integer(kind=4)) .a unit-size align:32 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type 0x7f138acd15e8 precision:32 min 0x7f138acbcd68 -2147483648> max pointer_to_this > arg:0 unit-size align:64 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type 0x7f138acd1738 precision:64 min 0x7f138acbcdf8 -9223372036854775808> max 9223372036854775807> pointer_to_this > used DI b.f90:1:0 size unit-size align:64 warn_if_not_align:0 context 0x7f138ae83200 MAIN__> chain used unsigned DI b.f90:2:0 size 64> unit-size align:64 warn_if_not_align:0 context 0x7f138ae83200 MAIN__ b.f90:4:0: 4 | a%len = 2 | internal compiler error: gimplification failed 0xb45602 gimplify_expr(tree_node**, gimple**, gimple**, bool (*)(tree_node*), int) ../../trunk/gcc/gimplify.c:12568 Regards Thomas
Re: [Patch, fortran] PR40196 - [F03] [F08] Type parameter inquiry (str%len, a%kind) and Complex parts (z%re, z%im)
Hi Bernhard,
Thanks for going through the patch:
snip
> missing space before open parenthesis
Corrected.
snip
> inq would be easier to understand and unambiguous imho.
Why? inquiry_type seems fine to me.
snip
> Is the switch really worth it? I'd have used a plain chain of strcmp,
> fwiw.
I have done it. However, I might revert in order to combine the switch
block where I set the typespec for the primary expression.
snip
> I guess RE and IM should be capitalised?
Done
> you could break here
>
> > +
> > for (ref = expr->ref; ref; ref = ref->next)
> > switch (ref->type)
> > {
Done
>
> > Index: gcc/fortran/trans-expr.c
snip...
> Don't you have to gfc_free_expr (se->expr) or gfc_replace_expr() ?
No these are tree expressions not gfc_expr. No cleanup is needed.
I haven't added testcases for errors. Does anybody think that this is necessary?
Cheers
Paul
Re: [Patch, fortran] PR40196 - [F03] [F08] Type parameter inquiry (str%len, a%kind) and Complex parts (z%re, z%im)
On Sat, 27 Oct 2018 20:03:47 +0100
Paul Richard Thomas wrote:
A few nits.
> + /* Pull an inquiry result out of an expression. */
> +
> + static bool
> + find_inquiry_ref (gfc_expr *p, gfc_expr **newp)
> + {
> + gfc_ref *ref;
> + gfc_ref *inquiry = NULL;
> + gfc_expr *tmp;
> +
> + tmp = gfc_copy_expr (p);
> +
> + if (tmp->ref && tmp->ref->type == REF_INQUIRY)
> + {
> + inquiry = tmp->ref;
> + tmp->ref = NULL;
> + }
> + else
> + {
> + for (ref = tmp->ref; ref; ref = ref->next)
> + if (ref->next && ref->next->type == REF_INQUIRY)
> + {
> + inquiry = ref->next;
> + ref->next = NULL;
> + }
> + }
> +
> + if(!inquiry)
missing space before open parenthesis
> *** typedef struct gfc_ref
> *** 1960,1965
> --- 1963,1970
> }
> ss;
>
> + inquiry_type i;
inq would be easier to understand and unambiguous imho.
> + /* Used by gfc_match_varspec() to match an inquiry reference. */
> +
> + static bool
> + is_inquiry_ref (const char *name, gfc_ref **ref)
> + {
> + inquiry_type type;
> +
> + if (name == NULL)
> + return false;
> +
> + if (ref) *ref = NULL;
> +
> + switch (name[0])
> + {
> + case 'r':
> + if (strcmp (name, "re") == 0)
> + type = INQUIRY_RE;
> + else
> + return false;
> + break;
> +
> + case 'i':
> + if (strcmp (name, "im") == 0)
> + type = INQUIRY_IM;
> + else
> + return false;
> + break;
> +
> + case 'k':
> + if (strcmp (name, "kind") == 0)
> + type = INQUIRY_KIND;
> + else
> + return false;
> + break;
> +
> + case 'l':
> + if (strcmp (name, "len") == 0)
> + type = INQUIRY_LEN;
> + else
> + return false;
> + break;
> +
> + default:
> + return false;
> + }
Is the switch really worth it? I'd have used a plain chain of strcmp,
fwiw.
> ! switch (tmp->u.i)
> ! {
> ! case INQUIRY_RE:
> ! case INQUIRY_IM:
> ! if (!gfc_notify_std (GFC_STD_F2008, "re or im
> part_refs at %C")) ! return MATCH_ERROR;
I guess RE and IM should be capitalised?
> *** gfc_variable_attr (gfc_expr *expr, gfc_t
> *** 2358,2363
> --- 2521,2527
> gfc_ref *ref;
> gfc_symbol *sym;
> gfc_component *comp;
> + bool has_inquiry_part;
>
> if (expr->expr_type != EXPR_VARIABLE && expr->expr_type !=
> EXPR_FUNCTION) gfc_internal_error ("gfc_variable_attr(): Expression
> isn't a variable"); *** gfc_variable_attr (gfc_expr
> *expr, gfc_t *** 2387,2392
> --- 2551,2561
> if (ts != NULL && expr->ts.type == BT_UNKNOWN)
> *ts = sym->ts;
>
> + has_inquiry_part = false;
> + for (ref = expr->ref; ref; ref = ref->next)
> + if (ref->type == REF_INQUIRY)
> + has_inquiry_part = true;
you could break here
> +
> for (ref = expr->ref; ref; ref = ref->next)
> switch (ref->type)
> {
> Index: gcc/fortran/trans-expr.c
> ===
> *** gcc/fortran/trans-expr.c (revision 265411)
> --- gcc/fortran/trans-expr.c (working copy)
> *** conv_parent_component_references (gfc_se
> *** 2510,2515
> --- 2510,2549
> conv_parent_component_references (se, );
> }
>
> +
> + static void
> + conv_inquiry (gfc_se * se, gfc_ref * ref, gfc_expr *expr,
> gfc_typespec *ts)
> + {
> + tree res = se->expr;
> +
> + switch (ref->u.i)
> + {
> + case INQUIRY_RE:
> + res = fold_build1_loc (input_location, REALPART_EXPR,
> + TREE_TYPE (TREE_TYPE (res)), res);
> + break;
> +
> + case INQUIRY_IM:
> + res = fold_build1_loc (input_location, IMAGPART_EXPR,
> + TREE_TYPE (TREE_TYPE (res)), res);
> + break;
> +
> + case INQUIRY_KIND:
> + res = build_int_cst (gfc_typenode_for_spec (>ts),
> +ts->kind);
> + break;
> +
> + case INQUIRY_LEN:
> + res = fold_convert (gfc_typenode_for_spec (>ts),
> + se->string_length);
> + break;
> +
> + default:
> + gcc_unreachable ();
> + }
> + se->expr = res;
Don't you have to gfc_free_expr (se->expr) or gfc_replace_expr() ?
cheers,
[Patch, fortran] PR40196 - [F03] [F08] Type parameter inquiry (str%len, a%kind) and Complex parts (z%re, z%im)
I was triggered to do this by one of the comments in response to Anton
Shterenlikht's standards survey. The comment was sufficiently
inconsiderate that my first thought was not to respond. However,
curiosity got the better of me... so said the dead cat!
There is a lot of this patch but it is (more or less) straight
forward. The tricky parts were to get the logic right in
gfc_match_varspec and in expr.c. One more step on the way to real
F2002 and F2008 compliance!
Bootstraps and regtests on FC28/x86_64 - OK for trunk?
Paul
2018-10-27 Paul Thomas
PR fortran/40196
* dependency.c (are_identical_variables): Return false if the
inquiry refs are not the same.
(gfc_ref_needs_temporary_p): Break on an inquiry ref.
* dump_parse_tree.c (show_ref): Show the inquiry ref type.
* expr.c (gfc_free_ref_list): Break on an inquiry ref.
(gfc_copy_ref): Copy the inquiry ref types.
(find_inquiry_ref): New function.
(simplify_const_ref, simplify_ref_chain): Call it. Add new arg
to simplify_ref_chain.
(gfc_simplify_expr): Use the new arg in call to
simplify_ref_chain.
(gfc_get_full_arrayspec_from_expr, gfc_is_coarray): Break on
inquiry ref.
(gfc_traverse_expr): Return true for inquiry ref.
* frontend-passes.c (gfc_expr_walker): Break on inquiry ref.
* gfortran.h : Add enums and union member in gfc_ref to
implement inquiry refs.
* intrinsic.c : Fix white nois.
* match.c (gfc_match_assignment): A constant lavlue is an
error.
* module.c : Add DECL_MIO_NAME for inquiry_type and the mstring
for inquiry_types.
(mio_ref): Handle inquiry refs.
* primary.c (is_inquiry_ref): New function.
(gfc_match_varspec): Handle inquiry refs calling new function.
(gfc_variable_attr): Detect inquiry ref for disambiguation
with components.
(caf_variable_attr): Treat inquiry and substring refs in the
same way.
* resolve.c (find_array_spec): ditto.
(gfc_resolve_substring_charlen): If there is neither a charlen
ref not an inquiry ref, return.
(resolve_ref): Handle inqiry refs as appropriate.
(resolve_allocate_expr): ENtities with an inquiry ref cannot be
allocated.
* simplify.c (simplify_bound, simplify_cobound): Punt on
inquiry refs.
* trans-array.c (get_array_ctor_var_strlen): Break on inquiry
ref.
*trans-expr.c (conv_inquiry): New function.
(gfc_conv_variable): Retain the last typespec to pass to
conv_inquiry on detecting an inquiry ref.
2018-10-27 Paul Thomas
PR fortran/40196
* gfortran.dg/inquiry_part_ref_1.f08: New test.
Index: gcc/fortran/dependency.c
===
*** gcc/fortran/dependency.c (revision 265411)
--- gcc/fortran/dependency.c (working copy)
*** are_identical_variables (gfc_expr *e1, g
*** 189,194
--- 189,199
break;
+ case REF_INQUIRY:
+ if (r1->u.i != r2->u.i)
+ return false;
+ break;
+
default:
gfc_internal_error ("are_identical_variables: Bad type");
}
*** gfc_ref_needs_temporary_p (gfc_ref *ref)
*** 905,910
--- 910,916
return subarray_p;
case REF_COMPONENT:
+ case REF_INQUIRY:
break;
}
Index: gcc/fortran/dump-parse-tree.c
===
*** gcc/fortran/dump-parse-tree.c (revision 265411)
--- gcc/fortran/dump-parse-tree.c (working copy)
*** show_ref (gfc_ref *p)
*** 308,313
--- 308,330
fputc (')', dumpfile);
break;
+ case REF_INQUIRY:
+ switch (p->u.i)
+ {
+ case INQUIRY_KIND:
+ fprintf (dumpfile, " INQUIRY_KIND ");
+ break;
+ case INQUIRY_LEN:
+ fprintf (dumpfile, " INQUIRY_LEN ");
+ break;
+ case INQUIRY_RE:
+ fprintf (dumpfile, " INQUIRY_RE ");
+ break;
+ case INQUIRY_IM:
+ fprintf (dumpfile, " INQUIRY_IM ");
+ }
+ break;
+
default:
gfc_internal_error ("show_ref(): Bad component code");
}
*** write_decl (gfc_typespec *ts, gfc_array_
*** 3167,3173
fputs (sym_name, dumpfile);
fputs (post, dumpfile);
!
if (rok == T_WARN)
fprintf (dumpfile," /* WARNING: Converting '%s' to interoperable type */",
gfc_typename (ts));
--- 3184,3190
fputs (sym_name, dumpfile);
fputs (post, dumpfile);
!
if (rok == T_WARN)
fprintf (dumpfile," /* WARNING: Converting '%s' to interoperable type */",
gfc_typename (ts));
Index: gcc/fortran/expr.c
===
*** gcc/fortran/expr.c (revision 265411)
--- gcc/fortran/expr.c (working copy)
*** gfc_free_ref_list (gfc_ref *p)
*** 599,604
--- 599,605
break;
case REF_COMPONENT:
+ case REF_INQUIRY:
break;
}
*** gfc_copy_ref (gfc_ref *src)
*** 756,761
--- 757,766
dest->u.c = src->u.c;
