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

Support trimming and AOT #239

Merged
merged 6 commits into from
Nov 26, 2022
Merged

Support trimming and AOT #239

merged 6 commits into from
Nov 26, 2022

Conversation

dmirmilshteyn
Copy link
Contributor

This PR enables the trimming and AOT analyzers, and fixes all of the corresponding warnings. There were only a few to fix, and they were all fixed by using the generic versions of some marshalling functions.

Unfortunately, those functions were introduced in .NET 4.5.1, so I had to bump the minimum framework to that. Hopefully this shouldn't cause too much of a problem, since .NET 4.0 is already long out of support.

I also added .NET 7.0 as a target framework so that AOT warnings are displayed during compilation (but that can be removed if wanted).

@flibitijibibo
Copy link
Owner

Sadly 4.0 will be a hard requirement pretty much forever (FNA requirement), but we already have a NETSTANDARD define so let's reuse that, since this is likely going to be mainly for NativeAOT anyway.

@dmirmilshteyn
Copy link
Contributor Author

Alright, I've readded 4.0 support using defines.

Copy link
Owner

@flibitijibibo flibitijibibo left a comment

Choose a reason for hiding this comment

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

Small change for old .NET, otherwise lgtm

src/SDL2.cs Show resolved Hide resolved
@flibitijibibo flibitijibibo merged commit 408bf22 into flibitijibibo:master Nov 26, 2022
@flibitijibibo
Copy link
Owner

Quick note for this one: Turns out where T : delegate isn't available for older versions... which surprised me, but oh well:

1634e5a

Would be nice to remove the redundant cast, but this is still better than nothing!

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.

2 participants