Consider '_' for members with access modifiers

[gusty]

This will allow the use of the '_' for members with access modifiers like:

 type X() = member private _.PrintSomething () = printfn "something"

[gusty]

I would need to add a test for this (it works on my machine) but I see that the tests I did (actually I modified existing ones) in the original PR are not here anymore, I couldn't find what happened, wanted to see if they were reverted but they simply disappeared from the history.
Can anyone advise?

[brettfo]

I looked through the history and I have no idea what happened to the original single underscore PR. It was merged into release/fsharp47, release/fsharp47 was merged into master, but it's not here.

To unblock you I've cherry-picked that PR into master again in #7634.

[auduchinok]

Could we maybe avoid merging such features/fixes to branches other than master? We can now disable features by language version switches.
It's very frustrating to see some things are in master, others are somewhere like in dev* or even in language version specific one.

[brettfo]

Could we maybe avoid merging such features/fixes to branches other than master?

Now that the langversion switches are generally available, yes, that's the plan. If it helps we generally try to add stuff only to master, except when we absolutely can't and now that we can enforce preview features, we'll do that.

Part of the reason we couldn't do this in master to begin with is that we also try to adhere to the rule that master should always be buildable and usable against the current RTM version of Visual Studio, but we didn't want to get into a scenario where a user had installed the latest nightly VSIX, used new syntax in their code, and added <LangVersion> (or whatever) to their .fsproj, and then the 'real' RTM of Visual Studio wouldn't honor that value because it wasn't passed on to fsc, and then fsc would fail for some obscure reason instead of failing with a more helpful " requries language version X".

[brettfo]

Quick update: Look at this comment for more context, but in short, master has the single-underscore feature and tests for this PR should instead be made in the tests/fsharp/core/members/self-identifier/version47/test.fs file.

[KevinRansom]

Thanks for this PR, I like it, however, we missed the boat on including in FSharp4.7. This needs to target a new language version. FSharp5.0

Because FSharp 4.7 has already shipped, the feature will need to be isolated so that compiling --langversion:FSharp4.7 will still fail.

Testing should be updated so that compiling FSharp4.7 produces an error message consistent with what appeared before.
E.g:

1>C:\Users\kevinr\source\repos\ConsoleApp62\ConsoleApp62\Program.fs(13,20): error FS0010: Unexpected symbol '_' in member definition. Expected identifier, '(', '(*)' or other token.
1>C:\Users\kevinr\source\repos\ConsoleApp62\ConsoleApp62\Program.fs(19,27): error FS0010: Unexpected symbol '_' in member definition. Expected identifier, '(', '(*)' or other token.
```

Thanks for this PR, I like it.

[cartermp]

@KevinRansom I disagree with this going into F# 5, it's clearly a cleanup of an existing feature which doesn't really warrant F# 5

[gusty]

@KevinRansom I think the new language switch is a nice feature, but we don't have to abuse of it and treat every single change/addition as a separate feature.

At some point we have to look for a compromise. In this case I think it's easy, as already described by @cartermp . Also note that this is not a breaking change.

[KevinRansom]

@gusty, it's not an abuse.

The purpose of the switch is to allow a developer working in an environment with mixed language versions to pick a maximum language version to compile with. If we add additional language features following the release of a specific language version then the initial versions of that compiler version won't work to compile the code.

From a grab the latest version of the language tools, we find that people fall into two groups.

1. Those who grab the initial release and stick with it through the release cycle.
2. Those who update with every dot release.

Someone in group 1 would not be able to build code from someone in group2 that relied on this new language feature.

Please note: this is a new language feature not a bug.

[gusty]

I see what you mean, but that's taking it to the extreme and not considering updates.

Now, it's true that from the pure technical viewpoint this is an additional feature, but from the conceptual view this is a bug, something we (and the final user) assumed the 4.7 version was going to handle but @pauldorehill just found out that surprisingly it didn't.

So, this is clearly a bug conceptually speaking, the behavior is unexpected and there is no logical reason to forbid _ in presence of access modifiers.

[cartermp]

@KevinRansom This is a bug fix, not a new feature. Not everything related to language features has to go under a flag.

[KevinRansom]

@cartermp
so the compiler generates, incorrect code when it sees the single _ or it fails to build with an error message that describes how to fix it?

And with and without the fix will the compiler generate code both times? or will it fail with an error when compiling the same FSharp 4.7 code on one of the compilers?

To be clear … the compiler works as intended in FSharp4.7 today. This PR relaxes a restriction imposed by the compiler.

[cartermp]

This is no different than any other bug fix or feature improvement where a compile error with an older compiler doesn't exist on a newer one.

This is not something that needs to go under LangVersion. We shouldn't stuff anything language-related under it just because we have it.

[KevinRansom]

@cartermp

We used to add new language features throughout the point release cycle, Whereby F#4.7 is the version of the language as it exists in the final point release? Is that still the plan?

[cartermp]

This is not a new language feature. This is a small augmentation to an existing language feature (and arguably a bug since the error violates user expectations, and fixing it doesn't break anything).

[gusty]

Standing aside from my position I find this discussion interesting.

Back to my position and trying to reconcile with Kevin's initial statement:

From a grab the latest version of the language tools, we find that people fall into two groups.

1. Those who grab the initial release and stick with it through the release cycle.
2. Those who update with every dot release.
   Someone in group 1 would not be able to build code from someone in group2 that relied on this new language feature.

I would amend it like this:

1. Those who grab the initial release and stick with it through the release cycle, applying only updates
2. Those who update with every dot release.

Someone in group 1 would not be able to build code from someone in group2 that relied on this new language feature, if he missed updates

[KevinRansom]

@Gustavo Guerra I believe that our metadata shows a chunk of people stay on RTM without applying updates, although a bigger chunk apply updates. I believe there is hardly any that mix it up and take some updates, E.g. 2019.2 and stop there. I’m not certain what happens with the dotnet cli installs that are separate from VS, I imagine they never get updated, until the developer remembers and updates manually. But that is just my feeling, I have no data available one way or another. Regardless, code may be compiled by FSharp4.7 compilers without this “bug fix”. Labelling this source code incompatibility “a bug fix” will enable us to provoke this outcome. Kevin

[dsyme]

Yes, as I understand it this is a fix to the 4.7 feature.

[0x53A]

In the VS 2017 cycle C# shipped a point release of the language with an update.

Would that make sense here?

Add a new langversion 4.7.1 and ship it with VS 2019.4 (or whichever version this will be merged)?

[cartermp]

We're not planning on a version between now and F# 5 (certainly not for this).

[cartermp]

Just as a point of clarification: since this is being viewed as a bug fix, there's no rev to the F# language going to be involved.