Skip to content

Allow individual projects to override LangVersion #37512

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

Merged
merged 1 commit into from
Jun 8, 2020

Conversation

eerhardt
Copy link
Member

@eerhardt eerhardt commented Jun 5, 2020

Setting LangVersion in a .targets file makes it hard to override LangVersion in individual projects. Instead, move the default project setting back to the .props file and condition VB based on $(MSBuildProjectExtension), which is available in the .props file.

ILCompiler.Reflection.ReadyToRun needs to set LangVersion to 7.3 so it doesn't break ILSpy.

Fix #37498

cc @stephentoub

Setting LangVersion in a .targets file makes it hard to override LangVersion in individual projects. Instead, move the default project setting back to the .props file and condition VB based on $(MSBuildProjectExtension), which is available in the .props file.

ILCompiler.Reflection.ReadyToRun needs to set LangVersion to 7.3 so it doesn't break ILSpy.

Fix dotnet#37498
@eerhardt eerhardt requested review from ViktorHofer and safern June 5, 2020 20:15
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the area-R2RDump-coreclr Ready-to-run image dump tool label Jun 5, 2020
@ghost
Copy link

ghost commented Jun 5, 2020

Tagging subscribers to this area: @ViktorHofer
Notify danmosemsft if you want to be subscribed.

@eerhardt eerhardt requested a review from cshung June 5, 2020 20:45
Copy link
Contributor

@cshung cshung left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the quick turnaround!

@jkotas
Copy link
Member

jkotas commented Jun 6, 2020

ILCompiler.Reflection.ReadyToRun needs to set LangVersion to 7.3 so it doesn't break ILSpy.

Why is that? Isn't netstandard2.0 reference sufficient to guarantee that the project won't use anything that is unsupported in .NET Framework?

@cshung
Copy link
Contributor

cshung commented Jun 6, 2020

Why is that? Isn't netstandard2.0 reference sufficient to guarantee that the project won't use anything that is unsupported in .NET Framework?

The way ILSpy used the binary is based on a private repo I owned to build the binary and publish to my personal nuget feed separately. In my private repo, it used the standard project templates that dotnet new classlib generates, and doesn't have all the magic in the runtime repo.

I could add <LangVersion>preview</LangVersion> in my private repo. I am just worried that this will happen.

Choosing a language version newer than the default can cause hard to diagnose compile-time and runtime errors.

Excerpt from: https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/configure-language-version

At some point, I would love to not have to own a private feed and a private repo. For now, however, it seems to be the best option to allow quick iteration.

@stephentoub stephentoub merged commit 84abb55 into dotnet:master Jun 8, 2020
@eerhardt eerhardt deleted the LangVersionChange branch June 8, 2020 21:03
@ghost ghost locked as resolved and limited conversation to collaborators Dec 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow individual project to override the default LangVersion settings
7 participants