Mantis Bug Tracker

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0005357OCamlOCaml generalpublic2011-09-17 01:072013-08-31 12:49
Reporterfuruse 
Assigned Tofrisch 
PrioritynormalSeverityfeatureReproducibilityhave not tried
StatusclosedResolutionfixed 
PlatformOSOS Version
Product Version3.12.1 
Target VersionFixed in Version3.13.0+dev 
Summary0005357: Warning for redundant open M and let open M in
DescriptionGHC 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 InformationAnd 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.
TagsNo tags attached.
Attached Filespatch file icon ocaml-redundant-open.patch [^] (22,303 bytes) 2011-09-17 01:07 [Show Content]

- Relationships
related to 0005438resolvedfrisch A warning to detect more unused values and value declarations 

-  Notes
(0006456)
protz (manager)
2011-12-21 14:57

That sounds like a pretty cool warning to have, and the patch seems to be moderately invasive. Jacques, any opinion on this?
(0006490)
garrigue (manager)
2011-12-22 10:52

After reading it, it seems to do something useful in a straightforward way.

My only reservation is the "with_open" type constructor.
I would prefer something cleaner, like
  type 'a info = Path.t * 'a * Path.t option
but this could actually make things more complicated...
This can easily be fixed afterwards.

Also we should probably remove all unneeded open's from the sources,
so there is no reason to disable the warning.

I'm for merging, but maybe better to ask on caml-devel first.
(0006494)
frisch (developer)
2011-12-22 11:08

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?
(0006499)
furuse (reporter)
2011-12-22 12:14

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.
(0006501)
frisch (developer)
2011-12-22 12:41

I've added a similar warning to the unused_declarations branch:

http://caml.inria.fr/cgi-bin/viewvc.cgi?view=revision&revision=11937 [^]
(0006718)
frisch (developer)
2012-01-18 10:36

See 0005438. The new warning 33 detects unused open statements.
(0007403)
furuse (reporter)
2012-05-03 04:31

Thanks, the new warnings rock!

- Issue History
Date Modified Username Field Change
2011-09-17 01:07 furuse New Issue
2011-09-17 01:07 furuse File Added: ocaml-redundant-open.patch
2011-12-21 14:57 protz Note Added: 0006456
2011-12-22 10:52 garrigue Note Added: 0006490
2011-12-22 11:08 frisch Note Added: 0006494
2011-12-22 12:14 furuse Note Added: 0006499
2011-12-22 12:41 frisch Note Added: 0006501
2012-01-17 23:27 lefessan Relationship added related to 0005438
2012-01-18 10:36 frisch Note Added: 0006718
2012-01-18 10:36 frisch Status new => resolved
2012-01-18 10:36 frisch Fixed in Version => 3.13.0+dev
2012-01-18 10:36 frisch Resolution open => fixed
2012-01-18 10:36 frisch Assigned To => frisch
2012-05-03 04:31 furuse Note Added: 0007403
2013-08-31 12:49 xleroy Status resolved => closed


Copyright © 2000 - 2011 MantisBT Group
Powered by Mantis Bugtracker