Fix floatcomp for %max

[JonoPrest]

A very quick one 🙏🏼

Closes #7332

[cristianoc]

Good catch!
Would you update the changelog too?

[zth]

And the test output (CI is red).

[JonoPrest]

Thanks guys!

@cristianoc are these supposed to be inverted?

rescript/compiler/ml/translcore.ml

Lines 212 to 233 in ba80b11

	(* FIXME: Core compatibility *)
	( "%bs_min",
	{
	objcomp = Pobjmax;
	intcomp = Pintmax;
	boolcomp = Pboolmax;
	floatcomp = Pfloatmax;
	stringcomp = Pstringmax;
	bigintcomp = Pbigintmax;
	simplify_constant_constructor = false;
	} );
	( "%bs_max",
	{
	objcomp = Pobjmin;
	intcomp = Pintmin;
	boolcomp = Pboolmin;
	floatcomp = Pfloatmin;
	stringcomp = Pstringmin;
	bigintcomp = Pbigintmin;
	simplify_constant_constructor = false;
	} );
	|]

ie %bs_min maps each comp to max and %bs_max maps to min

[cristianoc]

Thanks guys!

@cristianoc are these supposed to be inverted?

rescript/compiler/ml/translcore.ml

Lines 212 to 233 in 1d8560f

	(* FIXME: Core compatibility *)
	( "%bs_min",
	{
	objcomp = Pobjmax;
	intcomp = Pintmax;
	boolcomp = Pboolmax;
	floatcomp = Pboolmax;
	stringcomp = Pstringmax;
	bigintcomp = Pbigintmax;
	simplify_constant_constructor = false;
	} );
	( "%bs_max",
	{
	objcomp = Pobjmin;
	intcomp = Pintmin;
	boolcomp = Pboolmin;
	floatcomp = Pfloatmin;
	stringcomp = Pstringmin;
	bigintcomp = Pbigintmin;
	simplify_constant_constructor = false;
	} );
	|]

ie %bs_min maps each comp to max and %bs_max maps to min

Likely
To be sure, I would add a test

[JonoPrest]

Likely To be sure, I would add a test

@cristianoc I can see for example it's used here and bound to a "min" function.

Inverting them doesn't break any tests currently but it looks like the behaviour of inverting min with max is a mistake. And I think it's safe to assume so given how it's used.

This would go out of bounds if the 2 lists passed in had different lengths.

rescript/runtime/Stdlib_List.res

Lines 66 to 86 in 1d8560f

	module A = {
	@new external makeUninitializedUnsafe: int => array<'a> = "Array"
	external min: ('a, 'a) => 'a = "%bs_min"
	let reduceReverseU = (a, x, f) => {
	let r = ref(x)
	for i in Stdlib_Array.length(a) - 1 downto 0 {
	r.contents = f(r.contents, Stdlib_Array.getUnsafe(a, i))
	}
	r.contents
	}
	let reduceReverse2U = (a, b, x, f) => {
	let r = ref(x)
	let len = min(Stdlib_Array.length(a), Stdlib_Array.length(b))
	for i in len - 1 downto 0 {
	r.contents = f(r.contents, Stdlib_Array.getUnsafe(a, i), Stdlib_Array.getUnsafe(b, i))
	}
	r.contents
	}
	}

Would you like me to try and add a test for this reduceReverse2U?

[cometkim]

Oops..

[cristianoc]

Likely To be sure, I would add a test

@cristianoc I can see for example it's used here and bound to a "min" function.

Inverting them doesn't break any tests currently but it looks like the behaviour of inverting min with max is a mistake. And I think it's safe to assume so given how it's used.

This would go out of bounds if the 2 lists passed in had different lengths.

rescript/runtime/Stdlib_List.res

Lines 66 to 86 in 1d8560f

	module A = {
	@new external makeUninitializedUnsafe: int => array<'a> = "Array"
	external min: ('a, 'a) => 'a = "%bs_min"
	let reduceReverseU = (a, x, f) => {
	let r = ref(x)
	for i in Stdlib_Array.length(a) - 1 downto 0 {
	r.contents = f(r.contents, Stdlib_Array.getUnsafe(a, i))
	}
	r.contents
	}
	let reduceReverse2U = (a, b, x, f) => {
	let r = ref(x)
	let len = min(Stdlib_Array.length(a), Stdlib_Array.length(b))
	for i in len - 1 downto 0 {
	r.contents = f(r.contents, Stdlib_Array.getUnsafe(a, i), Stdlib_Array.getUnsafe(b, i))
	}
	r.contents
	}
	}

Would you like me to try and add a test for this reduceReverse2U?

Given there's already a test whose output changes, I think that' enough.
Good to go.