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

Match anonymous record's type inference with nominal records inference #759

Closed
5 tasks done
gusty opened this issue Jun 23, 2019 · 13 comments
Closed
5 tasks done

Match anonymous record's type inference with nominal records inference #759

gusty opened this issue Jun 23, 2019 · 13 comments

Comments

@gusty
Copy link

gusty commented Jun 23, 2019

I propose we use the same type of inference as for nominal records

The way it works now is that anonymous records inference occurs at a later stage.

Pros and Cons

The advantages of making this adjustment to F#: Currently because anonymous records are inferred at a later stage they are more prone to be type annotated than nominal records, which defeats the whole purpose of having anonymous records.

The disadvantages of making this adjustment to F#: Nominal records inference is considered a bit weird because when two records contain the same field names, the compiler takes the last one that was declared.

Extra information

Estimated cost: M

Related suggestions: dotnet/fsharp#6699

Affidavit (please submit!)

Please tick this by placing a cross in the box:

  • This is not a question (e.g. like one you might ask on stackoverflow) and I have searched stackoverflow for discussions of this issue
  • I have searched both open and closed suggestions on this site and believe this is not a duplicate
  • This is not something which has obviously "already been decided" in previous versions of F#. If you're questioning a fundamental design decision that has obviously already been taken (e.g. "Make F# untyped") then please don't submit it.

Please tick all that apply:

  • This is not a breaking change to the F# language design
  • I or my company would be willing to help implement and/or test this
@kspeakman
Copy link

kspeakman commented Nov 14, 2019

I think my comment applies here, but if I need to make a separate issue let me know.

We ran into this today. We were forced to twist our code into a particular shape in order to make anonymous records worth using.

type MyThing =
    { MyName: string
      MyStatus: MyStatus }

async {
    match! Db.read<{| Name: string; Status: string |}> config query with
    | Error ex -> ...
    | Ok list ->
        // does not compile, cannot determine type of item
        let toStatus item =
            { MyName = item.Name
              MyStatus = MyStatus.fromString item.Status }
        return Ok (List.map toStatus list)
}

Here's how I had to make it work:

async {
    match! Db.read<{| Name: string; Status: string |}> config query with
    | Error ex -> ...
    | Ok list ->
        let returnList =
            list
            |> List.map (fun item ->
                { MyName = item.Name
                  MyStatus = MyStatus.fromString item.Status }
            )
        return Ok returnList
}

This is not the actual code, but an example for demonstration.

This 2nd example is enough for type inference to determine the type of item. But I had to twist my code into a different shape (that I do not prefer) to reap any benefit from using anonymous records. I could have made the first code listing work if I had added a type annotation to the anonymous record. However, at that point it wouldn't be worth using it over a static record type. And this use case -- an interim, internal type -- seems like exactly the kind of reason for anonymous records to exist.

This would be solved if anonymous record type definitions were exposed to the type inference engine inside the scope where they are declared. So that the type inference would realize without annotation that the item fit the anonymous record type. Then I would have been able to use the first code example, which is cleaner to my eyes.

@gusty
Copy link
Author

gusty commented Nov 14, 2019

@cartermp Any update on this?
Does it make sense? Could it be approved for F# 5 ?
I know record inference is considered a bit weird, but it's been around since the beginning, let's don't bring completely different rules for anons.

@dsyme
Copy link
Collaborator

dsyme commented Nov 21, 2019

This is by-design based on the RFC, and I don't immediately expect we will change it.

@dsyme
Copy link
Collaborator

dsyme commented Nov 21, 2019

But I had to twist my code into a different shape (that I do not prefer) to reap any benefit from using anonymous records.

This is correct - the anonymity of anonymous records doesn't come without cost, nor do they give benefits in all situations. In general, type annotations are required to get decent name resolution and comprehensible errors for x.Name constructs. There are many cases where defining a record instead makes sense and may result in simpler and clearer code

@kspeakman
Copy link

kspeakman commented Nov 21, 2019

Our team has attempted to use anonymous records in place of transient records and tuples multiple times, but with no discernible benefit so far. It seems our code base just doesn't have or we lack instincts to recognize the beneficial use cases in our code. My comments and ergonomic suggestions above seem to expose that fact.

@cartermp
Copy link
Member

cartermp commented Nov 21, 2019

The

List.someFn (fun x -> ...) myType

vs.

myType |> List.someFn (fun x -> ...)

Isn't limited to anonymous records by any means here, though. This is a general F# tension point. The general guideline here is to use pipes to signal to the typechecker what something is.

@kspeakman
Copy link

kspeakman commented Nov 21, 2019

@cartermp Yes, that is a common issue with anonymous functions. But in this case, the first code listing would have worked by only replacing the anonymous record with a regular record. Because type inference would be aware of a regular record definition for the toStatus function, but not so for anonymous records.

@dsyme
Copy link
Collaborator

dsyme commented Nov 21, 2019

Because type inference would be aware of a regular record definition for the toStatus function, but not so for anonymous records.

You can also define an anonymous record type abbreviation and use a type annotation. It's essentially the same as defining a record type though

Our team has attempted to use anonymous records in place of transient records and tuples multiple times, but with no discernible benefit so far.

Yes, this is a feature people will try to use in various ways and it won't always give the results the user hopes for. I wrote this up somewhere, though it's not specifically in the RFC, but basically as soon as you need type annotations on code consuming the data then you may as well have defined a record type. This is a major reason why the RFC talks so much about "a smooth path to nominalization", so you don't get trapped in using anon records in sophisticated ways, and then pay an enormous cost to back out.

That is, you may have experimented with them and decided not to use them, but at least I assume you didn't get stuck in a corner where it was effectively impossible or very costly to back out of that experiment.

@kspeakman
Copy link

kspeakman commented Nov 21, 2019

This is a major reason why the RFC talks so much about "a smooth path to nominalization", so you don't get trapped in using anon records in sophisticated ways, and then pay an enormous cost to back out.

So it seems rather the purpose is to start out anonymous, such as in a REPL or data exploration. Then later solidify into named records as needed. That makes sense. And I suppose we assumed differently of them because we don't have those uses at present.

@gusty
Copy link
Author

gusty commented Nov 22, 2019

So it seems rather the purpose is to start out anonymous, such as in a REPL or data exploration. > Then later solidify into named records as needed.

Well, actually that's exactly my scenario. I'm trying to use them in ad-hoc queries in a REPL and they don't work, replacing them by nominal records solves the issue, but it doesn't makes any sense.

It's really sad, because I was always pushing to get anon records in F#, in order to get these kind of queries working, which defeats the purpose of using F# as opposed to C# in Linqpad.

If there's a solid decision of not matching the type inference mechanism of nominal records, is there a way to improve this type inference? Maybe using multipass inference, as it's being discussed in #594 ?

@dsyme
Copy link
Collaborator

dsyme commented Nov 22, 2019

I'm trying to use them in ad-hoc queries in a REPL and they don't work

Could you clarify - do you mean they don't work (unexpected), or that they require more type annotations than is palatable? I assume the latter.

which defeats the purpose of using F# as opposed to C# in Linqpad.

AFAIK inference for linqpad-like scenarios should be at least as good as for C# intra-method inference for anonymous C# objects, and likely better. Could you give examples if not?

thanks

@gusty
Copy link
Author

gusty commented Nov 22, 2019

Yes, I meant they require type annotations.
There are some SRTPs involved that fails, probably because they are being resolved before the type of the record, but with nominal records this is not a problem.
I will try to create a minimal repro of the problem.

@dsyme
Copy link
Collaborator

dsyme commented Apr 12, 2023

Closing per my comment here: #759 (comment)

This is all really by design

@dsyme dsyme closed this as completed Apr 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants