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.

Sign up for GitHub

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

Jump to bottom

Use the scoped keyword in LibraryImportGenerator #73479

Merged
merged 2 commits into from
Aug 24, 2022
Merged

Use the scoped keyword in LibraryImportGenerator #73479

merged 2 commits into from
Aug 24, 2022

Conversation

jkoritzinsky
Copy link
Member

@jkoritzinsky jkoritzinsky commented Aug 5, 2022 •
edited
Loading

Use the scoped keyword to significantly simplify the caller-allocated-buffer logic for stateful marshallers while also increasing safety/keeping us from shooting ourselves in the foot with lifetimes.

This PR also updates all of the in-box generators to build against 4.4 and fixes any build errors that were encountered as part of that process.

@jkoritzinsky jkoritzinsky added area-System.Runtime.InteropServices source-generator Indicates an issue with a source generator feature labels Aug 5, 2022
@ghost ghost assigned jkoritzinsky Aug 5, 2022
@ghost
Copy link

ghost commented Aug 5, 2022

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

Issue Details

Use the scoped keyword to significantly simplify the caller-allocated-buffer logic for stateful marshallers while also increasing safety/keeping us from shooting ourselves in the foot with lifetimes.

This is in draft until we can update the version of Roslyn we build the generator against to Roslyn 4.4 as that version guarantees the existence of the scoped keyword.

Author: jkoritzinsky
Assignees: -
Labels:

area-System.Runtime.InteropServices, source-generator
Milestone: -

@jkoritzinsky jkoritzinsky marked this pull request as ready for review August 16, 2022 21:02
@jkoritzinsky jkoritzinsky added this to the 8.0.0 milestone Aug 16, 2022
@jkoritzinsky
Copy link
Member Author

Closing and reopening to retrigger all legs now that packages have been mirrored

@jkoritzinsky jkoritzinsky reopened this Aug 16, 2022
@jkoritzinsky
Copy link
Member Author

/azp run dotnet-linker-tests, runtime, runtime-dev-innerloop, runtime-staging

@azure-pipelines
Copy link

Azure Pipelines successfully started running 4 pipeline(s).

@elinor-fung
Copy link
Member

Would it make sense to separate out the version upgrade from actually using the scoped keyword in LibraryImportGenerator at this point? It seems like (fallout from) the version upgrade has gotten more involved and become the bulk of this change now, so it might be nice to keep them separate.

@jkoritzinsky
Copy link
Member Author

I was thinking about that... I'll spin it out to another PR once I get CI green (so I don't need to iterate on it much).

@jkoritzinsky
Copy link
Member Author

Moved the code-style changes to #74445. @elinor-fung can I get another quick review look?

@jkoritzinsky
Use the scoped keyword to significantly simplify the caller-allocated…
d12d883 
…-buffer logic for stateful marshallers while also increasing safety/keeping us from shooting ourselves in the foot with lifetimes.

PR feedback

Use the official ScopedKeyword API
@jkoritzinsky
Copy link
Member Author

Failure is slow macs and #73688. Merging

@jkoritzinsky jkoritzinsky merged commit 03c2146 into dotnet:main Aug 24, 2022
@jkoritzinsky jkoritzinsky deleted the scoped-stackalloc branch August 24, 2022 02:39
@CarnaViire CarnaViire mentioned this pull request Aug 24, 2022
Closed
@kasperk81 kasperk81 mentioned this pull request Sep 17, 2022
Closed
@ghost ghost locked as resolved and limited conversation to collaborators Sep 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@elinor-fung elinor-fung elinor-fung approved these changes

@AaronRobinsonMSFT AaronRobinsonMSFT AaronRobinsonMSFT approved these changes

@lewing lewing Awaiting requested review from lewing lewing is a code owner

@pavelsavara pavelsavara Awaiting requested review from pavelsavara pavelsavara is a code owner

Assignees

@jkoritzinsky jkoritzinsky

Labels
area-System.Runtime.InteropServices source-generator Indicates an issue with a source generator feature
Projects
None yet
Milestone
8.0.0
Development

Successfully merging this pull request may close these issues.
3 participants
@jkoritzinsky @elinor-fung @AaronRobinsonMSFT