Le 27/07/2025 à 05:40, Yuao Ma a écrit :
Hi Mikael,
On 7/27/2025 3:57 AM, Mikael Morin wrote:
Hello,
here are a few more comments on the patch below.
It's not ready yet, but the remarks should be easily addressed.
Thanks for the thorough review! I've addressed the comments and attached
a new patch below.
diff --git a/gcc/fortran/trans-intrinsic.cc b/gcc/fortran/trans-
intrinsic.cc
index be984271d6a..c2a91c93d28 100644
--- a/gcc/fortran/trans-intrinsic.cc
+++ b/gcc/fortran/trans-intrinsic.cc
@@ -3466,6 +3466,73 @@ else
return gfc_finish_block (&block);
}
+static tree
+conv_intrinsic_split (gfc_code *code)
+{
+ stmtblock_t block;
+ gfc_se se;
+ tree string, string_len;
+ tree set, set_len;
+ tree pos, pos_for_call;
+ tree back;
+ tree fndecl, call;
+ gfc_expr *string_expr, *set_expr, *pos_expr, *back_expr;
+
+ string_expr = code->ext.actual->expr;
+ set_expr = code->ext.actual->next->expr;
+ pos_expr = code->ext.actual->next->next->expr;
+ back_expr = code->ext.actual->next->next->next->expr;
+
+ gfc_start_block (&block);
+
+ gfc_init_se (&se, NULL);
+ gfc_conv_expr (&se, string_expr);
+ gfc_conv_string_parameter (&se);
+ string_len = se.string_length;
+ gfc_add_block_to_block (&block, &se.pre);
+ gfc_add_block_to_block (&block, &se.post);
+ string = se.expr;
+
Be careful with this.
se.expr is only guaranteed to be valid after se.pre and before se.post.
Did you check that it works for example if the string actual argument
is an allocatable function?
Maybe we should split the pre/post handling into block/post_block? I
borrowed the style from conv_co_collective in the new patch to hopefully
resolve any potential issues.
Yes, I think it works.
+ gfc_init_se (&se, NULL);
+ gfc_conv_expr (&se, set_expr);
+ gfc_conv_string_parameter (&se);
+ set_len = se.string_length;
+ gfc_add_block_to_block (&block, &se.pre);
+ gfc_add_block_to_block (&block, &se.post);
+ set = se.expr;
+
Same here.
+ gfc_init_se (&se, NULL);
+ gfc_conv_expr (&se, pos_expr);
+ gfc_add_block_to_block (&block, &se.pre);
+ gfc_add_block_to_block (&block, &se.post);
+ pos = se.expr;
+ pos_for_call = fold_convert (gfc_charlen_type_node, pos);
+
... and here.
For simple variables like pos, we can store the value to a variable,
and then the se.post code can be executed without impacting the value.
Now that pos.post has been added to post_block, is it still necessary to
create the variable using gfc_create_var and related functions in this
new version?
No, using the variable permits the execution of the cleanup code
earlier, but there is nothing wrong with executing it after the call.
diff --git a/libgfortran/intrinsics/string_intrinsics_inc.c b/
libgfortran/intrinsics/string_intrinsics_inc.c
index d86bb6c8833..64b3d878a74 100644
--- a/libgfortran/intrinsics/string_intrinsics_inc.c
+++ b/libgfortran/intrinsics/string_intrinsics_inc.c
@@ -459,3 +464,50 @@ string_minmax (gfc_charlen_type *rlen, CHARTYPE
**dest, int op, int nargs, ...)
*dest = tmp;
}
}
+
+gfc_charlen_type
+string_split (gfc_charlen_type stringlen, const CHARTYPE *string,
+ gfc_charlen_type setlen, const CHARTYPE *set,
+ gfc_charlen_type pos, GFC_LOGICAL_4 back)
+{
+ gfc_charlen_type i, j;
+
+ if (!back)
+ {
+ if (pos > stringlen)
+ runtime_error ("If BACK is present with the value false, the
value of "
+ "POS shall be in the range [0, LEN (STRING)]");
+
The condition doesn't check pos >= 0; I think the case pos < 0 doesn't
work.
The variable pos is an unsigned type, so checking for < 0 is
unnecessary. (I was not initially aware of this, but the compiler
warning alerted me.)
Mmmh, good point. But POS is a user variable, so a signed type originally.
Can you check the following:
integer(kind=1) :: my_pos = -1
character(len=300) :: my_str = ""
call split(my_str, " ", my_pos)
Is it rejected at runtime?
I expect the unsigned conversion to convert -1 to 255 and then the call
would be (wrongly) accepted.
