Skip to content
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

Split identifier calculation from code context and bind marshallers to the code context #106846

Merged
merged 10 commits into from
Aug 30, 2024

Conversation

jkoritzinsky
Copy link
Member

Split out GetIdentifier and CodeEmitOptions from the StubCodeContext into a StubIdentifierContext.

Use only TypePositionInfo and StubCodeContext to resolve to generator instances and include StubCodeContext in the information bound into a bound marshaller.

Use StubIdentifierContext during actual codegen to provide identifiers for TypePositionInfo instances.

Construct the TypePositionInfo correctly from the start so we never have to rebind and remove the Rebind method.

Keep count info structured longer (required as part of the above changes) and fix a latent bug.

Copy link
Contributor

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

…the timeout that showed up. Maybe this will fix the test runs?
@jtschuster
Copy link
Member

I'm a bit confused why we can't store the StubIdentifierContext information in the IBoundMarshallingGenerator objects (aside from the stage information). Why do we need to pass it down through the GenerateX methods? Do we not know at generator resolution / binding time whether it will be the DefaultIdentifierContext or the LinearCollectionElementIdentifierContext?

@jkoritzinsky
Copy link
Member Author

We can't store the StubIdentifierContext as we can't construct a DefaultIdentifierContext with the correct identifiers for the stub return value until we've already bound the generators. Additionally, we have some scenarios (primarily the "can we convert" analyzers) where we don't actually need identifiers at all as we aren't generating code, we're only attempting to bind generators.

@jkoritzinsky
Copy link
Member Author

jkoritzinsky commented Aug 28, 2024

Additionally (until we change how we handle ownership), we have the OwnedValueContext (which is a StubIdentifierContext) as well, which we use only during code-gen in some stages (with the same TypePositionInfo and StubCodeContext), so being able to change how we use identifiers is important.

I'm also planning refactoring how we handle the fact that pointers can't be used in generics to use a custom StubIdentifierContext instead of syntax rewriting in preparation for #95882, which means we'll have another scenario where we use a separate identifier context at codegen time from bind time.

@jkoritzinsky
Copy link
Member Author

/ba-g Firefox failure unrelated

@jkoritzinsky jkoritzinsky merged commit c87cbf6 into dotnet:main Aug 30, 2024
93 of 95 checks passed
@jkoritzinsky jkoritzinsky deleted the stub-code-context-refactor branch August 30, 2024 04:36
jtschuster pushed a commit to jtschuster/runtime that referenced this pull request Sep 17, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Oct 2, 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
None yet
Development

Successfully merging this pull request may close these issues.

2 participants