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
Spurious unused value warning with destructive substitution #7852
Comments
Comment author: @trefis Btw, this would also happen when destructively substituting a module. |
Comment author: @trefis This was reverted in #2092 FTR, what was happening is basically this: Usage of items in OCaml is tracked by their name (i.e. the string, not the ident) and a Location. Once we're done with the typing (we're now in Compile_common.typecheck_intf), we call "ignore (Includemod.signatures info.env sg sg)". Which has the effect of marking every item in the signature as used (which makes sense for mlis). The issue is that we're doing that by going over the signature, taking the name + location of every item, and then using that to look in the table in Env. But the location we have is the new one, not the initial one which was used to add the items to the table. So we never mark the items that were initially added. |
Comment author: @gasche Thanks for the summary, I get it now. (Another motivation to have a proper "declaration path" notion instead of source locations.) |
Actually, I’d like to keep this open until “a proper fix” is implemented. |
I wrote a prototype for that based on 4.07 last week (because at the time it's still wasn't easy to test 4.08 / trunk on actual code) which I plan to rebase onto trunk (and submit a PR) next week. |
Thanks! I vaguely thought of this again when seeing the type pos =
Module of Ident.t | Modtype of Ident.t | Arg of Ident.t | Body of Ident.t
type error = pos list * Env.t * symptom |
This issue has been open one year with no activity. Consequently, it is being marked with the "stale" label. What this means is that the issue will be automatically closed in 30 days unless more comments are added or the "stale" label is removed. Comments that provide new information on the issue are especially welcome: is it still reproducible? did it appear in other contexts? how critical is it? etc. |
@trefis Has this been fixed now? |
Original bug ID: 7852
Reporter: @trefis
Assigned to: @trefis
Status: assigned (set by @trefis on 2018-09-21T16:46:49Z)
Resolution: open
Priority: normal
Severity: minor
Version: 4.08.0+dev/beta1/beta2
Category: typing
Monitored by: @nojb @hhugo @Drup
Bug description
This regression was introduced by #1737
I'll try to fix it without having to revert the GPR.
But if I fail, we can always revert it, it's not that important.
Steps to reproduce
The text was updated successfully, but these errors were encountered: