Skip to content

Fix parsing roundtrip error in raw idents#38

Merged
mshinwell merged 2 commits intomshinwell:5.2-runtime-wip-mainfrom
ncik-roberts:5.2-runtime-wip-main-fix-parsing-roundtrip-error
Aug 27, 2024
Merged

Fix parsing roundtrip error in raw idents#38
mshinwell merged 2 commits intomshinwell:5.2-runtime-wip-mainfrom
ncik-roberts:5.2-runtime-wip-main-fix-parsing-roundtrip-error

Conversation

@ncik-roberts
Copy link
Copy Markdown

The merge dropped a bunch of ident_of_name for Pexp_newtypes; this PR reintroduces them to get the pprintast parsetree test to pass.

Copy link
Copy Markdown

@ccasin ccasin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes look good, sorry I missed this in initial review.

I think there is one more missing: I can't leave a comment there, but upstream has a ident_of_name on the Pexp_newtype case of the pp_print_pexp_function helper in binding. I think the equivalent on our side should be to add it at in the printer for the result of tyvars_str on line 1764 in your version of the file

@ncik-roberts
Copy link
Copy Markdown
Author

Thanks for checking that — I agree that it's a bug, but disagree that it corresponds to pp_print_pexp_function upstream. In fact, I think the line you point out corresponds to a bug upstream:

$ opam exec --switch=5.2.0 -- ocaml -dsource
OCaml version 5.2.0
Enter #help;; for help.

# let f : type \#let. \#let -> \#let = fun x -> x;;

let f : type let. \#let -> \#let = fun x -> x;;

which is not to say that we shouldn't fix it, but that I'm just going to file an issue upstream.

@ncik-roberts
Copy link
Copy Markdown
Author

@mshinwell please merge this one.

@mshinwell mshinwell merged commit e133ae7 into mshinwell:5.2-runtime-wip-main Aug 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants