Skip to content

Commit 217c4fa

Browse files
committed
DCE: Implement map->list->merge pattern for source annotations
This refactors source annotations to use a clean architectural pattern: 1. **Two types**: FileAnnotations.builder (mutable) and FileAnnotations.t (immutable) 2. **Map phase**: process_cmt_file returns a builder per file 3. **Collect phase**: processCmtFiles collects builders into a list 4. **Merge phase**: merge_all combines all builders into immutable t 5. **Analyze phase**: solver receives immutable t (read-only) Key benefits: - Order independence: builders can be collected in any order - Parallelizable: map phase can run concurrently (future) - Incremental: replace one builder, re-merge (future) - Type-safe: t has no mutation functions in its API New files: - FileAnnotations.ml/.mli: builder/t types with clear API separation - DceFileProcessing.ml/.mli: AST processing returns builder Pattern documented in DEADCODE_REFACTOR_PLAN.md as reusable template for Tasks 4-7 (declarations, references, delayed items, file deps).
1 parent 093ef59 commit 217c4fa

File tree

9 files changed

+647
-436
lines changed

9 files changed

+647
-436
lines changed

analysis/reanalyze/DEADCODE_REFACTOR_PLAN.md

Lines changed: 245 additions & 182 deletions
Large diffs are not rendered by default.
Lines changed: 227 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,227 @@
1+
(** Per-file AST processing for dead code analysis.
2+
3+
This module uses FileAnnotations.builder during AST traversal
4+
and returns it for merging. The caller freezes it before
5+
passing to the solver. *)
6+
7+
open DeadCommon
8+
9+
(* ===== File context ===== *)
10+
11+
type file_context = {
12+
source_path: string;
13+
module_name: string;
14+
is_interface: bool;
15+
}
16+
17+
let module_name_tagged (file : file_context) =
18+
file.module_name |> Name.create ~isInterface:file.is_interface
19+
20+
(* ===== AST Processing (internal) ===== *)
21+
22+
module CollectAnnotations = struct
23+
let processAttributes ~state ~config ~doGenType ~name ~pos attributes =
24+
let getPayloadFun f = attributes |> Annotation.getAttributePayload f in
25+
let getPayload (x : string) =
26+
attributes |> Annotation.getAttributePayload (( = ) x)
27+
in
28+
if
29+
doGenType
30+
&& getPayloadFun Annotation.tagIsOneOfTheGenTypeAnnotations <> None
31+
then FileAnnotations.annotate_gentype state pos;
32+
if getPayload WriteDeadAnnotations.deadAnnotation <> None then
33+
FileAnnotations.annotate_dead state pos;
34+
let nameIsInLiveNamesOrPaths () =
35+
config.DceConfig.cli.live_names |> List.mem name
36+
||
37+
let fname =
38+
match Filename.is_relative pos.pos_fname with
39+
| true -> pos.pos_fname
40+
| false -> Filename.concat (Sys.getcwd ()) pos.pos_fname
41+
in
42+
let fnameLen = String.length fname in
43+
config.DceConfig.cli.live_paths
44+
|> List.exists (fun prefix ->
45+
String.length prefix <= fnameLen
46+
&&
47+
try String.sub fname 0 (String.length prefix) = prefix
48+
with Invalid_argument _ -> false)
49+
in
50+
if getPayload liveAnnotation <> None || nameIsInLiveNamesOrPaths () then
51+
FileAnnotations.annotate_live state pos;
52+
if attributes |> Annotation.isOcamlSuppressDeadWarning then
53+
FileAnnotations.annotate_live state pos
54+
55+
let collectExportLocations ~state ~config ~doGenType =
56+
let super = Tast_mapper.default in
57+
let currentlyDisableWarnings = ref false in
58+
let value_binding self
59+
({vb_attributes; vb_pat} as value_binding : Typedtree.value_binding) =
60+
(match vb_pat.pat_desc with
61+
| Tpat_var (id, {loc = {loc_start = pos}})
62+
| Tpat_alias ({pat_desc = Tpat_any}, id, {loc = {loc_start = pos}}) ->
63+
if !currentlyDisableWarnings then
64+
FileAnnotations.annotate_live state pos;
65+
vb_attributes
66+
|> processAttributes ~state ~config ~doGenType ~name:(id |> Ident.name)
67+
~pos
68+
| _ -> ());
69+
super.value_binding self value_binding
70+
in
71+
let type_kind toplevelAttrs self (typeKind : Typedtree.type_kind) =
72+
(match typeKind with
73+
| Ttype_record labelDeclarations ->
74+
labelDeclarations
75+
|> List.iter
76+
(fun ({ld_attributes; ld_loc} : Typedtree.label_declaration) ->
77+
toplevelAttrs @ ld_attributes
78+
|> processAttributes ~state ~config ~doGenType:false ~name:""
79+
~pos:ld_loc.loc_start)
80+
| Ttype_variant constructorDeclarations ->
81+
constructorDeclarations
82+
|> List.iter
83+
(fun
84+
({cd_attributes; cd_loc; cd_args} :
85+
Typedtree.constructor_declaration)
86+
->
87+
let _process_inline_records =
88+
match cd_args with
89+
| Cstr_record flds ->
90+
List.iter
91+
(fun ({ld_attributes; ld_loc} :
92+
Typedtree.label_declaration) ->
93+
toplevelAttrs @ cd_attributes @ ld_attributes
94+
|> processAttributes ~state ~config ~doGenType:false
95+
~name:"" ~pos:ld_loc.loc_start)
96+
flds
97+
| Cstr_tuple _ -> ()
98+
in
99+
toplevelAttrs @ cd_attributes
100+
|> processAttributes ~state ~config ~doGenType:false ~name:""
101+
~pos:cd_loc.loc_start)
102+
| _ -> ());
103+
super.type_kind self typeKind
104+
in
105+
let type_declaration self (typeDeclaration : Typedtree.type_declaration) =
106+
let attributes = typeDeclaration.typ_attributes in
107+
let _ = type_kind attributes self typeDeclaration.typ_kind in
108+
typeDeclaration
109+
in
110+
let value_description self
111+
({val_attributes; val_id; val_val = {val_loc = {loc_start = pos}}} as
112+
value_description :
113+
Typedtree.value_description) =
114+
if !currentlyDisableWarnings then FileAnnotations.annotate_live state pos;
115+
val_attributes
116+
|> processAttributes ~state ~config ~doGenType
117+
~name:(val_id |> Ident.name) ~pos;
118+
super.value_description self value_description
119+
in
120+
let structure_item self (item : Typedtree.structure_item) =
121+
(match item.str_desc with
122+
| Tstr_attribute attribute
123+
when [attribute] |> Annotation.isOcamlSuppressDeadWarning ->
124+
currentlyDisableWarnings := true
125+
| _ -> ());
126+
super.structure_item self item
127+
in
128+
let structure self (structure : Typedtree.structure) =
129+
let oldDisableWarnings = !currentlyDisableWarnings in
130+
super.structure self structure |> ignore;
131+
currentlyDisableWarnings := oldDisableWarnings;
132+
structure
133+
in
134+
let signature_item self (item : Typedtree.signature_item) =
135+
(match item.sig_desc with
136+
| Tsig_attribute attribute
137+
when [attribute] |> Annotation.isOcamlSuppressDeadWarning ->
138+
currentlyDisableWarnings := true
139+
| _ -> ());
140+
super.signature_item self item
141+
in
142+
let signature self (signature : Typedtree.signature) =
143+
let oldDisableWarnings = !currentlyDisableWarnings in
144+
super.signature self signature |> ignore;
145+
currentlyDisableWarnings := oldDisableWarnings;
146+
signature
147+
in
148+
{
149+
super with
150+
signature;
151+
signature_item;
152+
structure;
153+
structure_item;
154+
type_declaration;
155+
value_binding;
156+
value_description;
157+
}
158+
159+
let structure ~state ~config ~doGenType structure =
160+
let collectExportLocations =
161+
collectExportLocations ~state ~config ~doGenType
162+
in
163+
structure
164+
|> collectExportLocations.structure collectExportLocations
165+
|> ignore
166+
167+
let signature ~state ~config signature =
168+
let collectExportLocations =
169+
collectExportLocations ~state ~config ~doGenType:true
170+
in
171+
signature
172+
|> collectExportLocations.signature collectExportLocations
173+
|> ignore
174+
end
175+
176+
let processSignature ~config ~(file : file_context) ~doValues ~doTypes
177+
(signature : Types.signature) =
178+
let dead_common_file : FileContext.t =
179+
{
180+
source_path = file.source_path;
181+
module_name = file.module_name;
182+
is_interface = file.is_interface;
183+
}
184+
in
185+
signature
186+
|> List.iter (fun sig_item ->
187+
DeadValue.processSignatureItem ~config ~file:dead_common_file ~doValues
188+
~doTypes ~moduleLoc:Location.none
189+
~path:[module_name_tagged file]
190+
sig_item)
191+
192+
(* ===== Main entry point ===== *)
193+
194+
let process_cmt_file ~config ~(file : file_context) ~cmtFilePath
195+
(cmt_infos : Cmt_format.cmt_infos) : FileAnnotations.builder =
196+
(* Convert to DeadCommon.FileContext for functions that need it *)
197+
let dead_common_file : FileContext.t =
198+
{
199+
source_path = file.source_path;
200+
module_name = file.module_name;
201+
is_interface = file.is_interface;
202+
}
203+
in
204+
(* Mutable builder for AST processing *)
205+
let builder = FileAnnotations.create_builder () in
206+
(match cmt_infos.cmt_annots with
207+
| Interface signature ->
208+
CollectAnnotations.signature ~state:builder ~config signature;
209+
processSignature ~config ~file ~doValues:true ~doTypes:true
210+
signature.sig_type
211+
| Implementation structure ->
212+
let cmtiExists =
213+
Sys.file_exists ((cmtFilePath |> Filename.remove_extension) ^ ".cmti")
214+
in
215+
CollectAnnotations.structure ~state:builder ~config
216+
~doGenType:(not cmtiExists) structure;
217+
processSignature ~config ~file ~doValues:true ~doTypes:false
218+
structure.str_type;
219+
let doExternals = false in
220+
DeadValue.processStructure ~config ~file:dead_common_file ~doTypes:true
221+
~doExternals ~cmt_value_dependencies:cmt_infos.cmt_value_dependencies
222+
structure
223+
| _ -> ());
224+
DeadType.TypeDependencies.forceDelayedItems ~config;
225+
DeadType.TypeDependencies.clear ();
226+
(* Return builder - caller will merge and freeze *)
227+
builder
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
(** Per-file AST processing for dead code analysis.
2+
3+
This module uses [FileAnnotations.builder] during AST traversal
4+
and returns it for merging. The caller freezes the accumulated
5+
builder before passing to the solver. *)
6+
7+
type file_context = {
8+
source_path: string;
9+
module_name: string;
10+
is_interface: bool;
11+
}
12+
(** File context for processing *)
13+
14+
val process_cmt_file :
15+
config:DceConfig.t ->
16+
file:file_context ->
17+
cmtFilePath:string ->
18+
Cmt_format.cmt_infos ->
19+
FileAnnotations.builder
20+
(** Process a cmt file and return mutable builder.
21+
Caller should merge builders and freeze before passing to solver. *)

analysis/reanalyze/src/DeadCode.ml

Lines changed: 3 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1,37 +1,4 @@
1-
open DeadCommon
1+
(** Dead code analysis - cmt file processing.
2+
Delegates to DceFileProcessing for AST traversal. *)
23

3-
let processSignature ~config ~file ~doValues ~doTypes
4-
(signature : Types.signature) =
5-
signature
6-
|> List.iter (fun sig_item ->
7-
DeadValue.processSignatureItem ~config ~file ~doValues ~doTypes
8-
~moduleLoc:Location.none
9-
~path:[FileContext.module_name_tagged file]
10-
sig_item)
11-
12-
let processCmt ~state ~config ~file ~cmtFilePath
13-
(cmt_infos : Cmt_format.cmt_infos) =
14-
(match cmt_infos.cmt_annots with
15-
| Interface signature ->
16-
ProcessDeadAnnotations.signature ~state ~config signature;
17-
processSignature ~config ~file ~doValues:true ~doTypes:true
18-
signature.sig_type
19-
| Implementation structure ->
20-
let cmtiExists =
21-
Sys.file_exists ((cmtFilePath |> Filename.remove_extension) ^ ".cmti")
22-
in
23-
ProcessDeadAnnotations.structure ~state ~config ~doGenType:(not cmtiExists)
24-
structure;
25-
processSignature ~config ~file ~doValues:true ~doTypes:false
26-
structure.str_type;
27-
let doExternals =
28-
(* This is already handled at the interface level, avoid issues in inconsistent locations
29-
https://github.com/BuckleScript/syntax/pull/54
30-
Ideally, the handling should be less location-based, just like other language aspects. *)
31-
false
32-
in
33-
DeadValue.processStructure ~config ~file ~doTypes:true ~doExternals
34-
~cmt_value_dependencies:cmt_infos.cmt_value_dependencies structure
35-
| _ -> ());
36-
DeadType.TypeDependencies.forceDelayedItems ~config;
37-
DeadType.TypeDependencies.clear ()
4+
let processCmt = DceFileProcessing.process_cmt_file

0 commit comments

Comments
 (0)