Introduce ArgumentsMarshaller helpers for MethodWithInvocationGenerator and InvocationTypeGenerator
#731
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
... for `GeneratorUtil.CopyOutAndRefParameters`, so that this method does not need to dig the arguments array out of the invocation instance.
... by composing simpler node types in `CopyIn` instead. While this requires more allocations, it highlights the similarities between `Copy- In` and `CopyOut`, resulting in a cleaner interface. The newly introduced node type `ConvertArgumentToObjectExpression` may seem redundant right now (given the more general `ConvertExpression`), but it will allow us later to introduce conversions that are specific to copying arguments into & out of `IInvocation`.
... as the counterpart to `ConvertArgumentToObjectExpression`. For now, this is just a simplified version of `ConvertExpression`, but it gives us a place to add arguments-specific marshalling logic later on.
stakx
force-pushed
the
arguments-marshaller
branch
from
8f9f7ac to
eed779d
Compare
Using a sparse dictionary instead of a non-sparse plain array for track- ing mutable by-ref parameters is likely only more efficient for methods with a large number of parameters. While we're at it, prevent unnecessary allocations for parameter-less methods.
stakx
changed the title
ArgumentsMarshaller helper for MethodWithInvocationGeneratorArgumentsMarshaller helpers for MethodWithInvocationGenerator and InvocationTypeGenerator
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
This PR is one of the last preliminary code refactoring towards #663. Its goal is to pull together all logic related to the copying of argument & return values into / out of
IInvocationdone byMethodWithInvocationGeneratorandInvocationTypeGenerator.In the case of
MethodWithInvocationGenerator, that logic is currently spread out over three classes:MethodWithInvocationGeneratoritself, plusReferencesToObjectArrayExpressionandGeneratorUtil(which are both classes used exclusively, and only once each, by the former). This PR concentrates all of that logic in a single helper type,MethodWithInvocationGenerator.ArgumentsMarshaller.In the case of
InvocationTypeGenerator, the logic is already completely in that class. This PR refactors it into a second helper typeInvocationTypeGenerator.ArgumentsMarshallerfor consistency.The terms "copy in", "copy out", and "marshalling" are borrowed from remoting, since DynamicProxy creates something quite like a remoting boundary around each intercepted method. See also ECMA-335 I.12.5 and I.8.2.1.1 for more on these terms.