Uh oh!
There was an error while loading. Please reload this page.
Fix FS1182 not reported for unused let functions in class types - #19805
Conversation
Removed the 'not v.IsCompiledAsTopLevel' guard in the local 'reportIfUnused' helper inside CheckIncrementalClasses.IncrClassReprInfo.ChooseRepresentation. Class-let function bindings are promoted to compiler-generated members, which sets IsCompiledAsTopLevel=true and previously silently suppressed FS1182. The helper is local to class-binding representation choice, so this change cannot introduce module-level false positives. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
❗ Release notes requiredYou can open this PR in browser to add release notes: open in github.dev
|
T-Gro
left a comment
There was a problem hiding this comment.
Review Summary
Looks good — Clean, well-scoped fix with thorough test coverage.
Analysis
The root cause is correctly identified: class let-functions are promoted to compiler-generated members via AdjustValToHaveValReprInfo (line 370), which sets ValReprInfo, making IsCompiledAsTopLevel = true. The old not v.IsCompiledAsTopLevel guard in reportIfUnused() then silently suppressed FS1182 for all class let-functions.
Why the fix is safe:
No module-level false positives —
reportIfUnused()is local toIncrClassReprInfo.ChooseRepresentation, which only processes class-level bindings. Module-level bindings go throughPostInferenceChecks.BindValwhich has its own independentIsCompiledAsTopLevelguard (line 315 in PostInferenceChecks.fs).No duplicate warnings — After
AdjustValToHaveValReprInforuns, the class let-function hasIsCompiledAsTopLevel = true, soPostInferenceChecks.BindValwon't also emit FS1182 (its conditionnot v.IsCompiledAsTopLevel || topLevelBindingHiddenBySignatureFile()evaluates tofalsefor class let-functions that aren't module bindings).HasBeenReferenced is reliable — This flag is set during type-checking when the value is referenced, and applies uniformly to let-values and let-functions.
Tests
Excellent coverage:
- RED tests pin the bug fix (unused let-function variants, mixed used/unused, static let-function)
- GUARD tests prevent regressions (used functions, recursive self-reference, module-level, underscore-prefixed)
Minor
The release notes entry should include the PR link (as noted by the bot): [PR #19805](https://github.com/dotnet/fsharp/pull/19805).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Remove unused 'addHash' in FSharpProjectSnapshot (dead code in class) - Remove unused 'removeImplFilesThatHaveSignatures' in TransparentCompiler (dead code, only referenced in commented-out line) - Suppress FS1182 in FSharp.Compiler.Interactive.Settings (empty .txt resource file generates unreferenced GetStringFunc boilerplate) - Add #nowarn 1182 to code generator preamble for long-term fix - Add PR link to release notes entry Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- DependencyManagerInteractiveTests: prefix unused 'getErrors' with '_' (only referenced in commented-out test) - FSharpScriptTests: guard 'copyHousingToTemp' with #if NETSTANDARD (only used inside NETSTANDARD-conditional member) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
T-Gro
commented
Jun 3, 2026
/azp run fsharp-ci |
|
Azure Pipelines successfully started running 1 pipeline(s). |
🔍 Tooling Safety Check — Affects-Build-Infra
|
Uh oh!
There was an error while loading. Please reload this page.
Address review feedback: remove the unused _getErrors helper function and the commented-out test that references it, since both have been dead code for years. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
MSBuild 18.9+ transitively brings System.Resources.Extensions and System.Text.Json which reference System.ValueTuple 4.0.5.0, conflicting with the .NET Framework 4.7.2 facade (4.0.2.0). Binding redirects resolve this at runtime; suppress the build error under /warnaserror. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Uh oh!
There was an error while loading. Please reload this page.
Branch was behind main by 7 commits, most notably #19805 (Fix FS1182 not reported for unused let functions in class types). Diff vs origin/main was showing #19805's changes as 'reverted' by this branch — pure noise from stale base. Auto-merge takes main's version for all those files, shrinking the PR diff by ~110 LOC. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Summary
Fix#13849: FS1182 ("unused binding") warning was not reported for unused
letfunction bindings inside class types.Problem
Class-let function bindings are promoted to compiler-generated members, which sets
IsCompiledAsTopLevel=true. ThereportIfUnusedhelper inCheckIncrementalClasses.IncrClassReprInfo.ChooseRepresentationhad anot v.IsCompiledAsTopLevelguard that silently suppressed FS1182 for these bindings.Fix
Removed the
not v.IsCompiledAsTopLevelguard from the localreportIfUnusedhelper. This helper is scoped to class-binding representation choice, so the change cannot introduce module-level false positives.Test
Added test cases in
tests/FSharp.Compiler.ComponentTests/CompilerOptions/fsc/warnon/warnon.fsverifying that FS1182 is correctly emitted for unusedletfunctions in class types.Fixes#13849