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

Enable 4.7 by default #7204

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 24 additions & 23 deletions src/fsharp/LanguageFeatures.fs
Original file line number Diff line number Diff line change
Expand Up @@ -19,40 +19,42 @@ open System
/// LanguageFeature enumeration
[<RequireQualifiedAccess>]
type LanguageFeature =
| LanguageVersion46 = 0
| LanguageVersion47 = 1
| SingleUnderscorePattern = 2
| WildCardInForLoop = 3
| RelaxWhitespace = 4
| NameOf = 5
| ImplicitYield = 6
| OpenStaticClasses = 7
| PreviewVersion = 0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't change these numbers in future.

And I still believe it'd be better to have separate enums for language versions and features.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@auduchinok.
They aren't public and shouldn't be relied on. In fact, I wouldn't mind deleting the individual feature ID's to remove the possibility that we try to add a --enablefeature:WildCardInForLoop option.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's probably reasonable to think they may become public in FCS at some point when tooling wants to see which features are available.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@auduchinok,

The smallest subdivision of the language is language version, from it one can infer the set of features available. If we choose to add or I suppose remove features, then we will release a new language version, similarly to how we release new nuget packages and assembly versions for FSharp.Core.

It is not our intention to make the information in these structures public. I can see a good argument for removing

 | Preview = 0
 | LanguageVersion46 = 1
 | LanguageVersion47 = 2

from the LanguageFeature enumeration, given that it has so far not proven to be necessary to do a SupportsFeature.LanguageVersion4.7 call.

Anyway, my 2 cents, and probably not even worth that amount of coin.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The smallest subdivision of the language is language version, from it one can infer the set of features available.

@KevinRansom Thanks for your reply. I don't, however, realise how it should be inferred by external tools using FCS given the whole module is currently internal (unless the whole logic is reimplemented in each tool). There may be situations where tooling behaviour can also change depending on whether some feature is supported in particular language version.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@auduchinok Which situations? The way it works in Roslyn, which is the model we're adopting, is that tools assume latest. But because diagnostics will kick in if something doesn't actually compile, the only weird thing is displaying information when you're in an invalid state.

Here's an example in C# 7.3 but using nullable:

image

I could believe two opposing arguments:

  1. That it is incorrect behavior to show string?, since it's not allowed in the first place.
  2. That it is correct to show string?, since it is what the programmer wrote

Since this can be resolved either by turning string? to string or setting LangVersion ot 8.0 (or preview), it's unclear to me which is the correct behavior. But it's certainly easier from a tooling standpoint to not flow LangVersion into every tooling feature.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cartermp I don't have a particular example in mind but I can definitely imagine situations where an IDE could, for instance, produce different generated code or do code formatting differently depending on the language version in future.

| LanguageVersion46 = 1
| LanguageVersion47 = 2
| SingleUnderscorePattern = 3
| WildCardInForLoop = 4
| RelaxWhitespace = 5
| NameOf = 6
| ImplicitYield = 7
| OpenStaticClasses = 8


/// LanguageVersion management
type LanguageVersion (specifiedVersion) =

// When we increment language versions here preview is higher than current RTM version
static let languageVersion46 = 4.6m
static let languageVersion47 = 4.7m

static let previewVersion = languageVersion47 // Language version when preview specified
static let defaultVersion = languageVersion46 // Language version when default specified
static let previewVersion = 9999m // Language version when preview specified
static let defaultVersion = languageVersion47 // Language version when default specified
static let latestVersion = defaultVersion // Language version when latest specified
static let latestMajorVersion = languageVersion46 // Language version when latestmajor specified

static let validOptions = [| "preview"; "default"; "latest"; "latestmajor" |]
static let languageVersions = set [| latestVersion |]
static let languageVersions = set [| languageVersion46; languageVersion47 |]

static let features = dict [|
// Add new LanguageVersions here ...
LanguageFeature.LanguageVersion47, 4.7m
LanguageFeature.LanguageVersion46, 4.6m
LanguageFeature.SingleUnderscorePattern, previewVersion
LanguageFeature.WildCardInForLoop, previewVersion
LanguageFeature.RelaxWhitespace, previewVersion
LanguageFeature.NameOf, previewVersion
LanguageFeature.ImplicitYield, previewVersion
LanguageFeature.OpenStaticClasses, previewVersion
LanguageFeature.LanguageVersion46, languageVersion46
LanguageFeature.LanguageVersion47, languageVersion47
LanguageFeature.PreviewVersion, previewVersion
LanguageFeature.SingleUnderscorePattern, languageVersion47
LanguageFeature.WildCardInForLoop, languageVersion47
LanguageFeature.RelaxWhitespace, languageVersion47
LanguageFeature.NameOf, languageVersion47
LanguageFeature.ImplicitYield, languageVersion47
LanguageFeature.OpenStaticClasses, languageVersion47
|]

let specified =
Expand Down Expand Up @@ -88,7 +90,6 @@ type LanguageVersion (specifiedVersion) =
/// Get a list of valid versions for help text
member __.ValidVersions = [|
for v in languageVersions |> Seq.sort do
let label = if v = defaultVersion || v = latestVersion then "(Default)" else ""
yield sprintf "%M %s" v label
let label = if v = defaultVersion then " (Default)" else ""
yield sprintf "%M%s" v label
|]

18 changes: 10 additions & 8 deletions src/fsharp/LanguageFeatures.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,16 @@ module internal FSharp.Compiler.Features
/// LanguageFeature enumeration
[<RequireQualifiedAccess>]
type LanguageFeature =
| LanguageVersion46 = 0
| LanguageVersion47 = 1
| SingleUnderscorePattern = 2
| WildCardInForLoop = 3
| RelaxWhitespace = 4
| NameOf = 5
| ImplicitYield = 6
| OpenStaticClasses = 7
| PreviewVersion = 0
| LanguageVersion46 = 1
| LanguageVersion47 = 2
| SingleUnderscorePattern = 3
| WildCardInForLoop = 4
| RelaxWhitespace = 5
| NameOf = 6
| ImplicitYield = 7
| OpenStaticClasses = 8


/// LanguageVersion management
type LanguageVersion =
Expand Down
10 changes: 5 additions & 5 deletions tests/fsharp/tests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -1824,25 +1824,25 @@ module VersionTests =
let ``member-selfidentifier-version4.6``() = singleTestBuildAndRunVersion "core/members/self-identifier/version46" FSC_BUILDONLY "4.6"

[<Test>]
let ``member-selfidentifier-version4.7``() = singleTestBuildAndRunVersion "core/members/self-identifier/version47" FSC_BUILDONLY "preview"
let ``member-selfidentifier-version4.7``() = singleTestBuildAndRun "core/members/self-identifier/version47" FSC_BUILDONLY

[<Test>]
let ``indent-version4.6``() = singleTestBuildAndRunVersion "core/indent/version46" FSC_BUILDONLY "4.6"

[<Test>]
let ``indent-version4.7``() = singleTestBuildAndRunVersion "core/indent/version47" FSC_BUILDONLY "preview"
let ``indent-version4.7``() = singleTestBuildAndRun "core/indent/version47" FSC_BUILDONLY

[<Test>]
let ``nameof-version4.6``() = singleTestBuildAndRunVersion "core/nameof/version46" FSC_BUILDONLY "4.6"

[<Test>]
let ``nameof-version4.7``() = singleTestBuildAndRunVersion "core/nameof/version47" FSC_BUILDONLY "preview"
let ``nameof-version4.7``() = singleTestBuildAndRun "core/nameof/version47" FSC_BUILDONLY

[<Test>]
let ``nameof-execute``() = singleTestBuildAndRunVersion "core/nameof/version47" FSC_BASIC "preview"
let ``nameof-execute``() = singleTestBuildAndRun "core/nameof/version47" FSC_BASIC

[<Test>]
let ``nameof-fsi``() = singleTestBuildAndRunVersion "core/nameof/version47" FSI_BASIC "preview"
let ``nameof-fsi``() = singleTestBuildAndRun "core/nameof/version47" FSI_BASIC

#if !FSHARP_SUITE_DRIVES_CORECLR_TESTS
module ToolsTests =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,5 @@ preview
default
latest
latestmajor
4.6 (Default)
4.6
4.7 (Default)
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,5 @@ preview
default
latest
latestmajor
4.6 (Default)
4.6
4.7 (Default)
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
SOURCE=E_NameOfIntConst.fs SCFLAGS="--langversion:preview" # E_NameOfIntConst.fs
SOURCE=E_NameOfStringConst.fs SCFLAGS="--langversion:preview" # E_NameOfStringConst.fs
SOURCE=E_NameOfAppliedFunction.fs SCFLAGS="--langversion:preview" # E_NameOfAppliedFunction.fs
SOURCE=E_NameOfIntegerAppliedFunction.fs SCFLAGS="--langversion:preview" # E_NameOfIntegerAppliedFunction.fs
SOURCE=E_NameOfPartiallyAppliedFunction.fs SCFLAGS="--langversion:preview" # E_NameOfPartiallyAppliedFunction.fs
SOURCE=E_NameOfDictLookup.fs SCFLAGS="--langversion:preview" # E_NameOfDictLookup.fs
SOURCE=E_NameOfAdditionExpr.fs SCFLAGS="--langversion:preview" # E_NameOfAdditionExpr.fs
SOURCE=E_NameOfParameterAppliedFunction.fs SCFLAGS="--langversion:preview" # E_NameOfParameterAppliedFunction.fs
SOURCE=E_NameOfAsAFunction.fs SCFLAGS="--langversion:preview" # E_NameOfAsAFunction.fs
SOURCE=E_NameOfWithPipe.fs SCFLAGS="--langversion:preview" # E_NameOfWithPipe.fs
SOURCE=E_NameOfUnresolvableName.fs SCFLAGS="--langversion:preview" # E_NameOfUnresolvableName.fs
SOURCE=E_NameOfIntConst.fs # E_NameOfIntConst.fs
SOURCE=E_NameOfStringConst.fs # E_NameOfStringConst.fs
SOURCE=E_NameOfAppliedFunction.fs # E_NameOfAppliedFunction.fs
SOURCE=E_NameOfIntegerAppliedFunction.fs # E_NameOfIntegerAppliedFunction.fs
SOURCE=E_NameOfPartiallyAppliedFunction.fs # E_NameOfPartiallyAppliedFunction.fs
SOURCE=E_NameOfDictLookup.fs # E_NameOfDictLookup.fs
SOURCE=E_NameOfAdditionExpr.fs # E_NameOfAdditionExpr.fs
SOURCE=E_NameOfParameterAppliedFunction.fs # E_NameOfParameterAppliedFunction.fs
SOURCE=E_NameOfAsAFunction.fs # E_NameOfAsAFunction.fs
SOURCE=E_NameOfWithPipe.fs # E_NameOfWithPipe.fs
SOURCE=E_NameOfUnresolvableName.fs # E_NameOfUnresolvableName.fs
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
SOURCE=version46/W_IfThenElse01.fs SCFLAGS="--langversion:4.6 --test:ErrorRanges" # version46 W_IfThenElse01.fs
SOURCE=version46/W_IfThenElse02.fs SCFLAGS="--langversion:4.6 --test:ErrorRanges" # version46 W_IfThenElse02.fs
SOURCE=version46/W_IfThenElse03.fs SCFLAGS="--langversion:4.6 --test:ErrorRanges" # version46 W_IfThenElse03.fs
SOURCE=version47/W_IfThenElse01.fs SCFLAGS="--langversion:preview --test:ErrorRanges" # version47 W_IfThenElse01.fs
SOURCE=version47/W_IfThenElse02.fs SCFLAGS="--langversion:preview --test:ErrorRanges" # version47 W_IfThenElse02.fs
SOURCE=version47/W_IfThenElse03.fs SCFLAGS="--langversion:preview --test:ErrorRanges" # version47 W_IfThenElse03.fs
SOURCE=version47/W_IfThenElse01.fs SCFLAGS="--test:ErrorRanges" # W_IfThenElse01.fs
SOURCE=version47/W_IfThenElse02.fs SCFLAGS="--test:ErrorRanges" # W_IfThenElse02.fs
SOURCE=version47/W_IfThenElse03.fs SCFLAGS="--test:ErrorRanges" # W_IfThenElse03.fs

SOURCE=IfThenElse04.fs SCFLAGS="--test:ErrorRanges --warnaserror" # IfThenElse04.fs
SOURCE=IfThenElse05.fs SCFLAGS="--test:ErrorRanges --warnaserror" # IfThenElse05.fs
Expand Down
6 changes: 3 additions & 3 deletions tests/fsharpqa/Source/Warnings/env.lst
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,9 @@
SOURCE=version46/WarnIfDiscardedInList.fs SCFLAGS="--langversion:4.6" #version46/WarnIfDiscardedInList
SOURCE=version46/WarnIfDiscardedInList2.fs SCFLAGS="--langversion:4.6" #version46/WarnIfDiscardedInList2
SOURCE=version46/WarnIfDiscardedInList3.fs SCFLAGS="--langversion:4.6" #version46/WarnIfDiscardedInList3
SOURCE=version47/WarnIfDiscardedInList.fs SCFLAGS="--langversion:preview" #version47/WarnIfDiscardedInList
SOURCE=version47/WarnIfDiscardedInList2.fs SCFLAGS="--langversion:preview" #version47/WarnIfDiscardedInList2
SOURCE=version47/WarnIfDiscardedInList3.fs SCFLAGS="--langversion:preview" #version47/WarnIfDiscardedInList3
SOURCE=version47/WarnIfDiscardedInList.fs #version47/WarnIfDiscardedInList
SOURCE=version47/WarnIfDiscardedInList2.fs #version47/WarnIfDiscardedInList2
SOURCE=version47/WarnIfDiscardedInList3.fs #version47/WarnIfDiscardedInList3
SOURCE=WarnOnlyOnLastExpression.fs
SOURCE=WarnIfPossiblePropertySetter.fs
SOURCE=DoCannotHaveVisibilityDeclarations.fs
Expand Down