Warning for redundant open M and let open M in #5357
Comments
|
Comment author: @protz That sounds like a pretty cool warning to have, and the patch seems to be moderately invasive. Jacques, any opinion on this? |
|
Comment author: @garrigue After reading it, it seems to do something useful in a straightforward way. My only reservation is the "with_open" type constructor. Also we should probably remove all unneeded open's from the sources, I'm for merging, but maybe better to ask on caml-devel first. |
|
Comment author: @alainfrisch Something I don't understand with the implementation is that "open statements" are identified simply with the opened path. So if we have two opens on the same module, and only one is actually used, the other will not be reported as being unused? |
|
Comment author: furuse Jacques: I was kicked out from caml-devel... :-( Alain: Yeah, you are right. It should be better if we can detect all the unused open M's once and for all. I did not write that part just for simplicity. |
|
Comment author: @alainfrisch I've added a similar warning to the unused_declarations branch: http://caml.inria.fr/cgi-bin/viewvc.cgi?view=revision&revision=11937 |
|
Comment author: @alainfrisch See #5438. The new warning 33 detects unused open statements. |
|
Comment author: furuse Thanks, the new warnings rock! |
Original bug ID: 5357
Reporter: furuse
Assigned to: @alainfrisch
Status: closed (set by @xavierleroy on 2013-08-31T10:49:10Z)
Resolution: fixed
Priority: normal
Severity: feature
Version: 3.12.1
Fixed in version: 3.13.0+dev
Category: ~DO NOT USE (was: OCaml general)
Related to: #5438
Monitored by: meyer @protz mehdi "Pascal Cuoq" dario @ygrek @glondu "Julien Signoles" @hcarty @Chris00 nogin @garrigue
Bug description
GHC has a warning for never used imports; such imports are just redundant and cause unexpected name space contamination. The warning is useful to keep up your import list minimal as possible.
OCaml's open has the same issue of the name space contamination, and unnecessary opens should be warned, too.
Additional information
And I have added a new warning for it.
You can obtain the latest diff for OCaml 3.12.1 from my repo at https://bitbucket.org/camlspotter/mutated_ocaml , redundant_open_warning branch.
With this patch, I have found nearly 150 redundant opens in OCaml source code!
A global hashtbl and side effects are used to minimize the amount of modifications for easy patching. If we are going to implement this feature in a future OCaml officially, we could make it cleaner.
File attachments
The text was updated successfully, but these errors were encountered: