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

#show broken with -short-paths #9828

Closed
yallop opened this issue Aug 4, 2020 · 2 comments · Fixed by #9831
Closed

#show broken with -short-paths #9828

yallop opened this issue Aug 4, 2020 · 2 comments · Fixed by #9831
Labels
Milestone

Comments

@yallop
Copy link
Member

yallop commented Aug 4, 2020

This is a bug in 4.11.0+rc1:

$ ./install/bin/ocaml -short-paths
        OCaml version 4.11.0+dev16-2020-08-04

# type 'a t  = T;;
type 'a t = T
# #show t;;
>> Fatal error: Ident.rename t
Fatal error: exception Misc.Fatal_error

It was apparently introduced by #9086, specifically this change:

diff --git a/toplevel/topdirs.ml b/toplevel/topdirs.ml
index f4526692b6..78ab7eb51f 100644
--- a/toplevel/topdirs.ml
+++ b/toplevel/topdirs.ml
@@ -545,15 +545,66 @@ let () =
   reg_show_prim "show_type"
     (fun env loc id lid ->
        let _path, desc = Env.lookup_type ~loc lid env in
-       [ Sig_type (id, desc, Trec_not, Exported) ]
+       [ Sig_type (id, desc, Trec_first, Exported) ]
     )
     "Print the signature of the corresponding type constructor."

After this change, Printtyp.hide_rec_items attempts to rename persistent idents (i.e. Ident.Global values), which fails with the Misc.Fatal_error exception seen above.

I haven't investigated further, but I expect the easiest fix is to revert the single-line change above to restore the previous behaviour (i.e. printing type definitions with a bogus nonrec rather than exiting the interpreter with a fatal error).

@dra27 dra27 added this to the 4.11 milestone Aug 5, 2020
@dra27 dra27 added the bug label Aug 5, 2020
@Octachron
Copy link
Member

Yet another reason to switch to merlin's short path implementation. It is probably better to avoid a temporary regression on 4.11.0 indeed.

yallop added a commit to yallop/ocaml that referenced this issue Aug 5, 2020
@yallop
Copy link
Member Author

yallop commented Aug 5, 2020

I expect the easiest fix is to revert the single-line change above to restore the previous behaviour

#9831 makes that change.

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

Successfully merging a pull request may close this issue.

3 participants