Skip to content

Add a spec augment on escaping value type instances based on RefSafetyRulesAttribute#130863

Merged
jakobbotsch merged 8 commits into
dotnet:mainfrom
jakobbotsch:spec-byref-returns
Jul 22, 2026
Merged

Add a spec augment on escaping value type instances based on RefSafetyRulesAttribute#130863
jakobbotsch merged 8 commits into
dotnet:mainfrom
jakobbotsch:spec-byref-returns

Conversation

@jakobbotsch

Copy link
Copy Markdown
Member

Mostly written by @jkotas, with some small adaptations.

Contributes to #130185

cc @dotnet/jit-contrib @davidwrighton

…tyRulesAttribute`

Co-authored-by: Jan Kotas <jkotas@microsoft.com>
Copilot AI review requested due to automatic review settings July 16, 2026 11:26
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).
14 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the ECMA-335 addendum document to define a new opt-in IL validity restriction activated by applying System.Runtime.CompilerServices.RefSafetyRulesAttribute at the assembly level, specifically constraining when this from value-type instance methods may escape (Version 11+).

Changes:

  • Add a new “III.1.7.7” section describing RefSafetyRulesAttribute-based opt-in restrictions.
  • Specify (Version 11+) rules around escaping this from value type instance methods to enable additional optimizations (e.g., boxing elimination when safe).

Comment thread docs/design/specs/Ecma-335-Augments.md Outdated
Comment thread docs/design/specs/Ecma-335-Augments.md Outdated
Comment thread docs/design/specs/Ecma-335-Augments.md Outdated
Copilot AI review requested due to automatic review settings July 16, 2026 11:29

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 4 comments.

Comment thread docs/design/specs/Ecma-335-Augments.md Outdated
Comment thread docs/design/specs/Ecma-335-Augments.md Outdated
Comment thread docs/design/specs/Ecma-335-Augments.md
Comment thread docs/design/specs/Ecma-335-Augments.md Outdated
Copilot AI review requested due to automatic review settings July 16, 2026 11:33

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Comment thread docs/design/specs/Ecma-335-Augments.md Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 16, 2026 11:37

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Comment thread docs/design/specs/Ecma-335-Augments.md
Copilot AI review requested due to automatic review settings July 16, 2026 11:43

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.

Comment thread docs/design/specs/Ecma-335-Augments.md
Comment thread docs/design/specs/Ecma-335-Augments.md
@jakobbotsch

Copy link
Copy Markdown
Member Author

Also cc @jaredpar

Comment thread docs/design/specs/Ecma-335-Augments.md Outdated
Copilot AI review requested due to automatic review settings July 16, 2026 12:25

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.

Comment thread docs/design/specs/Ecma-335-Augments.md Outdated
Comment thread docs/design/specs/Ecma-335-Augments.md
Copilot AI review requested due to automatic review settings July 16, 2026 12:29

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.

Comment thread docs/design/specs/Ecma-335-Augments.md
Comment thread docs/design/specs/Ecma-335-Augments.md Outdated
Copilot AI review requested due to automatic review settings July 16, 2026 12:34

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Comment thread docs/design/specs/Ecma-335-Augments.md

@AndyAyersMS AndyAyersMS left a comment

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.

LGTM

@jkotas jkotas left a comment

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 would be a good idea to get an explicit ack from Roslyn team to make them aware that runtime plans to take advantage of these annotations.

Maybe submit a PR to add a note about the runtime dependency to https://github.com/dotnet/csharplang/blob/main/proposals/csharp-11.0/low-level-struct-improvements.md ?

Comment thread docs/design/specs/Ecma-335-Augments.md
@jakobbotsch

Copy link
Copy Markdown
Member Author

It would be a good idea to get an explicit ack from Roslyn team to make them aware that runtime plans to take advantage of these annotations.

Maybe submit a PR to add a note about the runtime dependency to https://github.com/dotnet/csharplang/blob/main/proposals/csharp-11.0/low-level-struct-improvements.md ?

Good idea, opened dotnet/csharplang#10269. I'll wait for the explicit confirmation before merging this.

@github-actions

github-actions Bot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Workflow state for the Holistic Review Orchestrator.

{
  "version": 5,
  "last_dispatched_commit": "eca6ddaf568370033ba28bef9a0960a2e4d0d21e",
  "last_dispatched_base_ref": "main",
  "last_dispatched_base_sha": "b78a8f8b1d15898ba39eb4fd63e2578edcf16c37",
  "last_reviewed_commit": "eca6ddaf568370033ba28bef9a0960a2e4d0d21e",
  "last_reviewed_base_ref": "main",
  "last_reviewed_base_sha": "b78a8f8b1d15898ba39eb4fd63e2578edcf16c37",
  "last_recorded_worker_run_id": "29686407160",
  "review_attempt_commit": "",
  "review_attempt_base_ref": "",
  "review_attempt_count": 0,
  "max_review_attempts": 5,
  "review_history_format": "holistic-review-disclosure-v1",
  "review_history": [
    {
      "commit": "eca6ddaf568370033ba28bef9a0960a2e4d0d21e",
      "review_id": 4730733397
    }
  ]
}

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Holistic Review

Motivation: This PR (contributing to #130185) documents a new opt-in ECMA-335 augment that lets an optimizing compiler eliminate value-type boxing by defining, at the IL specification level, when the this pointer of a value-type instance method is permitted to escape. Pinning down these escape rules in the spec is a necessary precondition for the JIT and other tools to rely on the optimization safely, so the change is well-motivated.

Approach: A single new subsection, III.1.7.7 Opt-in restrictions, is added to docs/design/specs/Ecma-335-Augments.md under III.1.7 Restrictions on CIL code sequences. It ties the restrictions to the presence and version of RefSafetyRulesAttribute on a module, then adds III.1.7.7.1 covering escape of this from value-type instance methods for version 11 and above, with the UnscopedRefAttribute and non-GC-heap-storage carve-outs. The structure (numbered augment section, "Add a new section ..." preamble, version-gated subsection) matches the conventions already used throughout this file, and the prose is precise: it defines "escape," including derived pointers via field offset, before applying it.

Summary: This is a docs-only change with no code, tests, or build impact. The definitions are internally consistent, correctly scoped to the opt-in attribute and version, and align with the existing managed-pointer escape terminology elsewhere in the document. The escape definition, the two exemption conditions, and the rationale sentence about boxing elimination are clear and unambiguous. I found no actionable issues. LGTM.

Note

This review was generated by this repository's Holistic Review agentic workflow to complement the built-in Copilot review.

Generated by Holistic Review · 35 AIC · ⌖ 14.4 AIC · ⊞ 10K

@jakobbotsch
jakobbotsch merged commit 1e4f2d6 into dotnet:main Jul 22, 2026
28 checks passed
@jakobbotsch
jakobbotsch deleted the spec-byref-returns branch July 22, 2026 12:39
@dotnet-milestone-bot dotnet-milestone-bot Bot added this to the 11.0-rc1 milestone Jul 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants