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.

Sign up for GitHub

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

Make MaxSupportedLangVersion calculation dynamic #75795

Merged
merged 5 commits into from
Nov 22, 2024
Merged

Make MaxSupportedLangVersion calculation dynamic #75795

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
be2840a
Make MaxSupportedLangVersion calculation dynamic
kasperk81 Nov 7, 2024
be16bc0
Update TargetTests.cs
kasperk81 Nov 7, 2024
3a088af
cap to max available version
kasperk81 Nov 7, 2024
7c63e3c
syntax
kasperk81 Nov 7, 2024
510f13f
Update CommandLineTests.cs
kasperk81 Nov 21, 2024
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
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Test/CommandLine/CommandLineTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1727,7 +1727,7 @@ public void LanguageVersionAdded_Canary()
// When a new version is added, this test will break. This list must be checked:
// - update the "UpgradeProject" codefixer
// - update all the tests that call this canary
// - update MaxSupportedLangVersion (a relevant test should break when new version is introduced)
// - update _MaxAvailableLangVersion (a relevant test should break when new version is introduced)
// - email release management to add to the release notes (see old example: https://github.com/dotnet/core/pull/1454)
AssertEx.SetEqual(new[] { "default", "1", "2", "3", "4", "5", "6", "7.0", "7.1", "7.2", "7.3", "8.0", "9.0", "10.0", "11.0", "12.0", "13.0", "latest", "latestmajor", "preview" },
Enum.GetValues(typeof(LanguageVersion)).Cast<LanguageVersion>().Select(v => v.ToDisplayString()));
Expand Down
34 changes: 14 additions & 20 deletions src/Compilers/Core/MSBuildTask/Microsoft.CSharp.Core.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,31 +7,25 @@
<!-- .NETCoreApp < 3.0, .NETStandard < 2.1, or any other target framework -->
<_MaxSupportedLangVersion Condition="('$(TargetFrameworkIdentifier)' != '.NETCoreApp' OR '$(_TargetFrameworkVersionWithoutV)' &lt; '3.0') AND
('$(TargetFrameworkIdentifier)' != '.NETStandard' OR '$(_TargetFrameworkVersionWithoutV)' &lt; '2.1')">7.3</_MaxSupportedLangVersion>

<!-- .NETCoreApp < 5.0, .NETStandard == 2.1 -->
<_MaxSupportedLangVersion Condition="(('$(TargetFrameworkIdentifier)' == '.NETCoreApp' AND '$(_TargetFrameworkVersionWithoutV)' &lt; '5.0') OR
('$(TargetFrameworkIdentifier)' == '.NETStandard' AND '$(_TargetFrameworkVersionWithoutV)' == '2.1')) AND
'$(_MaxSupportedLangVersion)' == ''">8.0</_MaxSupportedLangVersion>

<!-- .NETCoreApp == 5.0 -->
<_MaxSupportedLangVersion Condition="'$(TargetFrameworkIdentifier)' == '.NETCoreApp' AND '$(_TargetFrameworkVersionWithoutV)' == '5.0' AND
'$(_MaxSupportedLangVersion)' == ''">9.0</_MaxSupportedLangVersion>

<!-- .NETCoreApp == 6.0 -->
<_MaxSupportedLangVersion Condition="'$(TargetFrameworkIdentifier)' == '.NETCoreApp' AND '$(_TargetFrameworkVersionWithoutV)' == '6.0' AND
'$(_MaxSupportedLangVersion)' == ''">10.0</_MaxSupportedLangVersion>

<!-- .NETCoreApp == 7.0 -->
<_MaxSupportedLangVersion Condition="'$(TargetFrameworkIdentifier)' == '.NETCoreApp' AND '$(_TargetFrameworkVersionWithoutV)' == '7.0' AND
'$(_MaxSupportedLangVersion)' == ''">11.0</_MaxSupportedLangVersion>

<!-- .NETCoreApp == 8.0 -->
<_MaxSupportedLangVersion Condition="'$(TargetFrameworkIdentifier)' == '.NETCoreApp' AND '$(_TargetFrameworkVersionWithoutV)' == '8.0' AND
'$(_MaxSupportedLangVersion)' == ''">12.0</_MaxSupportedLangVersion>

<!-- .NETCoreApp == 9.0 -->
<_MaxSupportedLangVersion Condition="'$(TargetFrameworkIdentifier)' == '.NETCoreApp' AND '$(_TargetFrameworkVersionWithoutV)' == '9.0' AND
'$(_MaxSupportedLangVersion)' == ''">13.0</_MaxSupportedLangVersion>
<!--
Automatically calculate the maximum supported C# language version based on the .NET Target Framework.
- Pattern: .NET 5.0 uses C# 9.0, .NET 6.0 uses C# 10.0, and so on.
- Starting from C# 9.0 for .NET 5.0, we add the difference between the major .NET version and 5
to determine the correct language version.
-->
<_MaxSupportedLangVersion Condition="'$(TargetFrameworkIdentifier)' == '.NETCoreApp' AND
'$(_MaxSupportedLangVersion)' == ''">$([MSBuild]::Add(9, $([MSBuild]::Subtract($(_TargetFrameworkVersionWithoutV), 5)))).0</_MaxSupportedLangVersion>
kasperk81 marked this conversation as resolved.
Show resolved Hide resolved

<!-- Cap _MaxSupportedLangVersion if it exceeds _MaxAvailableLangVersion -->
<_MaxAvailableLangVersion>13.0</_MaxAvailableLangVersion>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jcouv When a new language version is released, we'll just need to update _MaxAvailableLangVersion here and make adjustments in TargetTests.cs (updating the net10.0 line and adding a net11.0 line to match net10.0's language version). This setup means we won’t need to modify anything else for new framework versions released during this period each year.

Copy link
Member

Choose a reason for hiding this comment

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

If we still need to update this file every year when we increase the max language version, then what do we actually save by making this change over the current system?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The current system requires two updates each year: one when a new framework version is released (around October/November, managed by dotnet/sdk) and another when a new language version is added, which is handled in this repo. With this change, we're eliminating the need for the first update, as the framework version alignment will now happen automatically.

Copy link
Member

Choose a reason for hiding this comment

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

I see. Thanks for clarifying for me. I'll let Julien be the second sign-off here though, since he does the roslyn-side of the process usually.

Copy link
Member

Choose a reason for hiding this comment

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

one [update] when a new framework version is released (around October/November, managed by dotnet/sdk)

@kasperk81 Would you mind sharing a link to one of those previous dotnet/sdk changes? That'll help provide more context

Copy link
Contributor Author

@kasperk81 kasperk81 Nov 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@jcouv it happens in the sdk each year when templates are updated. e.g. since dotnet/sdk#44349, if we use the daily build (net10.0) and do something like:

$ dotnet new record -n record1

it issues warnings:

Warning: Failed to evaluate bind symbol 'evaluatedLangVersion', it will be skipped.
Warning: Failed to evaluate bind symbol 'evaluatedLangVersion', it will be skipped.
The template "Record" was created successfully.

these type of warnings will go away when 14.0 will be introduced around july 2025 but then they will reappear around october 2025 until july 2026. pr is basically decoupling this sdkVersion + 1 to compilerLatestVersion binding so things keep working smoothly throughout the year.

Copy link
Member

Choose a reason for hiding this comment

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

The comment on the C# test named LanguageVersionAdded_Canary should be updated to say _MaxAvailableLangVersion instead of MaxSupportedLangVersion. Those are the instructions we follow when adding a new language version.

<_MaxSupportedLangVersion Condition="'$(_MaxSupportedLangVersion)' != '' AND
'$(_MaxSupportedLangVersion)' &gt; '$(_MaxAvailableLangVersion)'">$(_MaxAvailableLangVersion)</_MaxSupportedLangVersion>

<MaxSupportedLangVersion>$(_MaxSupportedLangVersion)</MaxSupportedLangVersion>
<LangVersion Condition="'$(LangVersion)' == '' AND '$(_MaxSupportedLangVersion)' != ''">$(_MaxSupportedLangVersion)</LangVersion>
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/Core/MSBuildTaskTests/TargetTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -403,7 +403,7 @@ public void GenerateEditorConfigCoreHandlesMalformedCompilerVisibleItemMetadata(
[InlineData(".NETCoreApp", "7.0", "11.0")]
[InlineData(".NETCoreApp", "8.0", "12.0")]
[InlineData(".NETCoreApp", "9.0", "13.0")]
[InlineData(".NETCoreApp", "10.0", "")]
[InlineData(".NETCoreApp", "10.0", "13.0")] // update when 14.0 is released

[InlineData(".NETStandard", "1.0", "7.3")]
[InlineData(".NETStandard", "1.5", "7.3")]
Expand Down
Loading