reanalyze: duplicated batch/reactive pipelines have silently drifted; the editor runs the untested one
Maintainer antworten meist innerhalb von 1 Tag
Dieses Issue hat noch niemand übernommen.
Bewertung
- Schwierigkeit
- 5/5
- Geschätzter Aufwand
- Über eine Woche
- Anfängerfreundlichkeit
- 35/100
- Issue-Typ
- Bug
- Klarheit
- Größtenteils klar
- Aktivitätsstatus
- Aktiv
- Tech-Stack
- ocaml
- Bereich
- compilers, testing-qa
Rechercherichtung
Beginne mit DeadType.ml, ReactiveTypeDeps.ml und dem Pfad reanalyze_server.ml:309, um zu vergleichen, wie Re-Export-Abhängigkeiten behandelt werden. Führe make -C tests/analysis_tests test aus und untersuche anschließend die reaktiven Ziele, die nicht standardmäßig aktiviert sind, sowie ci.yml:201. Erledigt ist die Aufgabe, wenn das eingecheckte Korpus übereinstimmende Batch- und reaktive Ergebnismengen enthält, einschließlich der TypeReexport-Fixtures, und CI diese Übereinstimmung prüft.
Vom Indexierungsmodell aus dem Issue-Text verfasst.
Beschreibung
reanalyze computes dead-code liveness twice: a batch pipeline (Dead_type, Liveness, the frozen stores) and a reactive one (Reactive_type_deps, Reactive_liveness, ...). They are meant to agree. They do not, and the way they drifted is worth recording, because the same thing nearly happened again last week.
Which path do users get?
Not the one CI tests.
(* reanalyze_server.ml:309 *)
(* Editor mode only: the server always behaves like `reanalyze -json`. *)
Cli.reactive := true; (* Force reactive mode in server. *)
Cli.reactive defaults to false (cli.ml:29), so:
| who | mode |
|---|---|
editor, via rescript-tools reanalyze-server |
reactive |
rescript-tools reanalyze on the CLI |
batch |
| CI | batch |
ci.yml:201 is the only place the suite runs: make -C tests/analysis_tests test, which reaches tests-reanalyze/deadcode/test.sh — batch, no -reactive. The two targets that would touch reactive, test-reactive-server and test-reanalyze-order-independence, are not default targets and appear in no workflow; their Makefile says "not run by default (takes longer)". So interactive users run the untested path.
The bug
On master, on the checked-in corpus:
batch 405 issues
reactive 412 issues
All 7 extra reactive findings are false positives on type re-export equations (type y = x = {...}):
TypeReexport.res:27:5-21 UseReexported.originalType.usedField is a record label never used to read a value
TypeReexport.res:48:5-24 UseOriginal.originalType.directlyUsed is a record label never used to read a value
TypeReexport.res:70:5-21 OnlyReexportedDead.originalType.usedField is a record label never used to read a value
TypeReexport.res:91:23 VariantUseReexported.originalType.A is a variant case which is never constructed
TypeReexport.res:109:23 VariantUseOriginal.originalType.A is a variant case which is never constructed
TypeReexportCrossFileA.res:5:3-19 originalRecord.usedField is a record label never used to read a value
TypeReexportCrossFileA.res:0:1 TypeReexportCrossFileA is a dead module as all its items are dead.
Every one of those labels is read, through the re-exporting type. The fixture states the expected behaviour in its own comments:
// Expected: originalType.usedField = LIVE (propagated)
// originalType.unusedField = DEAD
// warnings: only originalType.unusedField should be reported
Reactive says the opposite, and reports a whole module dead. This is what the editor shows.
Cause, confirmed by A/B
Disable only the manifest/re-export block at the end of Dead_type.process_type_label_dependencies and re-run the batch pass:
batch without manifest linking 412 issues
does batch-without-manifest == reactive ? True
reactive-only vs no-manifest: []
no-manifest-only vs reactive: []
Set-identical in both directions. The reactive pipeline is missing precisely that linking and nothing else.
How it got there
| date | ||
|---|---|---|
| 2025-12-28 | #8092 | reactive pipeline lands, incl. ReactiveTypeDeps.ml |
| 2026-01-09 | #8127 | reanalyze-server lands; the editor starts using reactive |
| 2026-01-23 | #8217 | re-export linking added to DeadType.ml only |
ReactiveTypeDeps.ml already existed when #8217 was written — it is in the tree at that commit, with twelve sibling Reactive* files. #8217 touched DeadType.ml, DeadValue.ml, DeadCommon.ml, Decl.ml, three new fixtures and expected/deadcode.txt, and no reactive file. The feature was one-sided from birth and has been for ~7 months.
Nothing caught it because the same commit added the fixture and regenerated expected/deadcode.txt in batch mode. The fixture that exists to pin this behaviour passes, in the mode nobody ships.
It just happened again
#8643 (record labels read through a coercion reported dead) has the same shape: a label reached through an aliasing relationship is not credited to the original declaration. The fix in #8645 was written against the batch pass first and would have shipped batch-only — the reactive half was added only because a reviewer asked whether the editor path was covered. Without that question this issue would have had a second entry.
Smaller things noticed nearby
Reactive_type_deps.all_type_refsis onlysame_path_refs, soadd_to_refs_builderwould drop the impl↔intf refs thatall_type_refs_fromincludes. Harmless today: bothadd_to_refs_builderhelpers are dead code, never called.
What would actually prevent the next one
- CI checks the two agree. Run the corpus in both modes, diff the finding sets. Cheap, and it would have failed the day #8217 landed. It fails today for the 7 above, so it lands with the re-export fix.
- Delete the duplication. The genuinely mode-specific part is narrow — how declarations are indexed and how an edge is recorded. The rules themselves (which declarations participate, how they pair, which direction the edge runs) can be shared; #8645 does this for coercions with
Dead_type.pair_coercion_labels/record_label_of_decl. Applied to re-export linking too, adding a rule to one pass and not the other stops being possible. - Or keep one path. If reactive is what ships, batch is a reference implementation that no user runs and that CI mistakes for the product. Retiring it removes the failure mode outright — but it wants (1) first, so the two are known to agree before one is deleted.
- Vorherrschende Sprache
- OCaml
- Sterne
- 7.5k
- Forks
- 484
- Ø Merge
- 20 Std. 27 Min.
- Gemergte PRs (30 T.)
- 120
Entwicklungsumgebung
Startet den Dev-Container des Projekts im Browser, mit Ihrem eigenen GitHub-Konto.
- Kein Dockerfile und keine Docker-Compose-Datei
- Keine Pull-Request-Vorlage
- Beitragsleitfaden lesen
Erste Schritte
- Lesen Sie das ganze Issue und danach den Beitragsleitfaden des Projekts.
- Schreiben Sie ins Issue, dass Sie es übernehmen — das erspart doppelte Arbeit.
- Forken Sie das Repository und arbeiten Sie in einem Branch.
- Öffnen Sie einen Pull Request, der die Issue-Nummer nennt.
Mehr aus rescript-lang/rescript
-
Schwierigkeit 5/5 Über eine Woche Anfängerfreundlichkeit 25/100
rescript-lang/rescript#8833 · 1 Reaktion ·
Maintainer antworten meist innerhalb von 1 Tag
-
rewatch watch on Linux stops watching a source folder when an overlapping recursive source folder is removed from rescript.jsonEvtl. vergeben @cknitt hat das vor 2 Tagen übernommen. Offen
Schwierigkeit 4/5 1-2 Tage Anfängerfreundlichkeit 35/100
rescript-lang/rescript#8825 ·
Maintainer antworten meist innerhalb von 1 Tag
-
Schwierigkeit 5/5 Über eine Woche Anfängerfreundlichkeit 35/100
rescript-lang/rescript#8806 ·
Maintainer antworten meist innerhalb von 1 Tag
-
Schwierigkeit 5/5 Über eine Woche Anfängerfreundlichkeit 35/100
rescript-lang/rescript#8798 ·
Maintainer antworten meist innerhalb von 1 Tag
-
Schwierigkeit 3/5 Ein halber Tag Anfängerfreundlichkeit 58/100
rescript-lang/rescript#8757 ·
Maintainer antworten meist innerhalb von 1 Tag
Alle Issues in rescript-lang/rescript
Ähnliche Issues
-
Schwierigkeit 2/5 1-3 Stunden Anfängerfreundlichkeit 70/100
-
Schwierigkeit 2/5 1-3 Stunden Anfängerfreundlichkeit 62/100
NationalSecurityAgency/ghidra#9884 ·
Maintainer antworten meist innerhalb von 2 Tagen
-
llvm.memcpy with destination align but no source align asserts "Missed Source alignment!"Evtl. vergeben @pvelesko hat das heute übernommen. Offen
Schwierigkeit 2/5 1-3 Stunden Anfängerfreundlichkeit 72/100
KhronosGroup/SPIRV-LLVM-Translator#4093 ·
Maintainer antworten meist innerhalb von 2 Tagen
-
Schwierigkeit 2/5 1-3 Stunden Anfängerfreundlichkeit 72/100
FStarLang/karamel#784 · 1 Kommentar ·
Maintainer antworten meist innerhalb von 1 Tag
-
UB: (int) cast of a user double outside int range in dispatch / vm_index_is_int / flat_index_atOffenarea:runtime-vm
Schwierigkeit 2/5 1-3 Stunden Anfängerfreundlichkeit 72/100
InauguralSystems/EigenScript#1748 ·
Maintainer antworten meist innerhalb von 1 Tag