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

Syntax: allow [module M(_:S) = struct end] syntax #1958

Merged
merged 1 commit into from Aug 7, 2018

Conversation

Projects
None yet
3 participants
@hhugo
Copy link
Contributor

commented Aug 2, 2018

While working on ocamlformat, I noticed the following syntax was not allowed.

module M(_:S) = struct end

_ is not accepted, only UIDENT is.

Note that the following is currently accepted

module M = functor (_:S) -> struct end
@hhugo

This comment has been minimized.

Copy link
Contributor Author

commented Aug 6, 2018

ping @gasche, since you seem to be in the mood for merging parser related PR :)

@gasche

gasche approved these changes Aug 6, 2018

Copy link
Member

left a comment

There you go. You have to edit and rebase Changes.

(I seem to vaguely remember that a related feature, module _ = ..., was controversial and ultimately shot down, but personally I think that those are just fine and in this case there is the functor (_ : S) -> argument which is strong.)

@yallop

This comment has been minimized.

Copy link
Member

commented Aug 6, 2018

I seem to vaguely remember that a related feature, module _ = ..., was controversial and ultimately shot down

Yes: the discussion's under Mantis 6662. The issue's still open, and perhaps the main objection (that changing the parsetree had a significant cost) is weaker now that ppx tooling has improved.

@gasche

This comment has been minimized.

Copy link
Member

commented Aug 6, 2018

I won't fight to put that back precisely during the time period where all parser changes induce more work for me, but I support your cause :-)

@hhugo

This comment has been minimized.

Copy link
Contributor Author

commented Aug 6, 2018

I don't know what section to put this one in. Language feature or Bug fixes ?

@gasche

This comment has been minimized.

Copy link
Member

commented Aug 6, 2018

Either is fine by me. I might go for "bug fix" to focus on bigger changes in the main sections.

@hhugo hhugo force-pushed the hhugo:functor branch from 151a82a to cda69ce Aug 6, 2018

@hhugo hhugo force-pushed the hhugo:functor branch from cda69ce to 3b448ca Aug 6, 2018

@hhugo

This comment has been minimized.

Copy link
Contributor Author

commented Aug 6, 2018

rebased & squashed

@gasche

This comment has been minimized.

Copy link
Member

commented Aug 6, 2018

I won't fight to put that back

I realize this comment was a bit dismissive. I would personally be happy to merge the module _ = ... change now (4.08 is going to be full of Parsetree changes anyway, I don't think this one would hurt in any noticeable way), but I'm trying to limit my involvement with OCaml development to a minimum during the POPL review period, so I won't go out of my way to move it. If you want to propose a PR, please feel free.

@gasche gasche merged commit e68a432 into ocaml:trunk Aug 7, 2018

2 checks passed

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

@hhugo hhugo deleted the hhugo:functor branch Aug 7, 2018

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.