[NativeAOT] Implement support for UnsafeAccessorAttribute
#86438
Labels
Milestone
Comments
|
Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas Issue DetailsImplement #81741 for NativeAOT.
See #86161 for CoreCLR support
|
6 tasks
vitek-karas
added a commit
that referenced
this issue
Jul 18, 2023
The core of the change is that `UnsafeAccessor` creates a code dependency from the accessor method to the target specified by the attribute. The trimmer needs to follow this dependency and preserve the target. Additionally, because the trimmer operates at the IL level, it needs to make sure that the target will keep its name and some other properties intact (so that the runtime implementation of the `UnsafeAccessor` can still work). Implementation choices: * The trimmer will mark the target as "accessed via reflection", this is a simple way to make sure that name and other properties about the target are preserved. This could be optimized in the future, but the savings are probably not that interesting. * The implementation ran into a problem when trying to precisely match the signature overload resolution. Due to Cecil issues and the fact that Cecil's resolution algorithm is not extensible, it was not possible to match the runtime's behavior without adding lot more complexity (currently it seems we would have to reimplement method resolution in the trimmer). So, to simplify the implementation, trimmer will mark all methods of a given name. This means it will mark more than necessary. This is fixable by adding more complexity to the code base if we think there's a good reason for it. * Due to the above choices, there are some behavioral differences: * Trimmer will warn if the target has data flow annotations, always. There's no way to "fix" this in the code without a suppression. * Trimmer will produce different warning codes even if there is a true data flow mismatch - this is because it treats the access as "reflection access" which produces different warning codes from direct access. * These differences are fixable, but it was not deemed necessary right now. * We decided that analyzer will not react to the attribute at all, and thus will not produce any diagnostics around it. The guiding reason to keep the implementation simple is that we don't expect the unsafe accessor to be used by developers directly, instead we assume that vast majority of its usages will be from source generators. So, developer UX is not as important. Test changes: * Adds directed tests for the marking behavior * Adds tests to verify that `Requires*` attributes behave correctly * Adds tests to verify that data flow annotations behave as expected (described above) * The tests are effectively a second validation of the NativeAOT implementation as they cover NativeAOT as well. Fixes in CoreCLR/NativeAOT: This change fixes one bug in the CoreCLR/NativeAOT implementation, unsafe accessor on a instance method of a value type must use "by-ref" parameter for the `this` parameter. Without the "by-ref" the accessor is considered invalid and will throw. This change also adds some tests to the CoreCLR/NativeAOT test suite. Part of #86161. Related to #86438. Feature design in #81741.
|
@vitek-karas Can we close this now? |
|
Yes - this is done I think. |
ghost
locked as resolved and limited conversation to collaborators
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Implement #81741 for NativeAOT.
UnsafeAccessorAttributenon-generic support #86932UnsafeAccessorAttributenon-generic support #86932See #86161 for CoreCLR support
The text was updated successfully, but these errors were encountered: