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

Comments

Projects
None yet
2 participants
@vicuna
Copy link

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

This comment has been minimized.

Copy link
Author

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

This comment has been minimized.

Copy link
Author

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

This comment has been minimized.

Copy link
Author

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

This comment has been minimized.

Copy link
Author

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

This comment has been minimized.

Copy link
Author

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

This comment has been minimized.

Copy link
Author

commented Mar 16, 2017

Comment author: @alainfrisch

#1110

@vicuna

This comment has been minimized.

Copy link
Author

commented Nov 9, 2018

Comment author: @alainfrisch

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

@vicuna vicuna closed this Nov 9, 2018

@vicuna vicuna added the typing label Mar 14, 2019

@vicuna vicuna added the bug label Mar 20, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.