-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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 LangVersion 7.2 for C# #19846
Add LangVersion 7.2 for C# #19846
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1488,5 +1488,9 @@ internal enum ErrorCode | |
ERR_PatternWrongGenericTypeInVersion = 8314, | ||
|
||
#endregion diagnostics introduced for C# 7.1 | ||
|
||
#region diagnostics introduced for C# 7.2 | ||
ERR_FeatureNotAvailableInVersion7_2 = 8320, | ||
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. Since this is going into master, and there are no concers of conflicts, we can just use 8315? 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. This is going into 15.6 branch. 7.1 is not closed yet, so I left some buffer. (Just today I had to use a new error code for a 7.1 fix). |
||
#endregion diagnostics introduced for C# 7.2 | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -74,6 +74,11 @@ public enum LanguageVersion | |
/// </summary> | ||
CSharp7_1 = 701, | ||
|
||
/// <summary> | ||
/// C# language version 7.2 | ||
/// </summary> | ||
CSharp7_2 = 702, | ||
|
||
/// <summary> | ||
/// The latest version of the language supported. | ||
/// </summary> | ||
|
@@ -94,6 +99,7 @@ internal static bool IsValid(this LanguageVersion value) | |
case LanguageVersion.CSharp6: | ||
case LanguageVersion.CSharp7: | ||
case LanguageVersion.CSharp7_1: | ||
case LanguageVersion.CSharp7_2: | ||
return true; | ||
} | ||
|
||
|
@@ -120,6 +126,8 @@ internal static ErrorCode GetErrorCode(this LanguageVersion version) | |
return ErrorCode.ERR_FeatureNotAvailableInVersion7; | ||
case LanguageVersion.CSharp7_1: | ||
return ErrorCode.ERR_FeatureNotAvailableInVersion7_1; | ||
case LanguageVersion.CSharp7_2: | ||
return ErrorCode.ERR_FeatureNotAvailableInVersion7_2; | ||
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.
What is the benefit of having an error code per version? It looks like we could easily use one parameterized error message. In fact, for some features in some situations we do this (use parameterized error message) when these errors cannot be used. #Closed 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'd raised that question before. Someone (maybe Vlad or Neal) had previously suggested this helps telemetry. 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. Dug up the telemetry comment (from Cyrus): #17435 (comment) @CyrusNajmabadi Could you expand on why separate error codes and diagnostics is preferable for LanguageVersion, rather than one parameterized set? 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. My telemetry comment was about you instantiating SolutionChangeAction directly. You need to create a subclass of that. I do not care if you use a single diagnostic ID or different diagnostic IDs. 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. Thanks for the clarification. I have a follow-up issue to use a generic diagnostic ID. 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 think @gafter was the person who pointed out the telemetry issue to me. He can confirm if that is still an issue or not. |
||
default: | ||
throw ExceptionUtilities.UnexpectedValue(version); | ||
} | ||
|
@@ -164,6 +172,8 @@ public static string ToDisplayString(this LanguageVersion version) | |
return "7.0"; | ||
case LanguageVersion.CSharp7_1: | ||
return "7.1"; | ||
case LanguageVersion.CSharp7_2: | ||
return "7.2"; | ||
case LanguageVersion.Default: | ||
return "default"; | ||
case LanguageVersion.Latest: | ||
|
@@ -235,6 +245,10 @@ public static bool TryParse(this string version, out LanguageVersion result) | |
result = LanguageVersion.CSharp7_1; | ||
return true; | ||
|
||
case "7.2": | ||
result = LanguageVersion.CSharp7_2; | ||
return true; | ||
|
||
default: | ||
result = LanguageVersion.Default; | ||
return false; | ||
|
@@ -249,7 +263,7 @@ public static LanguageVersion MapSpecifiedToEffectiveVersion(this LanguageVersio | |
switch (version) | ||
{ | ||
case LanguageVersion.Latest: | ||
return LanguageVersion.CSharp7_1; | ||
return LanguageVersion.CSharp7_2; | ||
case LanguageVersion.Default: | ||
return LanguageVersion.CSharp7; | ||
default: | ||
|
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.
I suggest to do a string search/replace over the modified error strings in the tests project, to make sure that the error comments in tests are updated as well.
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.
The resource strings I modified do not exist in tests. But I do have a test to extract the versions that appear on that line and verifies that it's complete: https://github.com/dotnet/roslyn/pull/19113/files#diff-492ea20f4ae7785ece041adfb13b4e7d