dotnet / fsharp

The F# compiler, F# core library, F# language service, and F# tooling integration for Visual Studio
https://dotnet.microsoft.com/languages/fsharp
MIT License
3.93k stars 787 forks source link

Anonymous records can't be used in fold #6699

Closed jbeeko closed 5 years ago

jbeeko commented 5 years ago

Please provide a succinct description of the issue.

Repro steps

Paste the following into a .fs file

//Unable to use anonymous records in List.fold
[1;2;3;4;5]
|> List.fold 
    (fun s i ->
        match i%2 with
        | 0 -> {|s with Evens = s.Evens + 1|}
        | _ -> {|s with Odds = s.Odds + 1|})
    {|Evens = 0; Odds = 0|}}

Note the indicated error on the s in the copy-update statements.

image

But the compiler seems to know that the state record passed into the fold is an anonymous reocord.

image

Expected behavior

It is possible to use the anonymous record and get the updated record back from the fold.

Actual behavior

Comiler error

Known workarounds

Create a type like so:


type Counter = {
    Evens:int
    Odds: int
}

[1;2;3;4;5]
|> List.fold 
    (fun s i ->
        match i%2 with
        | 0 -> {s with Evens = s.Evens + 1}
        | _ -> {s with Odds = s.Odds + 1})
    {Evens = 0; Odds = 0}

Related information

Provide any related information

cartermp commented 5 years ago

You can use a ||> pipeline like this:

({| Evens = 0; Odds = 0 |}, [1;2;3;4;5])
||> List.fold (fun s i ->
    match i%2 with
    | 0 -> {| s with Evens = s.Evens + 1 |}
    | _ -> {| s with Odds = s.Odds + 1 |})

I think this is expected/by design, as this is similar to other issues. Example:

// Error:
List.filter (fun s -> s.Length > 0) [""; "hello"]

//Works:
[""; "hello"] |> List.filter (fun s -> s.Length > 0)
jbeeko commented 5 years ago

Tks! for the tip and explaining that this is expected.

// Error:
List.filter (fun s -> s.Length > 0) [""; "hello"]

I my early days it was a bit off-putting because again s is identified as a sting by intelligence so I was like "WTH silly compiler" and sprinkled in type annotations. Stands out only because the compiler is normally so magical.

Anyway what is the protocol, do I close the case now or is that someone else's job.

KevinRansom commented 5 years ago

I think this is a bug:

[<EntryPoint>]
let main argv = 
    let x =
        [1;2;3;4;5] |> List.fold (fun s i -> match i % 2 with
                                             | 0 -> {|s with Evens = s.Evens + 1|}
                                             | _ -> {|s with Odds = s.Odds + 1|}) {|Evens = 0; Odds = 0|}

The type of s is correctly : image

That the detupling pipe operator causes it to work is the strongest indication that this is a bug.

dsyme commented 5 years ago

@KevinRansom This is just the usual issue where hover tips show the types reported after completing type inference, but some aspects of the type system work with respect to partial results.

@cartermp gives some other examples where this occurs above.

There's an old issue or suggestion about this somewhere

cartermp commented 5 years ago

Yes, because the tools can still infer things with partial data you get the tooltips but the code doesn't actually compile.

IMO what would be the appropriate fix is allowing the compiler to actually compile the code, as in the repro case and my own examples it's pretty clear that there is a type that matches what is going on in the lambda. I don't know if there's already an open issue here or in fslang-suggestions, though.

gusty commented 5 years ago

I came across a simpler example:

let db = 
    [
        {| name = "Gus"; id = 7|}
        {| name = "Jon"; id = 6|}
    ]

let flts = List.filter (fun x -> x.id = 7) db
                                 ˆˆˆˆ
~vsE3F3.fsx(145,34): error FS0072:
Lookup on object of indeterminate type based on information prior to this program point.
A type annotation may be needed prior to this program point to constrain the type of the object.
This may allow the lookup to be resolved.

But here intellisense also reports the error.

Obviously, using pipe forward solves the issue, but what's frustrating here, is that converting it to a nominal record type also solves the issue, so the question is why it works with nominal but not with anonymous? Maybe the compiler should look first for nominal records and then for anonymous.

If this is not enough related, I'm happy to file it as a separate bug.

matthid commented 5 years ago

@gusty

why it works with nominal but not with anonymous?

I wouldn't say it 'works' it's just that inference works in a single direction. The .Prop expression will "lock" into the last defined record containing the field, which is just a heuristic for very simple cases but not very useful either:

type Test1 = { id : int; name: string }
type Test2 = { id : int }

let db = 
    [
        { name = "Gus"; id = 7 }
        { name = "Jon"; id = 6 }
    ]

let flts = List.filter (fun x -> x.id = 7) db

image

We cannot do the same for anonymous records as that would be a breaking change.

gusty commented 5 years ago

We cannot do the same for anonymous records as that would be a breaking change.

How?

Anonymous records were not available before.

Even now in this new version with anonymous records, they are not accesible in these scenarios, unless they take priority over nominal ones, which is not what I propose as it would makes no sense. In this case it would rather be an "unbreaking change".

matthid commented 5 years ago

@gusty as I understand it you want x.Prop expression to lock on the previous record with Prop or previous anonymous record if none is found? I would argue that you should NEVER write code depending on this particular "feature" as of today, so we shouldn't extend it ;)

Ok imho we should either:

gusty commented 5 years ago

I agree in that it would be great to fix the entire inference scenario completely.

I also agree in that the current feature that works only with records is a bit weird, and the discussion wether this is a nice feature or not is a valid one.

But we shouldn't consider in these discussions only our day to day F# use at work, there many others scenarios, namely when writing ad-hoc queries, small scripts where these type inference restrictions bites a bit, not to forget that precisely these scenarios are also the ones in which anonymous types are a game changer.

matthid commented 5 years ago

and the discussion whether this is a nice feature or not is a valid one.

I don't really understand this point.

But we shouldn't consider in these discussions only our day to day F# use at work, there many others scenarios, namely when writing ad-hoc queries, small scripts where these type inference restrictions bites a bit

Yes, I completely agree. F# is heavily underused for scripting imho. That was one reason to start working on FAKE 5 ;). My point is actually the reverse: A lot of potentially new F# developers will get in touch with those small scripts. Therefore they shouldn't learn features which will become a lot less useful once you work in a larger code-base or introduce dependencies. This will irritate new users more than it helps users like us - as we already understand why some code works and some doesn't. Hence my opinion on not to use this 'feature' at all until inference is extended (which I doubt will happen anytime soon)

I'd suggest closing this issue or moving it to f# language suggestions. This should be discussed closely with

cartermp commented 5 years ago

Closing in favor of the language suggestions.

gusty commented 5 years ago

I added a suggestion for this: https://github.com/fsharp/fslang-suggestions/issues/759

leandromoh commented 3 years ago

You can use a ||> pipeline like this:

@cartermp thanks for the tip. I found me in the same weird situation. It solves the problem, however it really looks like a bug 😢