Skip to content

Add metadata rules for unsafe #9814

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add metadata rules for unsafe #9814

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 37 additions & 0 deletions proposals/unsafe-evolution.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,43 @@ partial class C2

For properties, `get` and `set/init` members can be independently declared as `unsafe`; marking the entire property as `unsafe` means that both the `get` and `set/init` members are unsafe.

#### Metadata

When an assembly is compiled with the new memory safety rules, it gets marked with `MemorySafetyRulesAttribute` (detailed below), filled in with `15` as the language version. This is a signal to
any downstream consumers that any members defined in the assembly will be properly attributed with `RequiresUnsafeAttribute` (detailed below) if an `unsafe` context is required to call them.
Any member in such an assembly that is not marked with `RequiresUnsafeAttribute` does not require an `unsafe` context to be called, regardless of the types in the signature of the member.

When a member is marked as `unsafe`, the compiler will synthesize a `RequiresUnsafeAttribute` application on the member in metadata. When a user-facing member marked as `unsafe` generates
hidden members, such as an auto-property's backing field or get/set methods, both the user-facing member and any hidden members generated by that user-facing member are all marked as `unsafe`,
and `RequiresUnsafeAttribute` is applied to all of them.

```cs
namespace System.Runtime.CompilerServices
{
/// <summary>Indicates the language version of the memory safety rules used when the module was compiled.</summary>
[AttributeUsage(AttributeTargets.Module, Inherited = false)]
public sealed class MemorySafetyRulesAttribute : Attribute
Copy link
Member

@AaronRobinsonMSFT AaronRobinsonMSFT Nov 10, 2025

Choose a reason for hiding this comment

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

Will this play well with other languages? Suppose that C# 15 adds new safety rules. Now also suppose that F# adds safety rules in version 8. Does Roslyn understand which langauge version the number is related to? Are attributes under System.Runtime.CompilerServices assumed to be limited to C#?

Copy link
Member Author

@333fred 333fred Nov 10, 2025

Choose a reason for hiding this comment

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

It's based on the RefSafetyRulesAttribute scheme, which also used language version. Perhaps @jaredpar or @RikkiGibson could comment on that.

Copy link
Member

@jjonescz jjonescz Nov 11, 2025

Choose a reason for hiding this comment

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

It's documented here: https://github.com/dotnet/csharplang/blob/main/proposals/csharp-11.0/low-level-struct-improvements.md#refsafetyrulesattribute. The version is actually ignored, but it could be considered in the future if there was another "ref safety rules" version.

Copy link
Member

@AaronRobinsonMSFT AaronRobinsonMSFT Nov 11, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

@jjonescz Thanks. I don't see anything specific about the attribute and other languages though. It seems like it is implicitly a C# only attribute. This gets back to my question of, what if F# has their own ref-safety rules? or another .NET compiler decides to express their own memory safety rules?

I'm not saying we should throw it all away or even add a new API or change the existing one. I think I would like to see some clear documentation on the API though say "is specifically for Roslyn" or "is specifically for C#". I'm not going to quibble about potentially other non-Roslyn C# compilers.

Copy link
Member

@RikkiGibson RikkiGibson Nov 11, 2025

Choose a reason for hiding this comment

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

The attribute is essentially for C#. The version number in the attribute is always a C# major language version. Languages that want to interop with C#'s unsafe code, will need to recognize and understand it, and possibly even emit it themselves if they want their "unsafe methods", etc. to be understood and treated similarly as C#'s.

This gets back to my question of, what if F# has their own ref-safety rules? or another .NET compiler decides to express their own memory safety rules?

Other languages/compilers can simply be oblivious to this attribute. That is the default state of affairs. If another language decided to implement a divergent concept of unsafe, then, we would likely remain in that situation of: we are oblivious to their unsafe mechanics, and they are oblivious to ours. That seems like something we should avoid, and also something which is unlikely to arise inadvertently.

In the past, F# did some work to check ref safety for ref structs, etc. It doesn't look like they recognize RefSafetyRulesAttribute, though, they might wish to do so in the future. That's the nature of the impact I would anticipate on other major .NET languages for this feature as well.

Copy link
Member

@AaronRobinsonMSFT AaronRobinsonMSFT Nov 11, 2025

Choose a reason for hiding this comment

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

The attribute is essentially for C#.

Then that should be documented cleared on the API.

{
/// <summary>Initializes a new instance of the <see cref="MemorySafetyRulesAttribute"/> class.</summary>
/// <param name="version">The language version of the memory safety rules used when the module was compiled.</param>
public MemorySafetyRulesAttribute(int version) => Version = version;

/// <summary>Gets the language version of the memory safety rules used when the module was compiled.</summary>
public int Version { get; }
}

[AttributeUsage(AttributeTargets.Field | AttributeTargets.Method | AttributeTargets.Property | AttributeTargets.Delegate | AttributeTargets.Constructor, AllowMultiple = false, Inherited = true)]
Copy link
Member

@jkotas jkotas Nov 10, 2025

Choose a reason for hiding this comment

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

Suggested change
[AttributeUsage(AttributeTargets.Field | AttributeTargets.Method | AttributeTargets.Property | AttributeTargets.Delegate | AttributeTargets.Constructor, AllowMultiple = false, Inherited = true)]
[AttributeUsage(AttributeTargets.Field | AttributeTargets.Method | AttributeTargets.Property | AttributeTargets.Constructor, AllowMultiple = false, Inherited = true)]

As discussed offline, we may want to mark delegate Invoke method as unsafe so that delegate types are not special. AttributeTargets.Delegate is not needed in that case.

public sealed class RequiresUnsafeAttribute : Attribute
{
}
}
```

#### Compat mode
Copy link
Member

@jcouv jcouv Nov 13, 2025

Choose a reason for hiding this comment

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

I think this compat behavior is fine to reduce virality of the feature, but we should consider offering a facility to tighten our enforcement.
The scenario is: I'm enabling the new rules for my project but want to ensure my references all using the new rules too.
Maybe that's an analyzer or maybe that's a different value to the project-level flag that turns on the new rules.

Copy link
Member Author

@333fred 333fred Nov 13, 2025

Choose a reason for hiding this comment

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

That feels like the job of either an analyzer or even an SDK build task, imo.

Copy link
Member

@jcouv jcouv Nov 13, 2025

Choose a reason for hiding this comment

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

Agreed, I'm not saying it would be implemented in the compiler.

In LDM last week, we discussed the need to review the full picture with all the pieces: compiler+analyzers+guidelines+SDK to see how it all fits.
If this spec is only meant to cover the language/compiler portion, where would be the overarching design at?

Copy link
Member Author

@333fred 333fred Nov 13, 2025

Choose a reason for hiding this comment

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

@agocke's doc here, also linked in this language proposal.

Copy link
Member

@jcouv jcouv Nov 14, 2025

Choose a reason for hiding this comment

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

@agocke Either you or JanK at LDM last week mentioned that the language rules would not be sufficient by themselves to deliver on the memory safety claim we're trying to offer as part of the feature, and we may need analyzers and coding guidelines to shore up some gaps.

Will you be on point to update the overall design doc with things that fall outside of language/compiler?

  • coding guidelines for proper usage of the feature
  • analyzers or additional SDK tooling

The reason I'm asking is I'd like to capture a open question: if we design the language/compiler for more compatibility (older compilers and not-opted-into-new-rules compiler are oblivious to new "unsafecaller" modifier) then should some other part of the system allow the user to crank the enforcement to "strict"?
The scenario is: I'm enabling the new rules for my project but want to ensure my references all using the new rules too (no "oblivious" code here).

Before discussing how this would be implemented, my question is whether that's a scenario we're trying to support.


For compat purposes, and to reduce the number of false negatives that occur when enabling the new rules, we have a fallback rule for modules that have not been updated to the new rules. For such modules,
a member is considered `unsafe` if it has a pointer or function pointer type in its signature.

## Open questions

### Local functions/lambda safe contexts
Expand Down