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

Give advisory messages when using ! and :=, and ask users to use .Value instead #569

Closed
4 of 5 tasks
dsyme opened this issue May 12, 2017 · 25 comments
Closed
4 of 5 tasks

Comments

@dsyme
Copy link
Collaborator

dsyme commented May 12, 2017

As of F# 4.0, let mutable x = ... supports automatic promotion to a reference cell if the x is captured in a closure. As a result, the explicit use of ref cells is now far less common for F# programming.

Because of this, I propose we gradually consider deprecating the default FSharp.Core definition of

let (!) (r: 'T ref)  = r.Value

and instead ask people to use r.Value instead

This has two advantages

  • for programmers coming from most other languages, ! already has a very specific meaning - usually boolean negation. For these people - and arguably others - r.Value is clearer

  • in the very, very long-term we could consider giving !x the usual meaning as an overloaded operator suitable for using with boolean values.

The disadvantages are somewhat obvious: it is changing something stable and which works.

Extra information

Estimated cost (XS, S, M, L, XL, XXL): S, but large downstream costs

Affadavit (must be submitted)

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 would be willing to help implement and/or test this
@dsyme dsyme changed the title Deprecate !, require the use of .Value Deprecate the default definition of (!) and ask users to use .Value instead or use a compat module May 12, 2017
@rmunn
Copy link

rmunn commented May 17, 2017

Very much in favor of deprecating the .Value meaning of the ! operator, but I'm not enthusiastic about the long-term idea of making ! mean not. My experience in languages like C# is that the ! character is easy to miss, especially when it's inside the parentheses that C# requires around if conditions. if (!foo) looks a lot like if (foo) if you're skimming code, but if (not foo) (or, since F# doesn't require parentheses, if not foo) is impossible to misread even when skimming.

If we do, sometime in the future, add ! as the Boolean not operator (once its current meaning has been deprecated and removed), I'd like to see a compiler warning, which is on by default, that says something like "Use of the ! operator is not recommended; use not instead", and let the users turn that warning off in their compiler options if they really want to use ! despite the warning. That's my personal code-style opinion, and others may differ -- but I like the aspect of F# that it tends to nudge you towards good design (e.g., making circular references hard). So I'd like to see another aspect of good design (using not instead of ! for negation) encouraged as well.

But in the short term, I'm certainly in favor of deprecating the r.Value meaning of !.

@0x53A
Copy link
Contributor

0x53A commented May 22, 2017

Slightly related, may I create a proposal to un-deprecate and / or instead of && / ||, or would that be shot down instantly anyway? =)

@dsyme
Copy link
Collaborator Author

dsyme commented May 23, 2017

@0x53A You could try :) But yes, I would close that

@dsyme
Copy link
Collaborator Author

dsyme commented May 23, 2017

@rmunn one problem with "not" is the extra parenthesization induced, e.g. not(f(x,y))

@dsyme
Copy link
Collaborator Author

dsyme commented Nov 16, 2017

I am marking this as approved-in-principle, though an RFC might not get through the discussion process, e.g. as we get all input on backwards-compat issues

@wallymathieu
Copy link

‘not’ is composable, while there are some operators found in other ml languages that could make it easier.

@wallymathieu
Copy link

wallymathieu commented Dec 24, 2017

Can't not(f(x,y)) be written as (not << f) (x,y) or by using <| you get not <| f (x,y) ?

@7sharp9
Copy link
Member

7sharp9 commented Jan 8, 2018

I think depreciating not is a good idea, Ive always found the syntax over baked. ! is pretty much fixed in programmers minds.

@cartermp
Copy link
Member

I'm in favor if there is a mechanism to convert syntax from !<expr> to <expr>.Value when someone upgrades to the latest F# (which I'd assume is a major version bump).

This can be done in VS by registering a code fix for a specific error message. Other editors would have to do a similar thing (cc @Krzysztof-Cieslak), thus the logic for rewriting the source would have to live in FCS.

@wallymathieu
Copy link

! is pretty much fixed in programmers minds.

I think it depends on the perspective and where you are coming from. The majority of programmers use c-like languages where ! is used.

In python not is used. In ruby you can use both not and !. In c-like languages like c#, java, JavaScript, c, c++ et.c. you have !. I've not seen many ml languages that use ! for not (I think OCaml and Haskell use not), except ReasonML that uses ! for not. In lisps, like clojure, scheme et.c., I think you use not.

Though I know of c# developers that prefer to use false == someCondition because they feel that ! is sometimes easily missed.

If you are coming from another background where you have used more math, then ! usually means factorial. But it was a small thing compared to what really tripped me up mostly when I started programming: You use int, long, float et.c. instead of how they are defined in math.

My personal preference would be to be able to use both not and ! for logical operator since then you can be a bit flexible in the way that you can use the language. I agree that using ! as content reference is kind of weird (like in OCaml).

@7sharp9
Copy link
Member

7sharp9 commented Mar 8, 2019

Pattern matching on value is quote nice to destructure ref cells anyway, I do wish that an Option had the capability to hold a reference going from None to Some much like a ref cell would hold a reference.

@cartermp
Copy link
Member

cartermp commented Mar 3, 2021

FYI we have a way to detect usage of ! when in conjunction with a diagnostic now. Today it can be used to offer a code fix:

image

In the future, it can be used to do the same but for a deprecation warning and add .Value instead.

@dsyme
Copy link
Collaborator Author

dsyme commented Mar 11, 2021

FYI we have a way to detect usage of ! when in conjunction with a diagnostic now. Today it can be used to offer a code fix:

Adding this suggested fix today would be great

@cartermp
Copy link
Member

Adding this suggested fix today would be great

We'd need the warning first, or an analyzer system that Roslyn can understand. Right now we're locked into diagnostics-based quick fixes. It could also be a CodeRefactoringProvider I guess too, that would be worth looking into.

@dsyme
Copy link
Collaborator Author

dsyme commented Mar 11, 2021

We'd need the warning first, or an analyzer system that Roslyn can understand. Right now we're locked into diagnostics-based quick fixes. It could also be a CodeRefactoringProvider I guess too, that would be worth looking into.

I guess we could look for all symbols resolving to the FSharp.Core operator ! based on symbol resolutions only

@cartermp
Copy link
Member

That isn't the issue. It's informing Roslyn's diagnostic-based system of quick fixes that there is a diagnostic we can listen to. You can't create custom document diagnostic analyzers right now, so there's no option to create this codefix today. The only option that may exist is a code refactoring provider.

@dsyme
Copy link
Collaborator Author

dsyme commented Mar 11, 2021

I see. Yes I hit this with the analyzer work and just added the analyzer diagnostics to the UnusedOpens diagnostic analyzer for now (which means unused opens won't show until all analyzers have run, and all analyzers must run before any of their diagnostics are shown).

@cartermp
Copy link
Member

Got the refactoring provider :)

refactor-deref

Will be in a VS 16.10 near you, provided there's nothing bad

@Happypig375
Copy link
Contributor

Should := be deprecated here as well?

@cartermp
Copy link
Member

Not unless it were to change the reference cell into a local and change all update sites to be <-. Note that this could not activate for a subset of use cases.

@Happypig375
Copy link
Contributor

@cartermp := is just short for .Value <-. I don't see how changing it to a local is necessary here.

@dsyme
Copy link
Collaborator Author

dsyme commented Jul 28, 2021

Should := be deprecated here as well?

Yes, we would recommend

expr.Value <- 1

@dsyme
Copy link
Collaborator Author

dsyme commented Jul 28, 2021

My specific proposal is to give informational messages from compiler and IDE when using !, :=, incr or decr from FSharp.Core.

Informational messages are different to warnings because --warnon doesn't fail the compilation with them (though you can turn on specific informational messages as errors using --warnon:NUMBER.

@dsyme dsyme changed the title Deprecate the default definition of (!) and ask users to use .Value instead or use a compat module Deprecate the default definition of (!) and ask users to use .Value instead Jul 28, 2021
@dsyme dsyme changed the title Deprecate the default definition of (!) and ask users to use .Value instead Give advisory messages when using ! and :=, and ask users to use .Value instead Jul 28, 2021
@dsyme
Copy link
Collaborator Author

dsyme commented Jul 28, 2021

@dsyme
Copy link
Collaborator Author

dsyme commented Oct 13, 2021

Completed in F# 6

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

7 participants