-
Notifications
You must be signed in to change notification settings - Fork 483
Add support for byref-like (ref struct) parameter and return types such as Span<T> and ReadOnlySpan<T>
#712
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
base: master
Are you sure you want to change the base?
Conversation
75d36f1 to
34dde0e
Compare
34dde0e to
9abd5df
Compare
src/Castle.Core/DynamicProxy/Generators/MethodWithInvocationGenerator.cs
Outdated
Show resolved
Hide resolved
| { | ||
| if (checkType != type) | ||
| { | ||
| throw new AccessViolationException(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we'll probably be adding a documentation page regarding by-ref-like values during interception, perhaps we could add exception messages with a link to the documentation page for more info...?
| // TODO: perhaps we should cache these `ConstructorInfo`s? | ||
| ConstructorInfo proxyCtor = typeof(ByRefLikeProxy<>).MakeGenericType(dereferencedArgumentType).GetConstructors().Single(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should implement ConstructorInfo caching here, as the "todo" comment suggest.
Since Span<T> and ReadOnlySpan<T> are commonly encountered types in the FCL, perhaps constructed generic types deriving from these should also be in the cache.
| new MethodInvocationExpression( | ||
| ThisExpression.Instance, | ||
| InvocationMethods.GetArgumentValue, | ||
| new LiteralIntExpression(i)), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An alternative to calling IInvocation.GetArgumentValue would be to query IInvocation.Arguments once, and then access the returned arguments array directly for every by-ref-like parameter? That might be more efficient for methods having more than one by-ref-like-typed parameter.
src/Castle.Core/DynamicProxy/Generators/InvocationTypeGenerator.cs
Outdated
Show resolved
Hide resolved
fc07c42 to
e839375
Compare
I originally called them "proxies" instead of "references", which seemed a little misleading given that DynamicProxy proxies provide the same public interface as the proxied types, which isn't the case with these new types. Alternative names considered were "value accessors" and "arguments". The former would lead to long type names (`ByRefLikeValueAccessor`), and the latter would be inaccurate once we are going to start using these types for `IInvocation.ReturnValue`, too. Also, I originally distinguished between an internal set of class types, and a set of public-facing interfaces. However there seemed to be little benefit to having two parallel type hierarchies. On the contrary: users observing (say) a `SpanReference` instance in the debugger and then being told in the documentation to access it thru the `ISpanReference` interface doesn't seem particularly user-friendly (nor discoverable). Therefore I went with the simpler design: I abandoned the interfaces.
Previously, four code locations were identified where "marshalling" of
byref-like values must take place:
+--------+ 1 +--------------+ 2 +---------------+
| | ------> | | ------> | |
| caller | | interceptors | | target method |
| | <------ | | <------ | |
+--------+ 4 +--------------+ 3 +---------------+
1. between calling user code and the interception pipeline
(typed byref-like arguments are put into `IInvocation`)
2. between the interception pipeline and target methods
(byref-like values are read from `IInvocation` into typed
parameters)
3. between target methods and the interception pipeline
(typed byref-like arguments are put back into `IInvocation`)
4. between the interception pipeline and calling user code
(byref-like values are read from `IInvocation` back into typed
parameters)
Currently, byref-like values simply get "nullified" (replaced with
`null` or their default value), but we want to change this behavior to
one that preserves them, so we replace the existing tests with new ones.
The general idea behind the new test fixtures is to test every one of
the above four boundaries in isolation.
We also add a test fixture for certain edge cases.
All of the added tests except those in `ProxyableTestCase` will fail
at this time.
... instead of nullifying them by making use of the `ByRefLikeReference` family of utility types created earlier.
... when byref-likes are present, as they will be gone from the evalua- tion stack after the intercepted method has returned for the first time. (DynamicProxy could in theory be made to tolerate nullified `ByRefLike- Reference` substitutes, but this doesn't seem worth the effort: after all, `IInvocationProceedInfo` is intended mostly for async interception scenarios, and even C# disallows `ref struct`s in `async` methods.)
... by creating defensive copies of byref-like `in` arguments and ref- erencing those copies instead of the actual parameters.
a3126c6 to
c9f7a72
Compare
ref struct) parameter and return types such as Span<T> and ReadOnlySpan<T>
Unlike my earlier draft #664, this PR will not require DynamicProxy user code to set up any value converters for byref-like parameters.
Instead, any byref-like argument will automatically get substituted in
IInvocationwith a "reference" type:SpanArgument<T>SpanProxy<T>SpanReference<T>forSpan<T>valuesReadOnlySpanArgument<T>ReadOnlySpanProxy<T>ReadOnlySpanReference<T>forReadOnlySpan<T>valuesByRefLikeProxyByRefLikeReferenceon .NET 8 for any non-span by-ref-like valuesByRefLikeArgument<TByRefLike>ByRefLikeProxy<TByRefLike>ByRefLikeReference<TByRefLike>on .NET 9+ for any non-span by-ref-like values of typeTByRefLikeEach of these (except the non-generic
ByRefLikeReference) has aref-returningValueproperty for accessing the actual value.These class types are essentially references to the actual byref-like parameters. They use unmanaged pointers (
void*) under the hood. I've added a big comment in theByRefLikeReference.cscode file explaining why I think this is safe.This should now be ready for merging:
ByRefLikeTestCase(which still expect byref-like parameters to default / get nullified)IInvocation→ preview it hereref/contract filesWill fix #651 and close #663 once completed and merged.