-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Switch to using the .NET SDK properties that are defined to provide the TFM instead of using the assembly that defines System.Object #80646
Switch to using the .NET SDK properties that are defined to provide the TFM instead of using the assembly that defines System.Object #80646
Conversation
…he TFM instead of using the assembly that defines System.Object
…ational version of the source generator. This will allow this change to be used by the SDK without SDK changes to expose the TFM properties to the source generators (as we don't ship Microsoft.Interop.LibraryImportGenerator.targets outside of the repository today).
Tagging subscribers to this area: @dotnet/interop-contrib Issue DetailsSwitch to using MSBuild properties defined by the .NET SDK or inferring the TFM from the generator version (as it ships with the TFM and is versioned identically) and using another SyntaxProvider to enable us to construct a We should backport this PR to .NET 7 to ensure good performance in VS.
|
src/libraries/Common/tests/SourceGenerators/GlobalOptionsOnlyProvider.cs
Show resolved
Hide resolved
...en/Microsoft.Interop.SourceGeneration/IncrementalGeneratorInitializationContextExtensions.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Runtime.InteropServices/tests/Common/TargetFrameworkOptions.cs
Show resolved
Hide resolved
...en/Microsoft.Interop.SourceGeneration/IncrementalGeneratorInitializationContextExtensions.cs
Outdated
Show resolved
Hide resolved
@chsienki PTAL |
...en/Microsoft.Interop.SourceGeneration/IncrementalGeneratorInitializationContextExtensions.cs
Outdated
Show resolved
Hide resolved
...en/Microsoft.Interop.SourceGeneration/IncrementalGeneratorInitializationContextExtensions.cs
Outdated
Show resolved
Hide resolved
...en/Microsoft.Interop.SourceGeneration/IncrementalGeneratorInitializationContextExtensions.cs
Outdated
Show resolved
Hide resolved
…ed mechanism to determine TFM since it's more for correctness than perf.
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.
LGTM. An alternative approach would be to use the MetadataReferencesProvider
to look at System.Runtime
or appropriate to figure out the target version rather than relying on the TFM from MSBuild; that would also only change when the user actually changes the TFM, rather than per key stroke, but I think either approach is fine.
@chsienki Would this mean looking at the version of the |
I looked at using the MetadataReferencesProvider. The up-side is that it will work in more scenarios and we won't need to fall back to our "use current assembly version" case as often. However, it looks like it can be a file path or an assembly name, which makes it hard to parse and adds complexity. I think for now, this is the best solution. |
The timeout is a known issue according to Build Analysis, so I'm going to merge this in. |
…he TFM instead of using the assembly that defines System.Object (dotnet#80646) Fixes dotnet#80621
Switch to using MSBuild properties defined by the .NET SDK or inferring the TFM from the generator version (as it ships with the TFM and is versioned identically) and using another SyntaxProvider to enable us to construct a
StubEnvironment
object accurately without having to query any information directly from theCompilation
object. In addition to fixing #80621, this change effectively enables all of the calculation work here to be done less often in a VS session. Previously we recalculated all of the information per keystroke. Now we'll only recalculate theSkipLocalsInit
check when the user provides or removes aSkipLocalsInit
attribute in their code or changes their TFM.We should backport this PR to .NET 7 to ensure good performance in VS.
Fixes #80621