Full NativeAOT support: net8.0 TFM, eliminate remaining reflection hazards, CI-enforced smoke test#243
Open
buvinghausen wants to merge 4 commits into
Open
Full NativeAOT support: net8.0 TFM, eliminate remaining reflection hazards, CI-enforced smoke test#243buvinghausen wants to merge 4 commits into
buvinghausen wants to merge 4 commits into
Conversation
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
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.
Summary
This finishes the AOT-compatibility work started in #242 end to end. That PR fixed the annotation-only gap (
IL2093onExcelDataReader.GetFieldType) but left two genuineRequiresDynamicCodehazards in place — enum handling on both the read and write paths still went through reflection that NativeAOT can't satisfy. This PR closes both, adds anet8.0target withIsAotCompatibleturned on, and backs it with an actualPublishAotsmoke test in CI so this doesn't regress silently.This branch is built directly on top of
aot_read_hotfix/#242 — it contains those same commits plus the rest of the fix. Merging this supersedes #242; recommend closing that one in favor of this one rather than reviewing/merging both.What was still broken after #242
FieldAccessor.cs:EnumAccessor<T>built itsEnum.TryParse<T>delegate viaMethodInfo.MakeGenericMethod— realIL3050, not fixable by annotation.Xlsx/XlsxDataWriter+FieldWriter.cs:EnumFieldWriter.Get(Type)usedType.MakeGenericType+Activator.CreateInstanceto construct a generic writer per enum type — same category of hazard, plus an unannotatedIL2070flow into it.Both are on the enum code path specifically, so any AOT consumer reading or writing an enum column would have hit them at publish time (or worse, at runtime if warnings went unnoticed).
What changed
Read path (
FieldAccessor.cs) —EnumAccessor<T>now calls the non-genericEnum.TryParse(Type, string, bool, out object)overload instead of reflecting up a closed generic method. No dynamic codegen at all.netstandard2.0doesn't have that 4-arg overload, so it falls back toEnum.Parse(Type, string, bool)+ try/catch under#if NETSTANDARD2_0, preserving identical (case-insensitive) parsing behavior across every TFM.Write path (
Xlsx/XlsxDataWriter+FieldWriter.cs) — the genericEnumFieldWriter<T>+MakeGenericType/Activator.CreateInstancefactory is gone. Replaced with a single non-genericEnumFieldWriterthat reads the boxed value viaDbDataReader.GetValue(ordinal)and resolves its name withEnum.GetName(Type, object).DynamicallyAccessedMembers(PublicFields)is threaded throughFieldWriter.Get(Type)and the writer's constructor (guarded#if NET5_0_OR_GREATER) so trim analysis is satisfied all the way back toDbDataReader.GetFieldType, which the BCL already annotates.Project: added
net8.0toTargetFrameworks. Fixed a latent bug in theDefineConstantsconditions while I was in there — they exact-matchednet6.0only, so a naive net8.0 addition would silently have lostSPAN;ASYNC;DATE_ONLY.IsAotCompatibleis scoped tonet8.0only (that property's supported floor is net7.0; net6.0/netstandard targets are unaffected).Enforcement: the repo already sets
TreatWarningsAsErrors=trueglobally, so turning onIsAotCompatiblefor net8.0 means any future trim/AOT regression fails the build directly — no separate gate needed.Smoke test: new
source/AotSmokeTestproject (net8.0,PublishAot=true). Round-trips string/int/enum columns throughExcelDataWriter→ExcelDataReaderand asserts the values match, specifically exercising both fixed enum paths under a realPublishAot-compiled binary rather than just static analysis. CI installsclang/zlib1g-dev, publishes it self-contained forlinux-x64, and runs the resulting native binary — that's the receipt.Testing
net8.0,net6.0,netstandard2.1,netstandard2.0) build with 0 warnings.AotSmokeTestcompiles cleanly through NativeAOT's IL-to-native stage locally; full publish+run verified in CI.