LibraryImportGenerator: prep for unsafe-v2#131041
Conversation
…containing type The LibraryImport source generator (and its downlevel variant) previously added an `unsafe` modifier to the generated partial method's containing type to provide an unsafe context for both the stub signature and body. In preparation for the C# "unsafe evolution" feature (https://github.com/dotnet/csharplang/blob/main/proposals/unsafe-evolution.md), where `unsafe` on a type becomes illegal and `unsafe` on a method no longer opens an unsafe context in its body, change the generator to emit code that is valid under both the current and the updated memory-safety rules: - Stop adding an `unsafe` modifier to the containing type. The user's own type modifiers are still copied verbatim, so a user-authored `unsafe partial class` is preserved. - Wrap the body of generated wrapper stubs in an explicit `unsafe { }` block. Forwarder stubs (bodyless `extern` declarations) get no unsafe block and rely on the copied user modifiers. `WrapMemberInContainingSyntaxWithUnsafeModifier` is renamed to `WrapMemberInContainingSyntax` accordingly. This is preparatory only; emitting different code specifically for the updated rules (e.g. `unsafe`/`safe` on `extern` forwarders) is left for follow-up work that depends on dotnet/roslyn#84555. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: a6c2ca1e-3757-422c-ab22-9d9d7ea3ad10
|
Azure Pipelines: Successfully started running 3 pipeline(s). 13 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
|
Tagging subscribers to this area: @dotnet/interop-contrib |
There was a problem hiding this comment.
Pull request overview
This PR updates the LibraryImportGenerator (and downlevel variant) to stop emitting unsafe on containing type declarations and instead emit an explicit unsafe { ... } block inside wrapper stub bodies, keeping the generated code shape compatible with evolving C# unsafe semantics. It also adds/adjusts unit tests to lock in the new emitted structure and updates an existing diagnostic expectation accordingly.
Changes:
- Wrap generated wrapper stub bodies in a single
unsafe { }statement block (instead of relying onunsafeon the containing type). - Update
ContainingSyntaxContextto wrap members in containing syntax without injecting anunsafetype modifier; rename the helper accordingly. - Add
UnsafeCodeGenerationstructural tests and updateCompileFails.ValidateRequireAllowUnsafeBlocksDiagnosticexpectations.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/UnsafeCodeGeneration.cs | Adds structural unit tests verifying wrapper stubs use a body unsafe block and that the generator does not inject unsafe onto containing types (while preserving user-authored unsafe). |
| src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/CompileFails.cs | Updates expectations/comments for AllowUnsafeBlocks=false now that trivial forwarder stubs don’t trigger CS0227 via generated type-level unsafe. |
| src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/ContainingSyntaxContext.cs | Renames/remodels member wrapping to avoid adding unsafe to containing types when emitting syntax-wrapped members. |
| src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/LibraryImportGenerator.cs | Wraps wrapper stub bodies in unsafe { } and switches to the new containing-syntax wrapper API. |
| src/libraries/System.Runtime.InteropServices/gen/DownlevelLibraryImportGenerator/DownlevelLibraryImportGenerator.cs | Mirrors the wrapper-stub unsafe { } body emission and containing-syntax wrapper API change for downlevel generation. |
This comment was marked as spam.
This comment was marked as spam.
|
PTAL @AaronRobinsonMSFT (assuming @jkoritzinsky is OOO) |
jtschuster
left a comment
There was a problem hiding this comment.
Could we also add test cases for forwarding stubs and marshalling stubs with 'unsafe' on the method declaration?
public static unsafe partial void Method();
public static unsafe partial void Method2(string s, int* i);| // The generator no longer emits an 'unsafe' modifier on the containing type, and the trivial | ||
| // (forwarder) stubs it produces here contain no unsafe code, so no CS0227 is reported. The | ||
| // analyzer still reports SYSLIB1062 once per compilation to recommend enabling AllowUnsafeBlocks. |
There was a problem hiding this comment.
nit: I don't think we should have a comment referencing warnings that aren't present anymore.
| // The generator no longer emits an 'unsafe' modifier on the containing type, and the trivial | |
| // (forwarder) stubs it produces here contain no unsafe code, so no CS0227 is reported. The | |
| // analyzer still reports SYSLIB1062 once per compilation to recommend enabling AllowUnsafeBlocks. | |
| // The analyzer reports SYSLIB1062 once per compilation to recommend enabling AllowUnsafeBlocks. |
| /// Unlike the containing-type modifiers that are copied verbatim from the user's declaration, | ||
| /// this intentionally does not add an <c>unsafe</c> modifier to any containing type: generated | ||
| /// members that require an unsafe context open an explicit <c>unsafe</c> block in their body instead. | ||
| /// This keeps the output valid under both the legacy and the updated (unsafe-evolution) memory-safety rules. |
There was a problem hiding this comment.
nit: I don't think the doc comment needs to explain how this differs from the old behavior.
| /// Unlike the containing-type modifiers that are copied verbatim from the user's declaration, | |
| /// this intentionally does not add an <c>unsafe</c> modifier to any containing type: generated | |
| /// members that require an unsafe context open an explicit <c>unsafe</c> block in their body instead. | |
| /// This keeps the output valid under both the legacy and the updated (unsafe-evolution) memory-safety rules. |
Prep for unsafe-v2 where
unsafeon types is illegal andunsafeon methods no longer opens an unsafe context in the body. Emit code valid under both old and new rules:unsafeto the generated stub's containing type (user's own modifiers are still copied as-is).unsafe { }block instead.extern) are unchanged.The exact behavior of LIG in unsafe-v2 context is currently blocked by dotnet/roslyn#84555 decision.