Skip to content

Unsafe evolution speclet: add more modifier restrictions - #10289

Merged
jjonescz merged 3 commits into
dotnet:mainfrom
jjonescz:unsafe-34-safe
Jul 30, 2026
Merged

Unsafe evolution speclet: add more modifier restrictions#10289
jjonescz merged 3 commits into
dotnet:mainfrom
jjonescz:unsafe-34-safe

Conversation

@jjonescz

Copy link
Copy Markdown
Member

No description provided.

@jjonescz
jjonescz requested a review from a team as a code owner July 28, 2026 11:51
@jjonescz jjonescz changed the title Unsafe evolution speclet: add more restrictions for safe Unsafe evolution speclet: add more modifier restrictions Jul 28, 2026
Comment on lines -330 to -331
When a user-facing *requires-unsafe* member generates hidden members, such as an auto-property's get/set methods,
both the user-facing member and any hidden members generated by that user-facing member are all *requires-unsafe*, and `RequiresUnsafeAttribute` is applied to all of them.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why remove this detail?

@jjonescz jjonescz Jul 28, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It doesn't seem to be adding anything new. get/set methods are already members to which the kept sentence applies. And now with safe/unsafe keywords, the caller-safety can differ between the accessor and the containing member, so this detail would have to be modified somehow, so I just thought it's easier to remove it.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think we should keep it. We can remove the line about auto-props, but this still impacts things such as the MoveNext of compiler-async or iterators.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

this still impacts things such as the MoveNext of compiler-async or iterators.

Currently it actually does not. These members are not user-callable, and we don't emit [RequiresUnsafe] on them. (It's a good point that we should clarify that fact either way though.)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Users could use reflection to call them. We should make sure that synthesized members generated from an unsafe method are also marked as unsafe.

@jjonescz jjonescz Jul 29, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Users could use reflection to call them.

Not sure how does the fact whether [RequiresUnsafe] is on such members make a difference - unless users calling those members via reflection manually check that attribute and then do... what? add unsafe block (which IDE immediately declares as unnecessary)?

Perhaps I can just add an open question for now?

(FWIW, I don't think it was previously clear from that block of speclet that it would apply to things like MoveNext.)

@333fred 333fred Jul 29, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It was certainly my intention that it apply to them. That's what I meant by hidden members generated by that user-facing member.

@jjonescz
jjonescz merged commit e79b059 into dotnet:main Jul 30, 2026
1 check passed
@jjonescz
jjonescz deleted the unsafe-34-safe branch July 30, 2026 09:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants