dotnet / dotnet/fsharp

Duplicate FS3261/FS3388/FS3262 warnings from two-phase overload resolution

Aperta
#19,782 0 commenti 0 reazioni 0 assegnatari Vedi su GitHub
Area-Compiler-Checking Area-Diagnostics Area-Nullness
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

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.