Skip to content

Commit d21b1fb

Browse files
committed
DCE: Remove WriteDeadAnnotations feature
The `-write` flag that auto-inserted `@dead` annotations into source files was removed as it added significant complexity for a rarely-used feature. ## Deleted - `WriteDeadAnnotations.ml` (156 lines) - `Common.Cli.write` ref - `DceConfig.cli.write` field - `type line` and `type lineAnnotation` in Common.ml - `shouldWriteLineAnnotation` and `lineAnnotation` fields in DeadWarning - `-write` CLI argument - `~config` parameter from `logAdditionalInfo` (now unused) ## Simplified - `emitWarning` no longer computes line annotations - `logAdditionalInfo` no longer needs config parameter - DeadWarning type now just has: deadWarning, path, message ## Rationale The feature: - Added file I/O during analysis (violated pure analysis principles) - Maintained global state (currentFile, currentFileLines refs) - Required threading lineAnnotation through warning system - Was rarely used (most users want to delete dead code, not annotate it) Users who want to suppress warnings can still manually add `@dead` annotations.
1 parent feb98a0 commit d21b1fb

File tree

10 files changed

+23
-745
lines changed

10 files changed

+23
-745
lines changed

analysis/reanalyze/DEADCODE_REFACTOR_PLAN.md

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ you can swap one file's data without affecting others.
100100
**Problem**: Analysis functions directly call:
101101
- `Log_.warning` - logging
102102
- `EmitJson` - JSON output
103-
- `WriteDeadAnnotations` - file I/O
103+
- ~~`WriteDeadAnnotations` - file I/O~~ (removed - added complexity with little value)
104104
- Direct mutation of result data structures
105105

106106
**Impact**: Can't get analysis results as data. Can't test without capturing I/O. Can't reuse analysis logic for different output formats.
@@ -440,19 +440,14 @@ This enables parallelization, caching, and incremental recomputation.
440440

441441
**Estimated effort**: Medium (many logging call sites, but mechanical)
442442

443-
### Task 9: Separate annotation computation from file writing (P5)
443+
### Task 9: ~~Separate annotation computation from file writing (P5)~~ REMOVED
444444

445-
**Value**: Can compute what to write without actually writing. Testable.
445+
**Status**: Removed ✅ - `WriteDeadAnnotations` feature was deleted entirely.
446446

447-
**Changes**:
448-
- [ ] `WriteDeadAnnotations`: Split into pure `compute_annotations` and impure `write_to_files`
449-
- [ ] Pure function takes deadness results, returns `(filepath * line_annotation list) list`
450-
- [ ] Impure function takes that list and does file I/O
451-
- [ ] Remove file I/O from analysis path
452-
453-
**Test**: Compute annotations, verify correct without touching filesystem.
454-
455-
**Estimated effort**: Small (single module)
447+
The `-write` flag that auto-inserted `@dead` annotations into source files was removed
448+
as it added significant complexity (global state, file I/O during analysis, extra types)
449+
for a rarely-used feature. Users who want to suppress dead code warnings can manually
450+
add `@dead` annotations.
456451

457452
### Task 10: Verify zero `DceConfig.current()` calls in analysis code
458453

analysis/reanalyze/src/CollectAnnotations.ml

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,7 @@ let processAttributes ~state ~config ~doGenType ~name ~pos attributes =
1414
doGenType
1515
&& getPayloadFun Annotation.tagIsOneOfTheGenTypeAnnotations <> None
1616
then FileAnnotations.annotate_gentype state pos;
17-
if getPayload WriteDeadAnnotations.deadAnnotation <> None then
18-
FileAnnotations.annotate_dead state pos;
17+
if getPayload "dead" <> None then FileAnnotations.annotate_dead state pos;
1918
let nameIsInLiveNamesOrPaths () =
2019
config.DceConfig.cli.live_names |> List.mem name
2120
||

analysis/reanalyze/src/Common.ml

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ module Cli = struct
1717

1818
let experimental = ref false
1919
let json = ref false
20-
let write = ref false
2120

2221
(* names to be considered live values *)
2322
let liveNames = ref ([] : string list)
@@ -174,8 +173,6 @@ type decl = {
174173
mutable report: bool;
175174
}
176175

177-
type line = {mutable declarations: decl list; original: string}
178-
179176
module ExnSet = Set.Make (Exn)
180177

181178
type missingThrowInfo = {
@@ -202,21 +199,13 @@ type deadWarning =
202199
| WarningDeadValueWithSideEffects
203200
| IncorrectDeadAnnotation
204201

205-
type lineAnnotation = (decl * line) option
206-
207202
type description =
208203
| Circular of {message: string}
209204
| ExceptionAnalysis of {message: string}
210205
| ExceptionAnalysisMissing of missingThrowInfo
211206
| DeadModule of {message: string}
212207
| DeadOptional of {deadOptional: deadOptional; message: string}
213-
| DeadWarning of {
214-
deadWarning: deadWarning;
215-
path: string;
216-
message: string;
217-
shouldWriteLineAnnotation: bool;
218-
lineAnnotation: lineAnnotation;
219-
}
208+
| DeadWarning of {deadWarning: deadWarning; path: string; message: string}
220209
| Termination of {termination: termination; message: string}
221210

222211
type issue = {

analysis/reanalyze/src/DceConfig.ml

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ type cli_config = {
77
debug: bool;
88
ci: bool;
99
json: bool;
10-
write: bool;
1110
live_names: string list;
1211
live_paths: string list;
1312
exclude_paths: string list;
@@ -25,7 +24,6 @@ let current () =
2524
debug = !Common.Cli.debug;
2625
ci = !Common.Cli.ci;
2726
json = !Common.Cli.json;
28-
write = !Common.Cli.write;
2927
live_names = !Common.Cli.liveNames;
3028
live_paths = !Common.Cli.livePaths;
3129
exclude_paths = !Common.Cli.excludePaths;

analysis/reanalyze/src/DeadCommon.ml

Lines changed: 4 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,9 @@ end
7878
let declGetLoc decl =
7979
let loc_start =
8080
let offset =
81-
WriteDeadAnnotations.offsetOfPosAdjustment decl.posAdjustment
81+
match decl.posAdjustment with
82+
| FirstVariant | Nothing -> 0
83+
| OtherVariant -> 2
8284
in
8385
let cnumWithOffset = decl.posStart.pos_cnum + offset in
8486
if cnumWithOffset < decl.posEnd.pos_cnum then
@@ -159,33 +161,11 @@ let addValueDeclaration ~config ~decls ~file ?(isToplevel = true)
159161

160162
let emitWarning ~config ~decl ~message deadWarning =
161163
let loc = decl |> declGetLoc in
162-
let isToplevelValueWithSideEffects decl =
163-
match decl.declKind with
164-
| Value {isToplevel; sideEffects} -> isToplevel && sideEffects
165-
| _ -> false
166-
in
167-
let shouldWriteLineAnnotation =
168-
(not (isToplevelValueWithSideEffects decl))
169-
&& Suppress.filter decl.pos
170-
&& deadWarning <> IncorrectDeadAnnotation
171-
in
172-
let lineAnnotation =
173-
if shouldWriteLineAnnotation then
174-
WriteDeadAnnotations.addLineAnnotation ~config ~decl
175-
else None
176-
in
177164
decl.path
178165
|> Path.toModuleName ~isType:(decl.declKind |> DeclKind.isType)
179166
|> DeadModules.checkModuleDead ~config ~fileName:decl.pos.pos_fname;
180167
Log_.warning ~loc
181-
(DeadWarning
182-
{
183-
deadWarning;
184-
path = Path.withoutHead decl.path;
185-
message;
186-
lineAnnotation;
187-
shouldWriteLineAnnotation;
188-
})
168+
(DeadWarning {deadWarning; path = Path.withoutHead decl.path; message})
189169

190170
module Decl = struct
191171
let isValue decl =

analysis/reanalyze/src/DeadType.ml

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -119,8 +119,12 @@ let addDeclaration ~config ~decls ~file ~(typeId : Ident.t)
119119
in
120120
let posAdjustment =
121121
(* In Res the variant loc can include the | and spaces after it *)
122-
if WriteDeadAnnotations.posLanguage cd_loc.loc_start = Res then
123-
if i = 0 then FirstVariant else OtherVariant
122+
let isRes =
123+
let fname = cd_loc.loc_start.pos_fname in
124+
Filename.check_suffix fname ".res"
125+
|| Filename.check_suffix fname ".resi"
126+
in
127+
if isRes then if i = 0 then FirstVariant else OtherVariant
124128
else Nothing
125129
in
126130
Ident.name cd_id |> Name.create

analysis/reanalyze/src/Log_.ml

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -107,12 +107,8 @@ let missingRaiseInfoToText {missingAnnotations; locFull} =
107107
~text:(Format.asprintf "@throws(%s)\\n" missingTxt)
108108
else ""
109109

110-
let logAdditionalInfo ~config ~(description : description) =
110+
let logAdditionalInfo ~(description : description) =
111111
match description with
112-
| DeadWarning {lineAnnotation; shouldWriteLineAnnotation} ->
113-
if shouldWriteLineAnnotation then
114-
WriteDeadAnnotations.lineAnnotationToString ~config lineAnnotation
115-
else ""
116112
| ExceptionAnalysisMissing missingRaiseInfo ->
117113
missingRaiseInfoToText missingRaiseInfo
118114
| _ -> ""
@@ -187,7 +183,7 @@ let logIssue ~config ~(issue : issue) =
187183
~range:(startLine, startCharacter, endLine, endCharacter)
188184
~message)
189185
()
190-
(logAdditionalInfo ~config ~description:issue.description)
186+
(logAdditionalInfo ~description:issue.description)
191187
(if config.DceConfig.cli.json then EmitJson.emitClose () else "")
192188
else
193189
let color =
@@ -197,7 +193,7 @@ let logIssue ~config ~(issue : issue) =
197193
in
198194
asprintf "@. %a@. %a@. %s%s@." color issue.name Loc.print issue.loc
199195
(descriptionToMessage issue.description)
200-
(logAdditionalInfo ~config ~description:issue.description)
196+
(logAdditionalInfo ~description:issue.description)
201197

202198
module Stats = struct
203199
let issues = ref []

analysis/reanalyze/src/Reanalyze.ml

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -153,8 +153,7 @@ let runAnalysis ~dce_config ~cmtRoot =
153153
let refs = References.freeze_builder refs_builder in
154154
let file_deps = FileDeps.freeze_builder file_deps_builder in
155155
DeadCommon.reportDead ~annotations ~decls ~refs ~file_deps
156-
~config:dce_config ~checkOptionalArg:DeadOptionalArgs.check;
157-
WriteDeadAnnotations.write ~config:dce_config);
156+
~config:dce_config ~checkOptionalArg:DeadOptionalArgs.check);
158157
if dce_config.DceConfig.run.exception_ then
159158
Exception.Checks.doChecks ~config:dce_config;
160159
if dce_config.DceConfig.run.termination && dce_config.DceConfig.cli.debug then
@@ -271,9 +270,6 @@ let cli () =
271270
specified)" );
272271
("-version", Unit versionAndExit, "Show version information and exit");
273272
("--version", Unit versionAndExit, "Show version information and exit");
274-
( "-write",
275-
Set Common.Cli.write,
276-
"Write @dead annotations directly in the source files" );
277273
]
278274
in
279275
Arg.parse speclist print_endline usage;

analysis/reanalyze/src/WriteDeadAnnotations.ml

Lines changed: 0 additions & 155 deletions
This file was deleted.

0 commit comments

Comments
 (0)