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

Allow sequences of SEMISEMI in Parse.use_file #1766

Merged
merged 2 commits into from May 11, 2018

Conversation

Projects
None yet
3 participants
@nojb
Copy link
Contributor

nojb commented May 6, 2018

See MPR#7793.

Currently Parse.implementation accepts sequences of ;; but Parser.use_file does not. This leads to files which are parseable using the compiler but not ocamldep (which uses Parse.use_file to process .ml files).

This PR rewrites the Parse.use_file rule in a manner similar to Parse.implementation to fix this discrepancy.

@nojb

This comment has been minimized.

Copy link
Contributor Author

nojb commented May 6, 2018

@hhugo

This comment has been minimized.

Copy link
Contributor

hhugo commented May 7, 2018

@damiendoligez, any chance to get this in 4.07 ?

@gasche

This comment has been minimized.

Copy link
Member

gasche commented May 7, 2018

This is a bugfix, so I see nothing wrong with having it in 4.07, but on the other hand there is a regression risk, so we need someone ( @hhugo?) to carefully review the fact that, indeed, this is synchronized with Parse.implementation, and you are not rejecting programs that it would accept.

@nojb

This comment has been minimized.

Copy link
Contributor Author

nojb commented May 9, 2018

@hhugo could you review this patch?

@hhugo

This comment has been minimized.

Copy link
Contributor

hhugo commented May 9, 2018

I'll try to review tomorrow morning

@hhugo

This comment has been minimized.

Copy link
Contributor

hhugo commented May 11, 2018

I've reviewed the code, it looks correct to me.

@nojb

This comment has been minimized.

Copy link
Contributor Author

nojb commented May 11, 2018

Thanks @hhugo! I will rebase and merge this evening unless someone objects.

@nojb nojb force-pushed the nojb:use_file_SEMISEMI branch from 836191b to 03805df May 11, 2018

@gasche

gasche approved these changes May 11, 2018

Copy link
Member

gasche left a comment

Approved based on @hhugo's review.

@nojb nojb merged commit 521b8de into ocaml:trunk May 11, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@nojb nojb deleted the nojb:use_file_SEMISEMI branch May 11, 2018

@nojb

This comment has been minimized.

Copy link
Contributor Author

nojb commented May 11, 2018

Merged and cherry-picked to 4.07 6fb86e1

@hhugo

This comment has been minimized.

Copy link
Contributor

hhugo commented May 11, 2018

Thanks a lot @nojb

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.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.