Repository navigation
Ensure that Language Version support flows to Parser #6891
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,90 @@ | ||
| // Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. | ||
|
|
||
| /// Coordinating compiler operations - configuration, loading initial context, reporting errors etc. | ||
| module internal FSharp.Compiler.Features | ||
|
|
||
| open System | ||
|
|
||
| //------------------------------------------------------------------------------------------------------------------ | ||
| // Language version command line switch | ||
| //------------------------------------------------------------------------------------------------------------------ | ||
| // Add your features to this List - in code use languageVersion.SupportsFeature(LanguageFeatures.yourFeature) | ||
| // a return value of false means your feature is not supported by the user's language selection | ||
| // All new language features added from now on must be protected by this. | ||
| // Note: | ||
| // * The fslang design process will require a decision about feature name and whether it is required. | ||
| // * When a feature is assigned a release language, we will scrub the code of feature references and apply | ||
| // the Release Language version. | ||
|
|
||
| /// LanguageFeature enumeration | ||
| [<RequireQualifiedAccess>] | ||
| type LanguageFeature = | ||
| | LanguageVersion46 = 0 | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it really a language feature? Wouldn't it be better as a separate enum for language versions? It could be quite straightforward to check if a feature supported by a language version.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are two audiences … compiler users are focused on language version. Thus the langversion switch. Feature developers are focused on their feature. Until a feature is confirmed for a specific language version, it is assigned to the version beyond latest. The developer merely asks if their feature is supported by the specified LanguageVersion, and invokes the feature code or the old version failure path. When a feature is approved for an RTM language version, the code will be modified to check for that specific language version and the feature flag will disappear. We will do this to keep the compatibility matrix from blowing our minds. The roslyn guys tell me this is what they do too, because no one was happy with enabling individual features. |
||
| | LanguageVersion47 = 1 | ||
| | SingleUnderscorePattern = 2 | ||
| | Nullness = 1000 | ||
| | ScriptingPackageManagement = 1001 | ||
|
|
||
| /// LanguageVersion management | ||
| type LanguageVersion (specifiedVersion) = | ||
|
|
||
| // When we increment language versions here preview is higher than current RTM version | ||
| static let languageVersion46 = 4.6m | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think that using numbers is better than enum here.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well we do a numeric comparison on the number, to figure out if the version is supported or not. Sure we could compare enum ordinals, but it's not really much of a thing. |
||
| 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 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 features = dict [| | ||
| // Add new LanguageVersions here ... | ||
| LanguageFeature.LanguageVersion47, 4.7m | ||
| LanguageFeature.LanguageVersion46, 4.6m | ||
| LanguageFeature.Nullness, previewVersion | ||
| LanguageFeature.ScriptingPackageManagement, previewVersion | ||
| LanguageFeature.SingleUnderscorePattern, previewVersion | ||
|
|
||
| // Add new LanguageFeatures here ... | ||
| |] | ||
|
|
||
| let specified = | ||
| match specifiedVersion with | ||
| | "?" -> 0m | ||
| | "preview" -> previewVersion | ||
| | "default" -> latestVersion | ||
| | "latest" -> latestVersion | ||
| | "latestmajor" -> latestMajorVersion | ||
| | _ -> | ||
| match Decimal.TryParse(specifiedVersion) with | ||
| | true, v -> v | ||
| | _ -> 0m | ||
|
|
||
| /// Check if this feature is supported by the selected langversion | ||
| member __.SupportsFeature featureId = | ||
| match features.TryGetValue featureId with | ||
| | true, v -> v <= specified | ||
| | false, _ -> false | ||
|
|
||
| /// Does the languageVersion support this version string | ||
| member __.ContainsVersion version = | ||
| match version with | ||
| | "?" | "preview" | "default" | "latest" | "latestmajor" -> true | ||
| | _ -> | ||
| match Decimal.TryParse(specifiedVersion) with | ||
| | true, v -> languageVersions.Contains v | ||
| | _ -> false | ||
|
|
||
| /// Get a list of valid strings for help text | ||
| member __.ValidOptions = validOptions | ||
|
|
||
| /// 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 | ||
| |] | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| // Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. | ||
|
|
||
| /// Coordinating compiler operations - configuration, loading initial context, reporting errors etc. | ||
| module internal FSharp.Compiler.Features | ||
|
|
||
| /// LanguageFeature enumeration | ||
| [<RequireQualifiedAccess>] | ||
| type LanguageFeature = | ||
| | LanguageVersion46 = 0 | ||
| | LanguageVersion47 = 1 | ||
| | SingleUnderscorePattern = 2 | ||
| | Nullness = 1000 | ||
| | ScriptingPackageManagement = 1001 | ||
|
|
||
| /// LanguageVersion management | ||
| type LanguageVersion = | ||
|
|
||
| /// Create a LanguageVersion management object | ||
| new: string -> LanguageVersion | ||
|
|
||
| /// Get the list of valid versions | ||
| member ContainsVersion: string -> bool | ||
|
|
||
| /// Does the specified LanguageVersion support the specified feature | ||
| member SupportsFeature: LanguageFeature -> bool | ||
|
|
||
| /// Get the list of valid versions | ||
| member ValidVersions: string array | ||
|
|
||
| /// Get the list of valid options | ||
| member ValidOptions: string array |
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is interesting that we have to put this kind of error message in FSComp. Before, we relied on what the parser would give us for the error. But since we are adding logic for language version, we have no choice but to copy the same error message that the parser would create, and then create a new resource in FSComp in order to raise the same error message in conjunction with language version. While this solves the problem for the language version, there is now potential for error text to be out of sync if the parser decides to change the text for the same kind of error in a different rule. Though, in this case, it is unlikely to change anytime soon.
While we do have other parser errors in FSComp, I don't think we have errors with exact text of what the parser would normally produce on its own; we only have specific parse errors that try to be more descriptive for the user.
This is sort of a code smell in my opinion, but not a big deal. It isn't necessarily due to the language version flag itself; this problem will exist if you try to preserve old parser logic and errors behind any kind of flag. However, I only think a flag would be absolutely necessary if we change the syntax in such a way that broke existing code; which that isn't even the purpose of the language version flag anyway or something that we ever wish to do.