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.

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

Clean up unsupported native AOT flags #2616

Merged
merged 2 commits into from
Aug 28, 2024

Conversation

MichalStrehovsky
Copy link
Member

  • llcOptimizationPreference was renamed to OptimizationPreference and became supported in .NET 8 (or maybe .NET 7, it doesn't matter)
  • IlcGenerateStackTraceData is now StackTraceSupport and supported. (Also got renamed in .NET 8 or maybe in 7 already.)

I'm leaving the old names of properties for backcompat. We could also delete them. I don't know how much BDN cares about .NET versions that are out of support.

The last one, IlcGenerateCompleteTypeMetadata is unsupported, we don't test it, it's a mode for troubleshooting trimming issues for people who ignore trimming warnings. I don't think BDN should be setting this. You'll only find bugs (dotnet/runtime#106439). So deleting that. Not deleting the API because it looks to be a public API.

* `llcOptimizationPreference` was renamed to `OptimizationPreference` and became supported in .NET 8 (or maybe .NET 7, it doesn't matter)
* `IlcGenerateStackTraceData` is now `StackTraceSupport` and supported. (Also got renamed in .NET 8 or earlier.)

I'm leaving the old names of properties for backcompat. We could also delete them. I don't know how much BDN cares about .NET versions that are out of support.

IlcGenerateCompleteTypeMetadata is unsupported, we don't test it, it's a mode for troubleshooting trimming issues for people who ignore trimming warnings. I don't think BDN should be setting this. You'll only find bugs (dotnet/runtime#106439). So deleting that. Not deleting the API because it looks to be a public API.
@am11
Copy link
Member

am11 commented Aug 15, 2024

@MichalStrehovsky, there is another warning due to line 167:

/Users/am11/.nuget/packages/microsoft.net.illink.tasks/9.0.0-rc.1.24410.5/build/Microsoft.NET.ILLink.targets(220,5): warning : Property 'TrimmerDefaultAction' is deprecated in .NET 7 and higher and will be ignored. Use TrimMode instead.

This patch should fix it:

-                : "<TrimMode>link</TrimMode><TrimmerDefaultAction>link</TrimmerDefaultAction>";
+                : @"<TrimMode>link</TrimMode>
+    <TrimmerDefaultAction Condition=""$([MSBuild]::VersionLessThan('$(NETCoreSdkVersion)', '7.0'))"">link</TrimmerDefaultAction>";

@MichalStrehovsky
Copy link
Member Author

This patch should fix it:

We'd also need to condition TrimMode - valid values of TrimMode after .NET 7 are partial and full (with the latter as the default). I'd honestly just delete the "less than or equal to .NET 6" support for native AOT, those were all various experimental unsupported products the perf of which is not very interesting anymore.

These are however all orthogonal to what this PR is doing.

Co-authored-by: Adeel Mujahid <3840695+am11@users.noreply.github.com>
Copy link
Member

@adamsitnik adamsitnik left a comment

Choose a reason for hiding this comment

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

LGTM, thank you @MichalStrehovsky and @am11 !

@adamsitnik adamsitnik merged commit ca5dfdf into dotnet:master Aug 28, 2024
8 checks passed
@MichalStrehovsky MichalStrehovsky deleted the patch-1 branch August 28, 2024 14:01
@AndreyAkinshin AndreyAkinshin added this to the v0.14.1 milestone Aug 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants