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

Zero method required in CEs, even when not used #8587

Closed
gusty opened this issue Feb 18, 2020 · 4 comments
Closed

Zero method required in CEs, even when not used #8587

gusty opened this issue Feb 18, 2020 · 4 comments
Labels
Bug Impact-Low (Internal MS Team use only) Describes an issue with limited impact on existing code.
Milestone

Comments

@gusty
Copy link
Contributor

gusty commented Feb 18, 2020

I was playing a bit with the new feature that allows implicit yields and realized that it's a good thing to write custom collections, like semigroups as the CE can't be empty.

But soon realized that it requires a Zero method, even when it is not used at all.

Repro steps

type NonEmpty<'t> = list<'t>  // for simplicity

type NelBuilder () =
    member __.Zero () = invalidOp "A NonEmptyList doesn't support the Zero operation."
    member __.Combine (a: NonEmpty<'T>, b) = a @ b
    member __.Yield  x = List.singleton x
    member __.Delay expr = expr () :  NonEmpty<'T>
let nel = NelBuilder ()

let x = nel {1; 2; 3}

// val x : NonEmpty<int> = [1; 2; 3]


type NelBuilder2 () =
    // member __.Zero () = invalidOp "A NonEmptyList doesn't support the Zero operation."
    member __.Combine (a: NonEmpty<'T>, b) = a @ b
    member __.Yield  x = List.singleton x
    member __.Delay expr = expr () :  NonEmpty<'T>
let nel2 = NelBuilder2 ()

let y = nel2 {1; 2; 3}

// ~vsA01F.fsx(22,21): error FS0708: This control construct may only be used if the computation expression builder defines a 'Zero' method

type NelBuilder3 () =
    // member __.Zero () = invalidOp "A NonEmptyList doesn't support the Zero operation."
    member __.Combine (a: NonEmpty<'T>, b) = a @ b
    member __.Yield  x = List.singleton x
    member __.Delay expr = expr () :  NonEmpty<'T>
let nel3 = NelBuilder3 ()

let y = nel3 {1; 2; 3; ()}

// ~vsA01F.fsx(33,24): error FS0708: This control construct may only be used if the computation expression builder defines a 'Zero' method

Expected behavior

Sample with nel compiles although Zero is not called.
Sample with nel2 compiles as Zero is not called.
Sample with nel3 doesn't compile as Zero is called.

Actual behavior

Sample with nel compiles although Zero is not called.
Sample with nel2 doesn't compile even though Zero is never called.
Sample with nel3 doesn't compile as Zero is called.

Known workarounds

Define a Zero method that throws a runtime exception but it defeats a bit the purpose of a compile-time non-empty-collection, as sample 1 would start compiling but failing at runtime.

Related information

Original comment: fsharp/fslang-suggestions#643 (comment)

@cartermp
Copy link
Contributor

@dsyme thoughts on this one?

@dsyme dsyme added the Bug label Sep 1, 2020
@cartermp cartermp added this to the Backlog milestone Sep 1, 2020
@dsyme
Copy link
Contributor

dsyme commented Sep 3, 2020

Yes, I agree this should not be required. I checked the implementation and I couldn't immediately spot the problem.

@dsyme dsyme added the Impact-Low (Internal MS Team use only) Describes an issue with limited impact on existing code. label Sep 3, 2020
@dsyme
Copy link
Contributor

dsyme commented Jan 12, 2021

My understanding is this is fully fixed by #10556

@gusty can you double check please?

@dsyme dsyme closed this as completed Jan 12, 2021
@cartermp cartermp modified the milestones: Backlog, 16.9 Jan 12, 2021
@gusty
Copy link
Contributor Author

gusty commented Jan 12, 2021

@dsyme I confirm it works on master now.

@laenas Thanks for fixing this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Impact-Low (Internal MS Team use only) Describes an issue with limited impact on existing code.
Projects
None yet
Development

No branches or pull requests

3 participants