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

Fix is_nonexpansive for Texp_open #2166

Merged
merged 3 commits into from Nov 27, 2018

Conversation

Projects
None yet
4 participants
@trefis
Copy link
Contributor

commented Nov 27, 2018

Ping @lpw25 .

@trefis trefis force-pushed the trefis:fix-open branch from 1b86558 to a761770 Nov 27, 2018

@lpw25

This comment has been minimized.

Copy link
Contributor

commented Nov 27, 2018

Looks good. Merging.

@lpw25 lpw25 merged commit 0ae2da1 into ocaml:trunk Nov 27, 2018

0 of 2 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details

@trefis trefis deleted the trefis:fix-open branch Nov 27, 2018

@gasche

This comment has been minimized.

Copy link
Member

commented Nov 28, 2018

What about the suggestion to make the pattern-matching in this function non-fragile?

@objmagic

This comment has been minimized.

Copy link
Contributor

commented Nov 28, 2018

@trefis Thanks for fixing this bug! sorry i missed it.

@gasche do you mean the is_nonexpansive function? and can you clarify what "fragile" here means?

@gasche

This comment has been minimized.

Copy link
Member

commented Nov 28, 2018

Yes, I mean this function. "fragile" is in the sense of warning 4:

4 Fragile pattern matching: matching that will remain complete even if additional constructors are added to one of the variant types matched.

Typically a pattern-matching is fragile if it ends with a clause | _ -> ....

@trefis

This comment has been minimized.

Copy link
Contributor Author

commented Nov 28, 2018

I wanted to. But when I asked Merlin to do that for me, the pattern it generated was fairly ugly. So I backtracked.

gretay-js added a commit to gretay-js/ocaml that referenced this pull request Dec 3, 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.