Duplicate FS3261/FS3388/FS3262 warnings from two-phase overload resolution
- Lingua principale
- F#
- Stelle
- 4.3k
- Fork
- 876
- Merge medio
- 5g 11h
- PR unite (30g)
- 153
Descrizione
### Problem
`TcMethodApplication` resolves method calls in two phases. For single-candidate calls (the common case), Phase 2 re-runs constraint checks that Phase 1 already committed, producing duplicate warnings and wasted work.
```fsharp
// --checknulls --langversion:preview
let f (c: C | null) = c.P // FS3261 emitted TWICE
```
### Root cause
**Phase 1** (`UnifyUniqueOverloading`, `ConstraintSolver.fs:4008`): For exactly 1 candidate, runs `CanMemberSigsMatchUpToCheck` with `NoTrace` to guide argument type inference. Solutions committed permanently. Warnings emitted via `CommitOperationResult` (`CheckExpressions.fs:10465`).
**Phase 2** (`ResolveOverloadingForCall`, `CheckExpressions.fs:10487`): Always called. For single candidate, skips `ResolveOverloadingCore` entirely (line 3654: `| _, [calledMeth] when not isOpConversion -> Some calledMeth, CompleteD, NoTrace`), but the final commit block (line 3733) unconditionally re-runs the full `CanMemberSigsMatchUpToCheck`. Warnings emitted via `RaiseOperationResult` (line 10527).
### What's redundant vs. new in Phase 2's final commit
| Sub-step | Redundant? | Why |
|---|---|---|
| TDC1: `TypesEquiv` (instantiation) | Yes | `calledTyArgs = minst` reused from Phase 1 (line 10455→10479). `FreshenMethInfo` in `CalledMeth` ctor only freshens property setters, not the method's type params. |
| TDC2: `TypesMustSubsume` (obj-arg) | Yes | `callerObjArgTys` computed once (line 10394), shared by both phases. Source of duplicate FS3261/3262. |
| TDC7: `ReturnTypesMustSubsumeOrConvert` | Yes | Same `returnTy`, same method signature. Source of duplicate FS3388. |
| TDC3-6: `ArgsMustSubsumeOrConvert` + paramArray + named + setters | **No** | Phase 1 used synthetic `mkSynUnit` placeholders. Phase 2 has real typed `Expr` args and `enforceNullableOptionalsKnownTypes=true`. |
### Affected warnings
FS3261 (nullness mismatch), FS3262 (strict-null-required), FS3388 (return-type TDC). Any warning from TDC1/2/7 inside `CanMemberSigsMatchUpToCheck`.
### Fix
Add a `CanMemberSigsMatchArgsOnly` helper (or guarded branch in the final commit) that runs only TDC3-6. Thread `alreadyValidatedByPhase1: bool` (= existing `uniquelyResolved` value) through `ResolveOverloadingForCall` → `ResolveOverloading`.
**Guards** (conservative — exclude from fast path):
- `not isOpConversion` — op_Implicit/op_Explicit have special return-type handling
- `cx.IsNone` — SRTP trait resolution not run through Phase 1
- `calledMethTrace = NoTrace` — single-candidate path only
**Why safe:**
- Phase 1 warnings hit the logger before Phase 2 starts (`CommitOperationResult` at line 10465)
- Multi-candidate path untouched (uses `WithTrace` replay)
- Phase 1 failure → exception at 10465 → `alreadyValidatedByPhase1` never true on error path
- Arg checking (genuinely new work) still runs fully
**Constraint on correctness:** Missing a warning (0 instead of 1) is drastically worse than duplicating (2 instead of 1). This fix only skips checks whose results are already committed and whose warnings are already emitted. No warning can be lost.
### Rollout
Consider gating behind `LanguageFeature` flag for one release. Validate with `--times` on large compilations. Update baselines containing duplicated warning lines via `TEST_UPDATE_BSL=1`.
### Related
- #19658, #17409 — nullness warning range/context improvements
Guida per i contributori
Apri la guida per i contributori
Direzione di ricerca
Inizia in ConstraintSolver.fs presso UnifyUniqueOverloading e in CheckExpressions.fs presso ResolveOverloadingForCall, ResolveOverloading e il blocco di commit finale intorno alle righe 10465-10527. Segui il percorso con un singolo candidato e valida le guards prima di limitare Phase 2 a controlli degli argomenti realmente nuovi. Il lavoro è completato quando FS3261, FS3262 e FS3388 compaiono una volta ciascuno, i casi multi-candidate e speciali rimangono invariati e le baseline interessate vengono aggiornate con TEST_UPDATE_BSL=1.
Scritto dal modello di indicizzazione a partire dal testo della issue.
Valutazione
- Stack tecnologico
- fsharp
- Ambito
- compilers
- Tipo di issue
- Bug
- Difficoltà
- 4/5
- Tempo stimato
- 3-5 giorni
- Stato di attività
- Tranquilla
- Chiarezza
- Specificata chiaramente
- Idoneità per principianti
- 48/100