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

Better support for unmanaged-like types #76928

Open
vmilea opened this issue Jan 25, 2025 · 8 comments
Open

Better support for unmanaged-like types #76928

vmilea opened this issue Jan 25, 2025 · 8 comments
Assignees
Labels
api-ready-for-review API is ready for review, it is NOT ready for implementation Area-Compilers Concept-API This issue involves adding, removing, clarification, or modification of an API. Feature Request Needs API Review Needs to be reviewed by the API review council
Milestone

Comments

@vmilea
Copy link

vmilea commented Jan 25, 2025

Background

Recent efforts have improved the predictability of layout in managed memory (for example, non-blittable bool / char fields no longer break sequential layout on the managed side). This enables high-performance serialization and interop without runtime marshalling or explicit layout attributes.

Motivation

I'm building an API that that takes blittable types:

void Blit(ReadOnlySpan<T> items) { ... }

"Blittable" in this case means unmanaged or nullable unmanaged. Yes, Nullable<T> where T : unmanaged is in fact blittable, let's move on. :)

Because Nullable<T> doesn't satisfy the struct / unmanaged constraints, you end up defining multiple overloads:

void Blit<T>(in T value) where T : unmanaged { ... }
void Blit<T>(in T? value) where T : unmanaged { ... }

This becomes exponentially worse the more blittable parameters there are:

void Blit<T1, T2>(in T1 a, in T2 b) where T1 : unmanaged where T2 : unmanaged { ... }
void Blit<T1, T2>(in T1 a, in T2? b) where T1 : unmanaged where T2 : unmanaged { ... }
void Blit<T1, T2>(in T1? a, in T2 b) where T1 : unmanaged where T2 : unmanaged { ... }
void Blit<T1, T2>(in T1? a, in T2? b) where T1 : unmanaged where T2 : unmanaged { ... }

So I decided to leave T unconstrained and instead throw an exception when the RuntimeHelpers.IsReferenceOrContainsReferences<T>() intrinsic returns false. This approach detects issues at runtime with zero overhead, but I'd like to add an analyzer error in case of misuse.

void Blit<[Blittable] T>(in T value) { ... }
...

Blit("oops"); // Error: Type 'string' is not Blittable

Proposed API

A couple of additions to the Roslyn API would help:

  • ITypeSymbol.IsReferenceOrContainsReferences: Equivalent to RuntimeHelpers.IsReferenceOrContainsReferences. Can already be done by checking all fields in a struct recursively, but should be faster through a dedicated API.

  • INamedTypeSymbol.LayoutKind: So the analyzer can warn against auto layout, which is currently tricky to infer (e.g. can't assume sequential layout for a struct lacking StructLayoutAttribute because it might be a forwarded type like ValueTuple and it has a TypeForwardedFromAttribute instead of the original attributes).

Risks

Can't see any.

@vmilea vmilea added Concept-API This issue involves adding, removing, clarification, or modification of an API. Feature Request labels Jan 25, 2025
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Language Design untriaged Issues and PRs which have not yet been triaged by a lead labels Jan 25, 2025
@vmilea
Copy link
Author

vmilea commented Jan 26, 2025

I've realized that IsReferenceOrContainsReference is unnecessary. Instead, you can check if the type is unmanaged or a nullable type with an unmanaged underlying type -- no recursion required.

@jaredpar jaredpar added this to the Backlog milestone Feb 5, 2025
@333fred
Copy link
Member

333fred commented Feb 5, 2025

I don't know how we'd implement LayoutKind in the compiler. Just like you, we cannot presume what the kind of the type symbol we see is; it could further change at runtime, so anything we reported would be a best guess anyway. I'll let @AlekseyTs chime in as well as the API owner, but I do not think this is an API that we can support.

@AlekseyTs
Copy link
Contributor

Compiler doesn't define or enforce type layout. Anyone can implement what they think runtime is going to do in terms of layout for a particular type (I think some of the aspects aren't documented and some are runtime version dependent). From the language point of view and for the purpose of compiling a program, compiler doesn't need to figure this out. Therefore, I do not think it is appropriate to provide an API like that directly on a type symbol.

@333fred
Copy link
Member

333fred commented Feb 5, 2025

Thanks Aleksey. I'm going to close this out as not planned.

@333fred 333fred closed this as not planned Won't fix, can't repro, duplicate, stale Feb 5, 2025
@dotnet-policy-service dotnet-policy-service bot removed the untriaged Issues and PRs which have not yet been triaged by a lead label Feb 5, 2025
@333fred 333fred added Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead and removed Area-Language Design labels Feb 5, 2025
@dotnet-policy-service dotnet-policy-service bot removed the untriaged Issues and PRs which have not yet been triaged by a lead label Feb 5, 2025
@vmilea
Copy link
Author

vmilea commented Feb 6, 2025

Compiler doesn't define or enforce type layout. Anyone can implement what they think runtime is going to do in terms of layout for a particular type (I think some of the aspects aren't documented and some are runtime version dependent). From the language point of view and for the purpose of compiling a program, compiler doesn't need to figure this out.

I'm asking for Roslyn to expose the layout kind from IL metadata, as specified in ECMA-335 I.9.5.

How the runtime determines the final layout is a separate matter. From the point of view of a library writer: unmanaged types have predictable layout since .NET 7, which I rely for critical optimizations in serialization / interop. The only pitfall is autolayout.

@AlekseyTs
Copy link
Contributor

I'm asking for Roslyn to expose the layout kind from IL metadata, as specified in ECMA-335 I.9.5.

Thank you for the clarification. I guess we have at least two options:

  1. Return just LayoutKind, i.e. the value available from NamedTypeSymbol.Layout.Kind internal API
  2. Or, return value of internal type TypeLayout from NamedTypeSymbol.Layout internal API

@vmilea Please confirm that either option is going to address your scenario.

@AlekseyTs AlekseyTs reopened this Feb 6, 2025
@AlekseyTs AlekseyTs added Need More Info The issue needs more information to proceed. api-needs-work API needs work before it is approved, it is NOT ready for implementation Needs API Review Needs to be reviewed by the API review council labels Feb 6, 2025
@vmilea
Copy link
Author

vmilea commented Feb 6, 2025

@vmilea Please confirm that either option is going to address your scenario.

Thanks, the first option is sufficient.

TypeLayout itself could be confusing. For example, what is the size of struct X { bool B; }? Is it 1 - the managed size? Or 4 - the marshalled size? No, it's actually 0 because no .size directive was given.

@dotnet-policy-service dotnet-policy-service bot added untriaged Issues and PRs which have not yet been triaged by a lead and removed Need More Info The issue needs more information to proceed. labels Feb 6, 2025
@AlekseyTs AlekseyTs added api-ready-for-review API is ready for review, it is NOT ready for implementation and removed untriaged Issues and PRs which have not yet been triaged by a lead api-needs-work API needs work before it is approved, it is NOT ready for implementation labels Feb 6, 2025
@333fred
Copy link
Member

333fred commented Feb 6, 2025

@vmilea can you please update your post by spelling out the exact API shape you're proposing?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-ready-for-review API is ready for review, it is NOT ready for implementation Area-Compilers Concept-API This issue involves adding, removing, clarification, or modification of an API. Feature Request Needs API Review Needs to be reviewed by the API review council
Projects
None yet
Development

No branches or pull requests

4 participants