-
Notifications
You must be signed in to change notification settings - Fork 120
Fix NativeAOT delegate commands by preserving reflection metadata #203
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
Conversation
|
Thank you! var docId = dynamicDependencyMethod.GetDocumentationCommentId();
var parametersPart = docId!.Substring(docId.IndexOf('('));
var memberSignature = dynamicDependencyMethod.Name + parametersPart;
var containingType = dynamicDependencyMethod.ContainingType.ToDisplayString(DynamicDependencyContainingTypeFormat);
dynamicDependencyAttribute = $"[global::System.Diagnostics.CodeAnalysis.DynamicDependency(\"{memberSignature}\", typeof({containingType}))]"; |
|
Thanks, using |
|
thank you! |
|
My pleasure. @neuecc Considering the scope of its use, I think I could improve |
|
PooledStringWriter is unused code so no need to care.(I've just removed unused code in master branch) |
Before this PR:
When using delegate handlers such as:
and
Commands.Rootincluded attributes likeValidationAttributes, the generated code would still callcommand.Method.GetParameters()even after source generation, This requires metadata that may be trimmed when compiling with Native AOT, causing a runtime exception.What this PR does:
MethodInfowas trimmed away - exit code != 0 and test fails.Emitterto fix the issueSymbolDisplayFormatto fully qualify type names.command.CommandMethodInfois null, meaning the runtime will rely on reflection to discover the delegate’s parameters, the emitter now uses the handler’sIMethodSymbolto create aDynamicDependencyattribute that points to the handler’s containing type.RunCommandmethod signature.CommandMethodInfois not null, no changes are made.Rationale:
This change ensures that the necessary
MethodInfois preserved when it would otherwise be trimmed.An alternative approach would be to cache the method and its parameters during build time, but that would require significant refactoring of
Emitter. The current solution fixes the issue for affected users, and caching could be explored as a potential improvement in the future.