Skip to content

Full NativeAOT support: net8.0 TFM, eliminate remaining reflection hazards, CI-enforced smoke test#243

Open
buvinghausen wants to merge 4 commits into
MarkPflug:mainfrom
buvinghausen:full_aot_fix
Open

Full NativeAOT support: net8.0 TFM, eliminate remaining reflection hazards, CI-enforced smoke test#243
buvinghausen wants to merge 4 commits into
MarkPflug:mainfrom
buvinghausen:full_aot_fix

Conversation

@buvinghausen

Copy link
Copy Markdown

Summary

This finishes the AOT-compatibility work started in #242 end to end. That PR fixed the annotation-only gap (IL2093 on ExcelDataReader.GetFieldType) but left two genuine RequiresDynamicCode hazards 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 a net8.0 target with IsAotCompatible turned on, and backs it with an actual PublishAot smoke 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 its Enum.TryParse<T> delegate via MethodInfo.MakeGenericMethod — real IL3050, not fixable by annotation.
  • Xlsx/XlsxDataWriter+FieldWriter.cs: EnumFieldWriter.Get(Type) used Type.MakeGenericType + Activator.CreateInstance to construct a generic writer per enum type — same category of hazard, plus an unannotated IL2070 flow 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-generic Enum.TryParse(Type, string, bool, out object) overload instead of reflecting up a closed generic method. No dynamic codegen at all. netstandard2.0 doesn't have that 4-arg overload, so it falls back to Enum.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 generic EnumFieldWriter<T> + MakeGenericType/Activator.CreateInstance factory is gone. Replaced with a single non-generic EnumFieldWriter that reads the boxed value via DbDataReader.GetValue(ordinal) and resolves its name with Enum.GetName(Type, object). DynamicallyAccessedMembers(PublicFields) is threaded through FieldWriter.Get(Type) and the writer's constructor (guarded #if NET5_0_OR_GREATER) so trim analysis is satisfied all the way back to DbDataReader.GetFieldType, which the BCL already annotates.

Project: added net8.0 to TargetFrameworks. Fixed a latent bug in the DefineConstants conditions while I was in there — they exact-matched net6.0 only, so a naive net8.0 addition would silently have lost SPAN;ASYNC;DATE_ONLY. IsAotCompatible is scoped to net8.0 only (that property's supported floor is net7.0; net6.0/netstandard targets are unaffected).

Enforcement: the repo already sets TreatWarningsAsErrors=true globally, so turning on IsAotCompatible for net8.0 means any future trim/AOT regression fails the build directly — no separate gate needed.

Smoke test: new source/AotSmokeTest project (net8.0, PublishAot=true). Round-trips string/int/enum columns through ExcelDataWriterExcelDataReader and asserts the values match, specifically exercising both fixed enum paths under a real PublishAot-compiled binary rather than just static analysis. CI installs clang/zlib1g-dev, publishes it self-contained for linux-x64, and runs the resulting native binary — that's the receipt.

Testing

  • All 4 TFMs (net8.0, net6.0, netstandard2.1, netstandard2.0) build with 0 warnings.
  • Full existing suite: 342 tests, 339 passed / 3 skipped, 0 failed, 0 regressions.
  • AotSmokeTest compiles cleanly through NativeAOT's IL-to-native stage locally; full publish+run verified in CI.

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.

1 participant