Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs should mention that open! also suppresses warning 33 #6638

Closed
vicuna opened this issue Oct 30, 2014 · 7 comments
Closed

docs should mention that open! also suppresses warning 33 #6638

vicuna opened this issue Oct 30, 2014 · 7 comments
Assignees

Comments

@vicuna
Copy link

vicuna commented Oct 30, 2014

Original bug ID: 6638
Reporter: dwang
Assigned to: @alainfrisch
Status: resolved (set by @alainfrisch on 2018-11-09T12:42:28Z)
Resolution: fixed
Priority: normal
Severity: minor
Version: 4.02.1
Fixed in version: 4.08.0+dev/beta1/beta2
Category: typing
Monitored by: @hcarty

Bug description

http://caml.inria.fr/pub/docs/manual-ocaml/extn.html#sec237 says:

Since OCaml 4.01, open statements shadowing an existing identifier
(which is later used) trigger the warning 44. Adding a ! character
after the open keyword indicates that such a shadowing is intentional
and should not trigger the warning.

But this code:

module M = struct type t end
open M

gives this error:

Warning 33: unused open M.

whereas this code:

module M = struct type t end
open! M

compiles fine.

Maybe we could just update the docs to say explicitly that open! also
suppresses warning 33?

@vicuna
Copy link
Author

vicuna commented Oct 30, 2014

Comment author: @lpw25

I think that open! should not suppress warning 33. If anything the semantics of open! (i.e. "I am deliberately shadowing a value with this open") make it even less likely that the open is not intended to be used.

@vicuna
Copy link
Author

vicuna commented Oct 31, 2014

Comment author: @garrigue

Note that the new -open command line parameter also relies on "open!".
Changing the behavior for one would also change it for the other.
I tend to agree with Leo that the right behavior is not to suppress warning 33
(which is disable by default anyway), but changing the behavior for one would also change it for the other.

@vicuna
Copy link
Author

vicuna commented Oct 31, 2014

Comment author: @sliquister

We can understand open! more generally as meaning "don't complain about this open". Otherwise there is no nice way to turn off warning 33 locally in structures, and in signatures it's even worse.

@vicuna
Copy link
Author

vicuna commented Feb 16, 2017

Comment author: @xavierleroy

In the end I don't understand if we need to change something or not. If not, please resolve this PR. If so, let's assign it to someone and set a target version.

@vicuna
Copy link
Author

vicuna commented Mar 16, 2017

Comment author: @alainfrisch

I think it's just a plain bug that warning 33 is suppressed by open!. The fix is easy, I'll work on it.

Note that the new -open command line parameter also relies on "open!".

Do we all agree that this command-line parameter should never raise warning 33? I think this will still be the case after the fix, since Env.open_signature will only raise warnings if passed an explicit non-ghost location.

@vicuna
Copy link
Author

vicuna commented Mar 16, 2017

Comment author: @alainfrisch

#1110

@vicuna
Copy link
Author

vicuna commented Nov 9, 2018

Comment author: @alainfrisch

Added a dedicated warning (66, twice as strong as 33) to report unused "open!".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants