Merged
Conversation
NinoFloris
force-pushed
the
aot-compatible
branch
4 times, most recently
from
6fe8b43 to
2f7fff7
Compare
NinoFloris
force-pushed
the
aot-compatible
branch
2 times, most recently
from
ba63dc2 to
9c3122c
Compare
# Conflicts: # src/Npgsql/TypeMapping/UserTypeMapper.cs
# Conflicts: # src/Npgsql/NpgsqlMultiHostDataSource.cs
NinoFloris
force-pushed
the
aot-compatible
branch
from
9c3122c to
0bf4037
Compare
roji
approved these changes
Sep 27, 2023
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Another draft based on #5123 so be sure to just look at the last commits.
These are the remaining missing annotations to be able to switch
/srctoIsAotCompatible:trueI've detailed the cases that need special mention below:
First off, all our GetSchema and descendants are marked as RUC
RequiresUnreferencedCode, this is because the alternative is marking any Type accessible via GetFieldType (so basically any type we have a mapping for) asDynamicallyAccessedMembers.PublicFields | PublicProperties. That IMO is very excessive to enable just that crappy feature. I hope we might be able to find a more palatable alternative...Secondly we now have RDC
RequiresDynamicCodeand RUC on ourNpgsqlDataSourceBuilder, as we have plenty of reflection and codegen through its default set of resolvers. This builder is referenced byset_ConnectionString(via SetupDataSource) andget_DbProviderFactory(via DbProviderFactory.CreateDataSource). However the base type defining these properties doesn't have RDC and RUC annotations so the analyzer complains that we don't match the base signature. I don't see a better solution here than suppressing that warning. We don't want to mark the entirety of NpgsqlConnection as RDC/RUC, which would normally be the approach to these mismatches.Finally we have some antique TypeConverter crap on NpgsqlRange, this entire Parse method should probably be redone based on spans and ISpanParsable. I've marked it as RUC for now.
Closes #4414, closes #4800