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

Tsubst remaining in structure after call to Env.save_signature #7929

Closed
vicuna opened this issue Feb 21, 2019 · 3 comments

Comments

Projects
None yet
2 participants
@vicuna
Copy link

commented Feb 21, 2019

Original bug ID: 7929
Reporter: @trefis
Assigned to: @trefis
Status: assigned (set by @trefis on 2019-02-21T11:15:08Z)
Resolution: open
Priority: normal
Severity: minor
Version: 4.08.0+dev/beta1/beta2
Category: typing
Monitored by: @nojb

Bug description

Noticed when trying odoc on 4.08: ocaml/odoc#311

The regression is due to: 4d4fd52#diff-668ada709c086c9f2b56861bb1872a5dR461

It made the assumption that the order in which we apply the substitution doesn't matter.
Which is wrong in that case because of: https://github.com/ocaml/ocaml/blob/trunk/typing/subst.ml#L345

The obvious fix is to replace the call to List.rev_map by List.rev followed by List.map.

Another potential option would be to introduce functions like class_declaration' (and others) which don't do the cleanup.
Use those in signature_item, and call cleanup_types only once at the end.

I'm going to experiment with that (unless someone jumps in to explain why it can't possibly be correct) and if it works I'll submit that as a fix instead of the obvious one.

@vicuna

This comment has been minimized.

Copy link
Author

commented Feb 21, 2019

Comment author: @gasche

(My personal take-away from this is that we should ramp up on full-opam testing of typer changes.)

@vicuna

This comment has been minimized.

Copy link
Author

commented Feb 21, 2019

Comment author: @trefis

Another potential option would be to introduce functions like
class_declaration' (and others) which don't do the cleanup.
Use those in signature_item, and call cleanup_types only once at the end.

Implemented in #2261

@vicuna vicuna added the typing label Mar 14, 2019

@trefis

This comment has been minimized.

Copy link
Contributor

commented Mar 15, 2019

Fixed by #2261.

@trefis trefis closed this Mar 15, 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.