Warning 32 ought to be respected about reanalyze

[chenglou]

Hey Josh. Unrelated but glad to see you again =)

[cristianoc]

So for now it seems that indeed treating 42 as @live is the way forward.

[cristianoc]

I’d have first to make it slower, to give an excuse.

[cristianoc]

I guess this falls in the category: it is dead, it should be considered dead, but should not be reported as dead.
Not sure I'd want to consider global options, only these local @@@ocaml.warning "-32". Which makes it an ad-hoc treatment. Also, not clear how to get hold of global options anyway. So one would observe a difference when disabling 32 in the command line, and in the code.

The PPX magic comes to bite back in many subtle ways.

[jberdine]

Yes: dead but shouldn't be reported. But it is dead in the [@live] sense rather than the [@dead] one, since such code might be the only use of other code, and that other code should not be reported dead.

Considering only the attributes seems pragmatically sensible, since it isn't clear what the use case might be for compiling with -w -32 and expecting dead code analysis to not be noisy.

FWIW, I don't think that this really has anything crucially to do with ppx. Warning 32 predates ppx, and there are reasonable cases where e.g. a codebase's style mandates that each type comes with a common set of operations like compare and hash.

Also, it is probably a good idea to handle the other "unused" warnings similarly. Maybe 32-39, 60, 66.

[jberdine]

edit: s/dead/not dead/

[cristianoc]

There’s also the automatic deletion of dead code. And for that one would like to delete even things kept alive only by this.
But it’s not a big issue. I’ll keep it simple.

[jberdine]

Ok, but for automatic deletion of dead code it's a ppx that outputs a new parsetree right, so no need to change the source, just give it to the rest of the compiler.

[cristianoc]

Currently the analysis adds annotations, and the deletion ppx runs afterwards. So there’s a limit anyway to what can be done with that setup, as you can’t annotate code generated by a ppx.
A fuller solution would require deep integration in the compiler, as typed ast to typed ast xform.

[jberdine]

Currently the analysis adds annotations

Does this work by adding attributes to the parsetree? If so, it would be rolled into one ppx pipeline and not have to worry about whether code was generated or not.

[cristianoc]

No it’s literally printing into the source file.
So it’s very easy to take a quick look at a project without going through the list of reported issues.

[cristianoc]

Btw the analysis operates on the typed tree. Does not fit a ppx pipeline.

[jberdine]

Ah, yes, I knew but overlooked that. It would be a bit of work then to get back to the untyped tree, or construct a new one, or... Hmm. I suspect that you could reuse the implementation technique of ppx_import though, as it reads cmt files and generates code. That needs to use the staged_pps instead of pps flow in dune, but that should also be doable for reanalyze I think.

[cristianoc]

The analysis is global so strictly speaking does not fit in the ppx flow by that reason alone. However of course it could store the results somewhere then run a ppx that reads them.

[jberdine]

Oh yeah, very true. I imagine that you can figure out something compositional to do... ;-)

[cristianoc]

@jberdine this is done in #13.
Unsure about whether to support other suppressions beyond 32. What did you have in mind?

[cristianoc]

A relevant one is 34 Unused type declaration. Though that's a bit different from the sense in which variant/records parts are unused.
Merging this for now.

[jberdine]

The following might also make sense:
37 Unused constructor.
38 Unused extension constructor.
60 Unused module declaration.

[jberdine]

It seems that code like

let[@warning "-32"] foo x = print_int x

still gets flagged if foo is otherwise dead.

[cristianoc]

Let me check.

[cristianoc]

Tried to repro the example here:
#22

No cigar.

What was the example?

[cristianoc]

Oh wait warning not ocaml.warning?

[jberdine]

Yes, they are synonyms.