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

Imply class-level and module-level `[<Extension>]` attribute #835

Open
cartermp opened this issue Feb 5, 2020 · 10 comments
Open

Imply class-level and module-level `[<Extension>]` attribute #835

cartermp opened this issue Feb 5, 2020 · 10 comments

Comments

@cartermp
Copy link
Member

@cartermp cartermp commented Feb 5, 2020

I propose we not require a top-level [<Extension>] attribute on a class when defining extension methods. Consider the following extension to IEnumerable<'T>:

open System.Collections.Generic
open System.Runtime.CompilerServices

[<Extension>]
type IEnumerableExtensions =
    [<Extension>]
    static member inline Sum(xs: IEnumerable<'T>) = Seq.sum xs

let f (x: IEnumerable<int>) = x.Sum()

Note that I must specify [<Extension>] on both the method and the containing class, otherwise it won't compile because Sum is not seen as an extension method.

It would be nice to just write the attribute for the method:

open System.Collections.Generic
open System.Runtime.CompilerServices

type IEnumerableExtensions =
    [<Extension>]
    static member inline Sum(xs: IEnumerable<'T>) = Seq.sum xs

let f (x: IEnumerable<int>) = x.Sum()

Additionally, it should emit the Extension attribute at the module level like C# does.

Pros and Cons

The advantages of making this adjustment to F# are:

  • Less stuff to remember
  • It's arguably more of a leaked implementation detail that the containing class must have this attribute
  • More similar to C#, which emits the attribute

The disadvantages of making this adjustment to F# are:

  • It's work

Extra information

Estimated cost (XS, S, M, L, XL, XXL): S

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
@7sharp9

This comment has been minimized.

Copy link
Member

@7sharp9 7sharp9 commented Feb 5, 2020

I thought you already had to do this to get visibility from C#?

@7sharp9

This comment has been minimized.

Copy link
Member

@7sharp9 7sharp9 commented Feb 5, 2020

Oh... misread, my mind remove the not keyword. Carry on...

@pblasucci

This comment has been minimized.

Copy link

@pblasucci pblasucci commented Feb 5, 2020

I’m all for this. But while we’re at it, could this also include emitting the assembly-level extension attribute as well? This is needed for compatibility with VB. Effectively, stick [<assembly: Extension>] somewhere... or else VB consumers lose out.

@matthid

This comment has been minimized.

Copy link

@matthid matthid commented Feb 5, 2020

@pblasucci Poor VB users, I think nobody does this in F#. Is this even mentioned in the F# extension docs?

@realvictorprm

This comment has been minimized.

Copy link
Member

@realvictorprm realvictorprm commented Feb 5, 2020

I'm all in for this too including what @pblasucci suggests. As F# users we know best how it feels to be left behind 😉

@cartermp

This comment has been minimized.

Copy link
Member Author

@cartermp cartermp commented Feb 5, 2020

@matthid

This comment has been minimized.

Copy link

@matthid matthid commented Feb 5, 2020

@cartermp Are you sure? I'm talking about the assembly level attribute. It is not part of any sample and not mentioned in the text (at least I don't see it anywhere)

In any case, this shows that the suggestion is reasonable ;)

@cartermp

This comment has been minimized.

Copy link
Member Author

@cartermp cartermp commented Feb 5, 2020

Oh, assembly level attributes. No, nobody uses those.

@smoothdeveloper

This comment has been minimized.

Copy link
Contributor

@smoothdeveloper smoothdeveloper commented Feb 6, 2020

@cartermp there are people defining F# extension methods to be called from VB.NET.

https://theburningmonk.com/2012/09/f-make-extension-methods-visible-to-c/

image

F# should do the same as C# in this regard and not treat VB.NET client code as alien, emitting the attribute on the assembly is necessary for this and should be part of the suggestion.

@cartermp cartermp changed the title Imply class-level `[<Extension>]` attribute Imply class-level and module-level `[<Extension>]` attribute Feb 9, 2020
@cartermp

This comment has been minimized.

Copy link
Member Author

@cartermp cartermp commented Feb 9, 2020

Adjusted

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants
You can’t perform that action at this time.