Skip to content

Allow closures to read a protected base field - #19991

Merged
T-Gro merged 6 commits into
mainfrom
fix/5302-protected-closure-access
Jun 30, 2026
Merged

Allow closures to read a protected base field#19991
T-Gro merged 6 commits into
mainfrom
fix/5302-protected-closure-access

Conversation

@T-Gro

@T-GroT-Gro commented Jun 24, 2026

Copy link
Copy Markdown
Member

Fixes#5302

// C#: class BaseClass { protected string ProtectedField; }typeDerivedClass()=inherit BaseClass()memberx.Run()=letf()= x.ProtectedField in f ()// was FS1097

Object expressions stay the boundary — the override body is a separate type, not nested under DerivedClass:

{new BaseClass()withoverridethis.ToString()=(lazy this.ProtectedField).Value }// still FS1097

--langversion:preview

@T-Gro
Enable reading a protected base field from a closure (#5302)
f7940f0
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@T-Gro
T-Gro requested a review from a team as a code ownerJune 24, 2026 12:26
@github-actions

github-actionsBot commented Jun 24, 2026•
edited
Loading

Copy link
Copy Markdown
Contributor

⚠️ Release notes required, but author opted out

Warning

Author opted out of release notes, check is disabled for this pull request.
cc @dotnet/fsharp-team-msft

@github-actionsgithub-actionsBot added the AI-Tooling-Check-Bypassed Tooling check: non-fork PR, not diff-analyzed label Jun 24, 2026
@T-Gro
T-Gro requested a review from abonieJune 25, 2026 13:48
@T-GroT-Gro added the NO_RELEASE_NOTES Label for pull requests which signals, that user opted-out of providing release notes label Jun 25, 2026
@github-project-automationgithub-project-automationBot moved this from New to In Progress in F# Compiler and ToolingJun 29, 2026
T-Gro
T-Gro commented Jun 30, 2026
View reviewed changes

@T-GroT-Gro left a comment

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

🤖 This review was generated by AI (@expert-reviewer agent). Findings may contain inaccuracies — please verify independently.

Nice, surgically-scoped fix for #5302 with strong test coverage (every closure shape, generic derived types, the object-expression soundness hole, the public-field precision case, and the without-feature rejection). The three-layer reasoning — keep the family region during type-check (except in object-expression bodies via eInObjectExpr), refuse the TLR lift in L3, and re-home the closure-bearing member in IlxGen under --realsig- — is coherent and well-commented. Refactoring exprReferencesProtectedILField/isProtectedILFieldSpec out of Optimizer.fs into AccessibilityLogic.fs for reuse is a good move.

No correctness concerns. The only notes below are non-blocking performance nits, both gated behind --langversion:preview (and, in IlxGen, additionally --realsig-), so the impact is narrow.

Comment threadsrc/Compiler/CodeGen/IlxGen.fs
Comment threadsrc/Compiler/Checking/AccessibilityLogic.fs
@T-GroT-Gro added the AI-reviewed PR reviewed by AI review council label Jun 30, 2026
Copilotand others added 4 commits June 30, 2026 11:57
Short-circuit exprReferencesProtectedILField fold once a hit is found
0767c42
Addresses review perf nits: the predicate is now invoked per-member (IlxGen) and per-TLR-candidate (SelectTLRVals), not just in the optimizer. Skip descending into a node's subtree once a protected-field access is recorded, bounding the post-hit cost to the current fold frontier instead of a full-tree walk.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Merge branch 'main' into fix/5302-protected-closure-access
dd69f48
Bring the protected-base-field-from-closure fix (#5302) up to date with
latest main (through #20005). Conflicts resolved across compiler sources;
FSharp.Compiler.Service builds clean.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Apply fantomas formatting to InnerLambdasToTopLevelFuncs.fsi
97c8f47
Fixes the CheckCodeFormatting CI failure: the multi-parameter MakeTopLevelRepresentationDecisions signature must be laid out one curried argument per line.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Merge remote-tracking branch 'origin/main' into fix/5302-protected-cl…
822181e
…osure-access
@T-Gro
T-Gro requested a review from abonieJune 30, 2026 14:53
@T-Gro
T-Gro enabled auto-merge (squash) June 30, 2026 14:53
@T-Gro
T-Gro merged commit da65353 into mainJun 30, 2026
49 checks passed
@github-project-automationgithub-project-automationBot moved this from In Progress to Done in F# Compiler and ToolingJun 30, 2026
This was referenced Jun 30, 2026
Merged
Merged
Merged
@T-Gro
T-Gro deleted the fix/5302-protected-closure-access branch July 1, 2026 08:52
@dotnet-botdotnet-bot mentioned this pull request Jul 2, 2026
Closed
T-Gro added a commit that referenced this pull request Jul 2, 2026
@T-Gro
Merge origin/main into fix/determinism-seq-vs-para
92f29ca
Resolve conflicts from the stacked-squash of #19810 (fix-deterministic-strings)
plus new main work:
- IlxGen.fs: keep #19929's CodegenFileScope/OrderKey redesign + content-derived
raw-data naming (supersedes #19810's counter approach); drop main's orphaned
PrimeRawDataValueTypeCounter/primedRawTypeCounter.
- Thread #19991's importMap/amap through MakeTopLevelRepresentationDecisions
(OptimizeInputs, InnerLambdasToTopLevelFuncs .fs/.fsi) and keep its
protected-base-field guard in GenMethodForBinding.
- CompilerGlobalState.fs: drop duplicate FreshCompilerGeneratedNameInScope.
- Keep valSourceOrderKey sort form; keep seq-vs-par CI leg; merge
DeterministicTests to Guid temp dir + 10 iterations + try/finally.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@abonieabonieabonie approved these changes

Assignees

No one assigned

Labels

AI-reviewedPR reviewed by AI review councilAI-Tooling-Check-BypassedTooling check: non-fork PR, not diff-analyzedNO_RELEASE_NOTESLabel for pull requests which signals, that user opted-out of providing release notes

Projects

F# Compiler and Tooling
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Protected member inaccessible

2 participants

@T-Gro@abonie