On 7/27/2025 8:51 PM, Mikael Morin wrote:
Le 27/07/2025 à 13:46, Yuao Ma a écrit :
On 7/27/2025 7:14 PM, Mikael Morin wrote:
Le 27/07/2025 à 11:37, Yuao Ma a écrit :
On 7/27/2025 5:19 PM, Mikael Morin wrote:
+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.
Yes, it's rejected at runtime. The trick lies in how the pos is
handled: it's first converted to gfc_charlen_type (an unsigned long
long). So, even if kind=1 and the input is -1, it wraps around to a
very large positive value (specifically, ULLONG_MAX - 1). This value
far exceeds 300, leading to a runtime error.
+ pos_for_call = fold_convert (gfc_charlen_type_node, pos);
Ok, according to the dump the conversion is to integer(kind=8) which
is signed. Anyway, I think it works for any sensible case.
So the case above is rejected, but now the value of pos printed is
not what the user specified.
This is what I get:
Fortran runtime error: If BACK is present with the value false, the
value of POS shall be in the range [0, LEN (STRING)], where POS =
18446744073709551615 and LEN (STRING) = 300
Good point. Since the conversion is always to integer(kind=8), we can
simply use %ld instead of %lu as the format argument.
Fixed.
Another thing I noticed:
+ gfc_add_expr_to_block (&block, call);
+ gfc_add_modify (&block, pos, fold_convert (TREE_TYPE (pos), call));
You are using the call twice; this results in two calls to the
function split in the dump:
_gfortran_string_split (300, &my_str, 1, &" "[1]{lb: 1 sz: 1},
(integer(kind=8)) my_pos, 0);
my_pos = (integer(kind=1)) _gfortran_string_split (300, &my_str,
1, &" "[1]{lb: 1 sz: 1}, (integer(kind=8)) my_pos, 0);
The gfc_add_expr_to_block call appears to be redundant.
Fixed.
I'm fine with this version.
I'll leave one more day for someone else to comment further and then
I'll push it for you.
Thanks for the patch.
Thanks for the detailed review! I'll keep working on other Fortran 2023
features as part of my GSoC project.
Best regards,
Yuao