Skip to content

Remove PointerNativeTypeAssignmentRewriter by introducing a separate local with the exact type and assigning to/from it - #107219

Merged
jkoritzinsky merged 2 commits into
dotnet:mainfrom
jkoritzinsky:remove-syntax-rewriter
Sep 4, 2024
Merged

jkoritzinsky merged 2 commits into
dotnet:mainfrom
jkoritzinsky:remove-syntax-rewriter

Conversation

@jkoritzinsky

Copy link
Copy Markdown
Member

This removes our syntax rewriting, one of the main blockers for us to move to StringBuilder/string based codegen.

Contributes to #95882

…local with the exact type and assigning to/from it

This removes our syntax rewriting, one of the main blockers for us to move to StringBuilder/string based codegen.
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @dotnet/interop-contrib
See info in area-owners.md if you want to be subscribed.

@jkoritzinsky

Copy link
Copy Markdown
Member Author

This PR is ready for review

Comment on lines -74 to +94
// <nativeIdentifier> = <convertToUnmanaged>;
var assignment = AssignmentExpression(
SyntaxKind.SimpleAssignmentExpression,
IdentifierName(nativeIdentifier),
convertToUnmanaged);

ExpressionSyntax assignment;

if (unmanagedType is PointerTypeInfo pointer)
// For some of our exception marshallers, our marshaller returns nint for pointer types.
// As a result, we need to insert a cast here in case we're in that scenario (which we can't detect specifically).
if (unmanagedType is PointerTypeInfo ptrType)
{
// <nativeIdentifier> = (<nativeType>)<convertToUnmanaged>;
assignment = AssignmentExpression(SyntaxKind.SimpleAssignmentExpression,
IdentifierName(nativeIdentifier),
CastExpression(ptrType.Syntax, convertToUnmanaged));
}
else
{
var rewriter = new PointerNativeTypeAssignmentRewriter(assignment.Right.ToString(), (PointerTypeSyntax)pointer.Syntax);
assignment = (AssignmentExpressionSyntax)rewriter.Visit(assignment);
// <nativeIdentifier> = <convertToUnmanaged>;
assignment = AssignmentExpression(
SyntaxKind.SimpleAssignmentExpression,
IdentifierName(nativeIdentifier),
convertToUnmanaged);
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need anything like this for native marshalling?

@jkoritzinsky jkoritzinsky Sep 4, 2024 •

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We only need this for unmarshalling because we only hit it in a specific (due to us artificially resolving the marshaller) scenario. We don't put ourselves into that situation in the native->managed case.

@jkoritzinsky
jkoritzinsky merged commit 4ebb841 into dotnet:main Sep 4, 2024
@jkoritzinsky
jkoritzinsky deleted the remove-syntax-rewriter branch September 4, 2024 23:56
radekdoulik pushed a commit to radekdoulik/runtime that referenced this pull request Sep 6, 2024
jtschuster pushed a commit to jtschuster/runtime that referenced this pull request Sep 17, 2024
sirntar pushed a commit to sirntar/runtime that referenced this pull request Sep 30, 2024
@github-actions github-actions Bot locked and limited conversation to collaborators Oct 6, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-System.Runtime.InteropServices source-generator Indicates an issue with a source generator feature

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants