search

ocaml-flambda / flambda-backend

The Flambda backend project for OCaml
93 stars 67 forks source link

Fix warning 34 (unused type declaration) for locally-abstract types #2683

Closed ncik-roberts closed 3 weeks ago

ncik-roberts commented 4 weeks ago

Fix a bug introduced in #1548: we unintentionally stopped warning on some unused locally-abstract types. Add a test to ensure this doesn't happen again in the future.

The bug exists upstream too.

The source of the bug is that the "unused X" check in env.ml does not run if the location is marked as ghost. #1548 makes more Pexp_newtypes ghost. (I think they should be marked ghost: the range of text indicated by the location in the source program is not something that can be called a "newtype expression", and this is the usual criteria we use. But the fact of the matter is that the compiler uses ghostiness as a proxy for other properties, and sometimes there is a sharp edge like this one.)

The fix I take is to use the identifier's location (which is never ghost) for purposes of this check, not the Pexp_newtype's location.

Review: the first commit adds a regression test with bad old output, and the second commit fixes the bug and updates the test output.

ncik-roberts commented 3 weeks ago

My testing didn't turn up any ppx that relies on this. I'll merge soon.