Skip to content

Commit 7936d69

Browse files
committed
DCE: Remove global file context, thread explicit file_context
Task 1 of the dead code refactor plan: eliminate global mutable state for current file context. Changes: - Add DeadCommon.FileContext.t with source_path, module_name, is_interface - Thread ~file parameter through DeadCode, DeadValue, DeadType, DeadCommon - Thread ~file through Exception.processCmt and Arnold.processCmt - Remove Common.currentSrc, currentModule, currentModuleName globals Design improvement: - FileContext.module_name is now a raw string (e.g. "ExnB"), not Name.t - Added FileContext.module_name_tagged helper to create Name.t when needed - This avoids confusion: raw name for hashtable keys, tagged name for paths - Previously the interface encoding (+prefix) leaked into code that expected raw names This makes it possible to process files concurrently or out of order, as analysis no longer depends on hidden global state for file context.
1 parent 9ba5d2d commit 7936d69

File tree

13 files changed

+134
-108
lines changed

13 files changed

+134
-108
lines changed

analysis/reanalyze/DEADCODE_REFACTOR_PLAN.md

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@
2121

2222
**Used by**: `DeadCommon.addDeclaration_`, `DeadType.addTypeDependenciesAcrossFiles`, `DeadValue` path construction.
2323

24+
**Status**: ✅ FIXED in Task 1 - explicit `file_context` now threaded through all analysis functions.
25+
2426
### P2: Global analysis tables
2527
**Problem**: All analysis results accumulate in global hashtables:
2628
- `DeadCommon.decls` - all declarations
@@ -42,6 +44,8 @@
4244
### P4: Global configuration reads
4345
**Problem**: Analysis code directly reads `!Common.Cli.debug`, `RunConfig.runConfig.transitive`, etc. scattered throughout. Can't run analysis with different configs without mutating globals.
4446

47+
**Status**: ✅ FIXED in Task 2 - explicit `config` now threaded through all analysis functions.
48+
4549
### P5: Side effects mixed with analysis
4650
**Problem**: Analysis functions directly call:
4751
- `Log_.warning` - logging
@@ -135,10 +139,13 @@ Each task should:
135139
**Value**: Makes it possible to process files concurrently or out of order.
136140

137141
**Changes**:
138-
- [ ] Create `DeadFileContext.t` type with `source_path`, `module_name`, `is_interface` fields
139-
- [ ] Thread through `DeadCode.processCmt`, `DeadValue`, `DeadType`, `DeadCommon.addDeclaration_`
140-
- [ ] Remove all reads of `Common.currentSrc`, `currentModule`, `currentModuleName` from DCE code
141-
- [ ] Delete the globals (or mark as deprecated if still used by Exception/Arnold)
142+
- [x] Create `DeadCommon.FileContext.t` type with `source_path`, `module_name`, `is_interface` fields
143+
- [x] Thread through `DeadCode.processCmt`, `DeadValue`, `DeadType`, `DeadCommon.addDeclaration_`
144+
- [x] Thread through `Exception.processCmt`, `Arnold.processCmt`
145+
- [x] Remove all reads of `Common.currentSrc`, `currentModule`, `currentModuleName` from DCE code
146+
- [x] Delete the globals `currentSrc`, `currentModule`, `currentModuleName` from `Common.ml`
147+
148+
**Status**: Complete ✅
142149

143150
**Test**: Run analysis on same files but vary the order - should get identical results.
144151

@@ -288,14 +295,15 @@ Each task should:
288295

289296
## Execution Strategy
290297

291-
**Recommended order**: 1 → 2 (complete all analyses) → 3 → 4 → 5 → 6 → 7 → 8 → 9 → 10 (verify) → 11 (test)
298+
**Completed**: Task 1 ✅, Task 2 ✅, Task 10 ✅
299+
300+
**Remaining order**: 3 → 4 → 5 → 6 → 7 → 8 → 9 → 11 (test)
292301

293302
**Why this order?**
294-
- Tasks 1-2 remove implicit dependencies (file context, config) - these are foundational
295-
- Task 2 must be **fully complete** (DCE + Exception + Arnold) before proceeding
296-
- Tasks 3-7 localize global state - can be done incrementally once inputs are explicit
303+
- Tasks 1-2 remove implicit dependencies (file context, config) - ✅ DONE
304+
- Tasks 3-7 localize global state - can be done incrementally now that inputs are explicit
297305
- Tasks 8-9 separate pure/impure - can only do this once state is local
298-
- Task 10 verifies no global config reads remain
306+
- Task 10 verifies no global config reads remain - ✅ DONE
299307
- Task 11 validates everything
300308

301309
**Alternative**: Could do 3-7 in any order (they're mostly independent).

analysis/reanalyze/src/Arnold.ml

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ module Stats = struct
111111
incr nCacheChecks;
112112
if hit then incr nCacheHits;
113113
if config.DceConfig.cli.debug then
114-
Log_.warning ~config ~forStats:false ~loc
114+
Log_.warning ~forStats:false ~loc
115115
(Termination
116116
{
117117
termination = TerminationAnalysisInternal;
@@ -125,7 +125,7 @@ module Stats = struct
125125

126126
let logResult ~config ~functionCall ~loc ~resString =
127127
if config.DceConfig.cli.debug then
128-
Log_.warning ~config ~forStats:false ~loc
128+
Log_.warning ~forStats:false ~loc
129129
(Termination
130130
{
131131
termination = TerminationAnalysisInternal;
@@ -611,7 +611,7 @@ module ExtendFunctionTable = struct
611611
then (
612612
functionTable |> FunctionTable.addFunction ~functionName;
613613
if config.DceConfig.cli.debug then
614-
Log_.warning ~config ~forStats:false ~loc
614+
Log_.warning ~forStats:false ~loc
615615
(Termination
616616
{
617617
termination = TerminationAnalysisInternal;
@@ -633,7 +633,7 @@ module ExtendFunctionTable = struct
633633
functionTable
634634
|> FunctionTable.addLabelToKind ~functionName ~label;
635635
if config.DceConfig.cli.debug then
636-
Log_.warning ~config ~forStats:false ~loc
636+
Log_.warning ~forStats:false ~loc
637637
(Termination
638638
{
639639
termination = TerminationAnalysisInternal;
@@ -700,7 +700,7 @@ module CheckExpressionWellFormed = struct
700700
functionTable
701701
|> FunctionTable.addLabelToKind ~functionName ~label;
702702
if config.DceConfig.cli.debug then
703-
Log_.warning ~config ~forStats:false ~loc:body.exp_loc
703+
Log_.warning ~forStats:false ~loc:body.exp_loc
704704
(Termination
705705
{
706706
termination = TerminationAnalysisInternal;
@@ -879,7 +879,7 @@ module Compile = struct
879879
newFunctionName;
880880
newFunctionDefinition.body <- Some (vb_expr |> expression ~ctx:newCtx);
881881
if config.DceConfig.cli.debug then
882-
Log_.warning ~config ~forStats:false ~loc:pat_loc
882+
Log_.warning ~forStats:false ~loc:pat_loc
883883
(Termination
884884
{
885885
termination = TerminationAnalysisInternal;
@@ -1410,7 +1410,7 @@ let processStructure ~config (structure : Typedtree.structure) =
14101410
let traverseAst = traverseAst ~config ~valueBindingsTable in
14111411
structure |> traverseAst.structure traverseAst |> ignore
14121412

1413-
let processCmt ~config (cmt_infos : Cmt_format.cmt_infos) =
1413+
let processCmt ~config ~file:_ (cmt_infos : Cmt_format.cmt_infos) =
14141414
match cmt_infos.cmt_annots with
14151415
| Interface _ -> ()
14161416
| Implementation structure -> processStructure ~config structure

analysis/reanalyze/src/Common.ml

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,3 @@
1-
let currentSrc = ref ""
2-
let currentModule = ref ""
3-
let currentModuleName = ref ("" |> Name.create)
41
let runConfig = RunConfig.runConfig
52

63
(* Location printer: `filename:line: ' *)

analysis/reanalyze/src/DeadCode.ml

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,32 +1,35 @@
11
open DeadCommon
22

3-
let processSignature ~config ~doValues ~doTypes (signature : Types.signature) =
3+
let processSignature ~config ~file ~doValues ~doTypes
4+
(signature : Types.signature) =
45
signature
56
|> List.iter (fun sig_item ->
6-
DeadValue.processSignatureItem ~config ~doValues ~doTypes
7+
DeadValue.processSignatureItem ~config ~file ~doValues ~doTypes
78
~moduleLoc:Location.none
8-
~path:[!Common.currentModuleName]
9+
~path:[FileContext.module_name_tagged file]
910
sig_item)
1011

11-
let processCmt ~config ~cmtFilePath (cmt_infos : Cmt_format.cmt_infos) =
12+
let processCmt ~config ~file ~cmtFilePath (cmt_infos : Cmt_format.cmt_infos) =
1213
(match cmt_infos.cmt_annots with
1314
| Interface signature ->
1415
ProcessDeadAnnotations.signature ~config signature;
15-
processSignature ~config ~doValues:true ~doTypes:true signature.sig_type
16+
processSignature ~config ~file ~doValues:true ~doTypes:true
17+
signature.sig_type
1618
| Implementation structure ->
1719
let cmtiExists =
1820
Sys.file_exists ((cmtFilePath |> Filename.remove_extension) ^ ".cmti")
1921
in
2022
ProcessDeadAnnotations.structure ~config ~doGenType:(not cmtiExists)
2123
structure;
22-
processSignature ~config ~doValues:true ~doTypes:false structure.str_type;
24+
processSignature ~config ~file ~doValues:true ~doTypes:false
25+
structure.str_type;
2326
let doExternals =
2427
(* This is already handled at the interface level, avoid issues in inconsistent locations
2528
https://github.com/BuckleScript/syntax/pull/54
2629
Ideally, the handling should be less location-based, just like other language aspects. *)
2730
false
2831
in
29-
DeadValue.processStructure ~config ~doTypes:true ~doExternals
32+
DeadValue.processStructure ~config ~file ~doTypes:true ~doExternals
3033
~cmt_value_dependencies:cmt_infos.cmt_value_dependencies structure
3134
| _ -> ());
3235
DeadType.TypeDependencies.forceDelayedItems ~config;

analysis/reanalyze/src/DeadCommon.ml

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,11 @@
1+
module FileContext = struct
2+
type t = {source_path: string; module_name: string; is_interface: bool}
3+
4+
(** Get module name as Name.t tagged with interface/implementation info *)
5+
let module_name_tagged file =
6+
file.module_name |> Name.create ~isInterface:file.is_interface
7+
end
8+
19
(* Adapted from https://github.com/LexiFi/dead_code_analyzer *)
210

311
open Common
@@ -170,7 +178,7 @@ let iterFilesFromRootsToLeaves ~config iterFun =
170178
{Location.none with loc_start = pos; loc_end = pos}
171179
in
172180
if Config.warnOnCircularDependencies then
173-
Log_.warning ~config ~loc
181+
Log_.warning ~loc
174182
(Circular
175183
{
176184
message =
@@ -355,8 +363,9 @@ module ProcessDeadAnnotations = struct
355363
|> ignore
356364
end
357365

358-
let addDeclaration_ ~config ?posEnd ?posStart ~declKind ~path
359-
~(loc : Location.t) ?(posAdjustment = Nothing) ~moduleLoc (name : Name.t) =
366+
let addDeclaration_ ~config ~(file : FileContext.t) ?posEnd ?posStart ~declKind
367+
~path ~(loc : Location.t) ?(posAdjustment = Nothing) ~moduleLoc
368+
(name : Name.t) =
360369
let pos = loc.loc_start in
361370
let posStart =
362371
match posStart with
@@ -373,10 +382,7 @@ let addDeclaration_ ~config ?posEnd ?posStart ~declKind ~path
373382
module M : Set.S with type elt = int
374383
will create value definitions whose location is in set.mli
375384
*)
376-
if
377-
(not loc.loc_ghost)
378-
&& (!currentSrc = pos.pos_fname || !currentModule == "*include*")
379-
then (
385+
if (not loc.loc_ghost) && pos.pos_fname = file.source_path then (
380386
if config.DceConfig.cli.debug then
381387
Log_.item "add%sDeclaration %s %s path:%s@."
382388
(declKind |> DeclKind.toString)
@@ -396,10 +402,10 @@ let addDeclaration_ ~config ?posEnd ?posStart ~declKind ~path
396402
in
397403
PosHash.replace decls pos decl)
398404

399-
let addValueDeclaration ~config ?(isToplevel = true) ~(loc : Location.t)
405+
let addValueDeclaration ~config ~file ?(isToplevel = true) ~(loc : Location.t)
400406
~moduleLoc ?(optionalArgs = OptionalArgs.empty) ~path ~sideEffects name =
401407
name
402-
|> addDeclaration_ ~config
408+
|> addDeclaration_ ~config ~file
403409
~declKind:(Value {isToplevel; optionalArgs; sideEffects})
404410
~loc ~moduleLoc ~path
405411

@@ -423,7 +429,7 @@ let emitWarning ~config ~decl ~message deadWarning =
423429
decl.path
424430
|> Path.toModuleName ~isType:(decl.declKind |> DeclKind.isType)
425431
|> DeadModules.checkModuleDead ~config ~fileName:decl.pos.pos_fname;
426-
Log_.warning ~config ~loc
432+
Log_.warning ~loc
427433
(DeadWarning
428434
{
429435
deadWarning;

analysis/reanalyze/src/DeadException.ml

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,13 @@ type item = {exceptionPath: Path.t; locFrom: Location.t}
66
let delayedItems = ref []
77
let declarations = Hashtbl.create 1
88

9-
let add ~config ~path ~loc ~(strLoc : Location.t) name =
9+
let add ~config ~file ~path ~loc ~(strLoc : Location.t) name =
1010
let exceptionPath = name :: path in
1111
Hashtbl.add declarations exceptionPath loc;
1212
name
13-
|> addDeclaration_ ~config ~posEnd:strLoc.loc_end ~posStart:strLoc.loc_start
14-
~declKind:Exception ~moduleLoc:(ModulePath.getCurrent ()).loc ~path ~loc
13+
|> addDeclaration_ ~config ~file ~posEnd:strLoc.loc_end
14+
~posStart:strLoc.loc_start ~declKind:Exception
15+
~moduleLoc:(ModulePath.getCurrent ()).loc ~path ~loc
1516

1617
let forceDelayedItems ~config =
1718
let items = !delayedItems |> List.rev in
@@ -22,7 +23,7 @@ let forceDelayedItems ~config =
2223
| None -> ()
2324
| Some locTo ->
2425
(* Delayed exception references don't need a binding context; use an empty state. *)
25-
DeadCommon.addValueReference ~config ~binding:Location.none
26+
addValueReference ~config ~binding:Location.none
2627
~addFileReference:true ~locFrom ~locTo)
2728

2829
let markAsUsed ~config ~(binding : Location.t) ~(locFrom : Location.t)
@@ -33,6 +34,4 @@ let markAsUsed ~config ~(binding : Location.t) ~(locFrom : Location.t)
3334
path_ |> Path.fromPathT |> Path.moduleToImplementation
3435
in
3536
delayedItems := {exceptionPath; locFrom} :: !delayedItems
36-
else
37-
DeadCommon.addValueReference ~config ~binding ~addFileReference:true
38-
~locFrom ~locTo
37+
else addValueReference ~config ~binding ~addFileReference:true ~locFrom ~locTo

analysis/reanalyze/src/DeadModules.ml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ let checkModuleDead ~config ~fileName:pos_fname moduleName =
3333
{Location.loc_start = pos; loc_end = pos; loc_ghost = false}
3434
else loc
3535
in
36-
Log_.warning ~config ~loc
36+
Log_.warning ~loc
3737
(Common.DeadModule
3838
{
3939
message =

analysis/reanalyze/src/DeadOptionalArgs.ml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -81,14 +81,14 @@ let forceDelayedItems () =
8181
OptionalArgs.combine rFrom.optionalArgs rTo.optionalArgs
8282
| _ -> ())
8383

84-
let check ~config decl =
84+
let check ~config:_ decl =
8585
match decl with
8686
| {declKind = Value {optionalArgs}}
8787
when active ()
8888
&& not (ProcessDeadAnnotations.isAnnotatedGenTypeOrLive decl.pos) ->
8989
optionalArgs
9090
|> OptionalArgs.iterUnused (fun s ->
91-
Log_.warning ~config ~loc:(decl |> declGetLoc)
91+
Log_.warning ~loc:(decl |> declGetLoc)
9292
(DeadOptional
9393
{
9494
deadOptional = WarningUnusedArgument;
@@ -101,7 +101,7 @@ let check ~config decl =
101101
}));
102102
optionalArgs
103103
|> OptionalArgs.iterAlwaysUsed (fun s nCalls ->
104-
Log_.warning ~config ~loc:(decl |> declGetLoc)
104+
Log_.warning ~loc:(decl |> declGetLoc)
105105
(DeadOptional
106106
{
107107
deadOptional = WarningRedundantOptionalArgument;

analysis/reanalyze/src/DeadType.ml

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,9 @@ let extendTypeDependencies ~config (loc1 : Location.t) (loc2 : Location.t) =
4141
TypeDependencies.add loc1 loc2)
4242

4343
(* Type dependencies between Foo.re and Foo.rei *)
44-
let addTypeDependenciesAcrossFiles ~config ~pathToType ~loc ~typeLabelName =
45-
let isInterface = Filename.check_suffix !Common.currentSrc "i" in
44+
let addTypeDependenciesAcrossFiles ~config ~file ~pathToType ~loc ~typeLabelName
45+
=
46+
let isInterface = file.FileContext.is_interface in
4647
if not isInterface then (
4748
let path_1 = pathToType |> Path.moduleToInterface in
4849
let path_2 = path_1 |> Path.typeToInterface in
@@ -80,17 +81,18 @@ let addTypeDependenciesInnerModule ~config ~pathToType ~loc ~typeLabelName =
8081
extendTypeDependencies ~config loc2 loc
8182
| None -> TypeLabels.add path loc
8283

83-
let addDeclaration ~config ~(typeId : Ident.t) ~(typeKind : Types.type_kind) =
84+
let addDeclaration ~config ~file ~(typeId : Ident.t)
85+
~(typeKind : Types.type_kind) =
8486
let currentModulePath = ModulePath.getCurrent () in
8587
let pathToType =
8688
(typeId |> Ident.name |> Name.create)
87-
:: (currentModulePath.path @ [!Common.currentModuleName])
89+
:: (currentModulePath.path @ [FileContext.module_name_tagged file])
8890
in
8991
let processTypeLabel ?(posAdjustment = Nothing) typeLabelName ~declKind
9092
~(loc : Location.t) =
91-
addDeclaration_ ~config ~declKind ~path:pathToType ~loc
93+
addDeclaration_ ~config ~file ~declKind ~path:pathToType ~loc
9294
~moduleLoc:currentModulePath.loc ~posAdjustment typeLabelName;
93-
addTypeDependenciesAcrossFiles ~config ~pathToType ~loc ~typeLabelName;
95+
addTypeDependenciesAcrossFiles ~config ~file ~pathToType ~loc ~typeLabelName;
9496
addTypeDependenciesInnerModule ~config ~pathToType ~loc ~typeLabelName;
9597
TypeLabels.add (typeLabelName :: pathToType) loc
9698
in

0 commit comments

Comments
 (0)