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

Should FSharpList`1 be sealed? #123

Closed
KevinRansom opened this issue Jan 26, 2015 · 1 comment
Closed

Should FSharpList`1 be sealed? #123

KevinRansom opened this issue Jan 26, 2015 · 1 comment

Comments

@KevinRansom
Copy link
Member

originally opened at codeplex by marten_range

Inheriting a List<'T> in F# is forbidden:

type MyList() = 
    inherit List<int>()
// Compiler: error FS0945: Cannot inherit a sealed type

However creative C# developers can do this:

class MyList : FSharpList<int> 
{
    MyList () : base(1, FSharpList<int>.Empty)
    {
    }

    public IEnumerator<int> GetEnumerator ()
    {
        yield break;
    }
}

IMHO FSharpList should include the sealed flag to prevent misuse.

Adding sealed to FSharpList could, I guess, introduce a regression for developers that are overriding FSharpList but I suspect it's a quite rare regression.

Mårten

Comments
dsyme wrote Aug 6, 2014 at 7:25 AM [x]
I would be happy for the "Sealed" flag to be added to this type.

marten_range wrote Aug 7, 2014 at 12:59 PM [x]
Tried to add the [] attribute. Didn't work so I will need to dig more into how the assembly metadata is generated. Should be a good exercise.

marten_range wrote Aug 7, 2014 at 2:44 PM [x]

I think I found a place where sealed can be injected for union cases. Seems to work for types like:

type X = | Y |  Z

However FSharpList1 despise looking like a union to me is generated differently so FSharpList1 don't get tagged sealed.

FSharpLìst1 special cases (like TailOrNull and HeadOrDefault) has surprised me before so if anyone has any tips on how to control the code generation for FSharpList1 that would be very interesting to me.

Thanks.

@dsyme
Copy link
Contributor

dsyme commented Jan 27, 2015

Since this affects the compiled form of FSHarp.Core.dll, this is a feature request. Closing this in favour of the F# Language User Voice tracking entry http://fslang.uservoice.com/forums/245727-f-language/suggestions/7017131-make-f-union-types-like-list-that-have-no-subty

@dsyme dsyme closed this as completed Jan 27, 2015
dsyme added a commit that referenced this issue Feb 13, 2015
fixes #123
closes #143

commit 5566c99ebc936ad16081dda5f9a6c9754d4e095d
Author: latkin <[email protected]>
Date:   Fri Feb 13 12:55:07 2015 -0800

    Workaround for lack of Type.IsSealed in some portable profiles

commit 7bf077f
Author: Don Syme <[email protected]>
Date:   Fri Feb 13 10:53:42 2015 +0000

    fix codegen tests

commit cec8ada
Merge: 95cc10d ac85db7
Author: Don Syme <[email protected]>
Date:   Fri Feb 13 09:57:44 2015 +0000

    Merge branch 'fsharp4' of http://github.com/Microsoft/visualfsharp into fix-123

commit 95cc10d
Author: Don Syme <[email protected]>
Date:   Fri Jan 30 08:30:24 2015 +0000

    update baselines

commit 3050f4f
Author: Don Syme <[email protected]>
Date:   Thu Jan 29 16:27:55 2015 +0000

    fix test mistake

commit 5be64f6
Author: Don Syme <[email protected]>
Date:   Thu Jan 29 12:50:07 2015 +0000

    update baselines of tests

commit 99a552c
Merge: d91cb38 d17d429
Author: Don Syme <[email protected]>
Date:   Thu Jan 29 12:22:05 2015 +0000

    Merge branch 'fsharp4' of http://github.com/Microsoft/visualfsharp into fix-123

commit d91cb38
Author: Don Syme <[email protected]>
Date:   Thu Jan 29 10:18:54 2015 +0000

    NOP commit to run appveyor

commit 426f2a2
Author: Don Syme <[email protected]>
Date:   Tue Jan 27 16:43:43 2015 +0000

    add extra tests for IsSealed

commit 8dc04ab
Author: Don Syme <[email protected]>
Date:   Tue Jan 27 16:36:41 2015 +0000

    Fix #123 - Union types without sub-classes should be sealed
@latkin latkin added the fixed label Feb 13, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants