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

add isNotNull to FSharp.Core #99

Closed
baronfel opened this issue Oct 20, 2016 · 7 comments
Closed

add isNotNull to FSharp.Core #99

baronfel opened this issue Oct 20, 2016 · 7 comments
Labels

Comments

@baronfel
Copy link
Contributor

baronfel commented Oct 20, 2016

add isNotNull to FSharp.Core [13419354]

Submitted by Gauthier Segay on 4/13/2016 12:00:00 AM
2 votes on UserVoice prior to migration

Using "not (isNull a)" in conditions forces usage of parens or pipe operator and is not optimal readability compared to "a |> isNotNull" or "isNotNull a"
let inline isNotNull a = not (isNull a)
in absence of this function, people often take the shortcut of "a <> null" which according to lint is not optimal.

Response

** by fslang-admin on 6/13/2016 12:00:00 AM **

Declining per my comment below.
Don Syme
F# Language Evolution

Original UserVoice Submission
Archived Uservoice Comments

@abelbraaksma
Copy link
Member

When this was converted, the comments after the decline showed (by Daniel Robinson, 13 Sept 2016):

Did I miss something? It looks like this was added to FSharp.Core -
https://github.com/Microsoft/visualfsharp/blob/36050db52bc4d4876b16b7f62b5e67cdee34f2aa/src/fsharp/FSharp.Core/prim-types.fsi#L2157-L2158

So: this was declined because F# should not include negations of operators or functions and not make exceptions, but I think it has made it into the Core after all.

@dsyme
Copy link
Collaborator

dsyme commented Dec 8, 2016

@abelbraaksma Well spotted :)

@forki Ideally this should be removed from FSharp.Core , see your commit here: dotnet/fsharp@374bbc8, thanks

@dsyme dsyme reopened this Dec 8, 2016
@forki
Copy link
Member

forki commented Dec 8, 2016 via email

@dsyme
Copy link
Collaborator

dsyme commented Dec 8, 2016

Will do. But keep in mind that FCS AFAIK already "released" this.

FCS doesn't ship a new FSharp.Core. Mono is still shipping FSharp.Core 4.1.0.0 (though maybe it gets rebuilt with this in it)

@KevinRansom
Copy link

@dsyme why are we removing this API? it's certainly not the only new public Api .core.dll.

And on an entirely different note ... why does FCS not ship an FSharp.Core.dll? that seems a bit weird to me

@dsyme
Copy link
Collaborator

dsyme commented Dec 13, 2016

@dsyme why are we removing this API? it's certainly not the only new public Api .core.dll.

It was never approved during the design process (indeed was explicitly rejected, because we have a rule not to put "NotXYZ" predicates in the core library), it just crept in during some code cleanup.

And on an entirely different note ... why does FCS not ship an FSharp.Core.dll? that seems a bit weird to me

FCS is a DLL shipped as a nuget package with dependency on the FSharp.Core nuget package >= 4.4.0.0 (previously >= 4.3.1.0)

@dsyme
Copy link
Collaborator

dsyme commented Mar 1, 2017

Closing as declined - we added isNull but not the negative

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

No branches or pull requests

5 participants