Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implicit int to float conversion in function arguments fails when using arithmetic operations #17074

Open
SchlenkR opened this issue Apr 20, 2024 · 13 comments
Labels
Area-Compiler-Checking Type checking, attributes and all aspects of logic checking Bug Impact-Medium (Internal MS Team use only) Describes an issue with moderate impact on existing code.
Milestone

Comments

@SchlenkR
Copy link
Contributor

When a function

let a (f: float) = f

let r0 = a 1.0
let r1 = a 1
let r2 = a (10 / 2) // Error: he type 'float' does not match the type 'int'
let r3 =
    let x = 10 / 2
    a x
let r4 = 10 / 2

It appears that implicit conversion from integer to float is not working as expected when passing an integer that is an expression involving (somehow) arithmetic operations. This behavior is inconsistent other cases (like shown in the example).

I say: The expr at r2 should compile, too, like all other exprs.

See also here: https://twitter.com/SchlenkR/status/1781698130578211247

Note My experience is that this bug is a real pain for beginners, and it is extremely annoying when (e.g.) mapping values at API layers, etc.

Thanks

@github-actions github-actions bot added this to the Backlog milestone Apr 20, 2024
@SchlenkR
Copy link
Contributor Author

SchlenkR commented Apr 20, 2024

A potential workaround that might work in some situations is to use inline functions with op_Explicit constraint (like for example so):

// f: 'a (requires static member op_Explicit )
let inline a f = float f

let r0 = a 1.0
let r1 = a 1
let r2 = a (10 / 2) // No more error; it compiles
let r3 =
    let x = 10 / 2
    a x
let r4 = 10 / 2

@vzarytovskii
Copy link
Member

This is kind of expected, since by the time implicit conversion happens (AdjustRequiredTypeForTypeDirectedConversions), the actual type of the argument is not yet known/solved, thus conversion can't apply (since per RFC all types must be known):

image

Technically, it's solvable (by running some parts of conversion later, post inference), but by no means it's a small or easy fix.

It's not limited to operators, most of the complex expressions will fail.

@vzarytovskii
Copy link
Member

It's even more nuanced than I thought. The range of the error is weird because it actually belongs to op_Division, so what happens is:

  1. We restrict the type of a (both return and argument) to float.
  2. We then do the division (call op_Division), and infer its return type as float, since we restricted it earlier.
  3. We can't find op_Division: int * int -> float on any type, and produce a "weird" looking error with "weird" range.

These make it much harder to fix tbh.

@SchlenkR
Copy link
Contributor Author

SchlenkR commented Apr 22, 2024

I got some more test cases:

let a (f: float) = f
let inline div x y = x / y
let divInts x y = x / y

let r0 = a 1.0
let r1 = a 1
let r2 = a (10 / 2) // Error: the type 'float' does not match the type 'int'
let r3 =
    let x = 10 / 2
    a x
let r4 = 10 / 2

// > It's not limited to operators, most of the complex expressions will fail.
// These don't fail:
let r5 = a ((fun () -> 10 / 2) ())
let r6 = a (let x = 10 / 2 in x)
let r7 = a (int (1 * 10 / 2 * 1))

// Important: Error is shown in IDE, but
// in FSI, it compiles and evaluates to 5.0 (ok),
let r8 = a (divInts 10 99) // Error (IDE only !!): the type 'float' does not match the type 'int'

let r9 = a (div 10 2) // Error: the type 'float' does not match the type 'int'

@SchlenkR
Copy link
Contributor Author

We then do the division (call op_Division), and infer its return type as float, since we restricted it earlier.

Ok, I think I understand; it's overload resolution in combination of an operator (which also involves overload resolution). So inference in that case somehow requires a "best match" strategy respecting implicit conversion, etc.? Huh...

@vzarytovskii
Copy link
Member

vzarytovskii commented Apr 22, 2024

We then do the division (call op_Division), and infer its return type as float, since we restricted it earlier.

Ok, I think I understand; it's overload resolution in combination of an operator (which also involves overload resolution). So inference in that case somehow requires a "best match" strategy respecting implicit conversion, etc.? Huh...

It's multiple things, yeah. The main one in original issue is "we restricted the type first, then try to find corresponding op_Division based on this restriction". In case of the following, we already know it's int, since those are solved already:

let r5 = a ((fun () -> 10 / 2) ())
let r6 = a (let x = 10 / 2 in x)
let r7 = a (int (1 * 10 / 2 * 1))

r5 - we defined a function, and reduced it already, so 5.0 will be already in compilation (lifted to the property of the closure class), and it won't need a conversion (we already did it), this, for example, won't work for the same reason:

let r5 = a ((fun x -> 10 / x) 2)

r6 - well, pretty much the same reason as r5

r7 - again, same as two above.

I'm not entirely sure why don't we do anything in the r2 case, needs more investigation, I suppose we have some intrinsics which can reduce calling of the function/expression into a constant.

@vzarytovskii
Copy link
Member

vzarytovskii commented Apr 22, 2024

I'm pretty sure this is caused by the same "language feature" as #17062, which is specifics of "standard" operators resolution in F#, for example, this let r = a ((10 / 2) : int) will work, as it will tell compiler "don't infer type of op_Division, it's int".

@edgarfgp
Copy link
Contributor

edgarfgp commented Apr 23, 2024

@vzarytovskii Can this be consider a regression ?

Edit: This appeared during the AmplifyingF# Lectures https://amplifyingfsharp.io/fsharp-essentials/ too and it was confusing for the students.

@vzarytovskii
Copy link
Member

@vzarytovskii Can this be consider a regression ?

Not sure about it, it's been there for a while, and not recent (sharplab has it). Probably existed since conversions were introduced?

I'm also not entirely sure how fix would look like.

@edgarfgp
Copy link
Contributor

edgarfgp commented Apr 23, 2024

Reading point 1.1.3 Making Types Simple seems like this is not as the spec describes.

Although F# can typically infer types on your behalf, occasionally you must provide explicit type annotations in F# code. For example, the following code uses a type annotation for one of the parameters to tell the compiler the type of the input.

    > let concat (x : string) y = x + y;;
    val concat : string -> string -> string

Because x is stated to be of type `string`, and the only version of the `+` operator that accepts a left-hand argument of type `string` also takes a `string` as the right-hand argument, the F# compiler infers that the parameter `y` must also be a string. Thus, the result of `x + y` is the concatenation of the strings. Without the type annotation, the F# compiler would not have known which version of the `+` operator was intended and would have assumed `int` data by default.

@vzarytovskii
Copy link
Member

vzarytovskii commented Apr 23, 2024

Reading point 1.1.3 Making Types Simple seems like this is not as the spec describes.


Although F# can typically infer types on your behalf, occasionally you must provide explicit type annotations in F# code. For example, the following code uses a type annotation for one of the parameters to tell the compiler the type of the input.



    > let concat (x : string) y = x + y;;

    val concat : string -> string -> string



Because x is stated to be of type `string`, and the only version of the `+` operator that accepts a left-hand argument of type `string` also takes a `string` as the right-hand argument, the F# compiler infers that the parameter `y` must also be a string. Thus, the result of `x + y` is the concatenation of the strings. Without the type annotation, the F# compiler would not have known which version of the `+` operator was intended and would have assumed `int` data by default.

Implicit conversions are not part of the spec, they were introduced in F# 6 or 7, and obviously it doesn't work like that. It's a bug which is related to conversions and operators, and not a regression.

@edgarfgp
Copy link
Contributor

edgarfgp commented Apr 23, 2024

Agreed that it doesn’t include the implicit conversions. But the principle still stand that once you add a type annotation to function parameter this should be the type used in its body right ?

Implicit conversions are not part of the spec, they were introduced in F# 6 or 7, and obviously it doesn't work like that. It's a bug which is related to conversions and operators, and not a regression.

Right. I guess this is the RFC https://github.com/fsharp/fslang-design/blob/main/FSharp-6.0/FS-1093-additional-conversions.md

@vzarytovskii
Copy link
Member

But the principle still stand that once you add a type annotation to function parameter this should be the type used in its body right ?

Yes, and it uses it, it uses it to determine return type of op_Division - a float, but real type is int, and hence the error message and range pointing to last digit.

@abonie abonie added Impact-Medium (Internal MS Team use only) Describes an issue with moderate impact on existing code. Area-Compiler-Checking Type checking, attributes and all aspects of logic checking and removed Needs-Triage labels Apr 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compiler-Checking Type checking, attributes and all aspects of logic checking Bug Impact-Medium (Internal MS Team use only) Describes an issue with moderate impact on existing code.
Projects
Status: New
Development

No branches or pull requests

4 participants