Support for Closure::bind is missing

[stof]

PHP 5.4 introduced Closure::bind() and Closure::bindTo() but they are not implemented in HHVM. This prevents some tools from working (even when they do a version check as HHVM reports a PHP version higher than 5.4).

See phpspec/prophecy#62 for such a report

[scannell]

@paroski can you comment here? I seem to remember some specific difficulties with these.

[paroski]

We can implement these, though using Closure::bind() and Closure::bindTo() will be incompatible with RepoAuthoritative mode.

[henrikbjorn]

what is RepoAuthoritative mode ?

[danslo]

http://www.hhvm.com/blog/257/go-faster

Scroll down :)

[Majkl578]

Bump. Closures binding is in PHP since 5.4.0 so I think HHVM should support it as well. And maybe put a red warning somewhere about the incompatibility with RepoAuthoritative. (Btw. isn't HipHop's goal to reach parity with Zend?)

[scannell]

It is on the list of things to do for compatibility work but not that high at the moment. If someone wants to implement these without RepoAuthoritative support (which is parity with constructs like create_function() and eval()) before we do we'd be happy to review a PR.

[Ocramius]

@paroski what is the reason for Repo.Authoritative incompatibility? May help in understanding the issue there

Also - does work on ext_closure.cpp and ext_closure.h suffice to get this running (besides tests, ofc)?

[scannell]

I believe @paroski is on vacation and will be back after the US holidays.

Fundamentally, this feature is incompatible with RepoAuthoritative (with WholeProgram) mode where all dynamic code constructs are banned -- it is not an issue with a particular implementation. This is fine though; HHVM also supports eval(), create_function(), etc. but not in RepoAuthoritative.

I am guessing getting this to work is going to require diving more deeply into the internals than just the closure extension but I'm not entirely sure as I haven't looked at implementing this.

[scannell]

This was an attempt (which probably needs to rebased) at this if anyone wants to pick it up: https://gist.github.com/scannell/24b3b018f7c1954991b0

It didn't work as-is; the JIT currently asserts.

[bountysource-support]

Facebook has posted a $200 bounty on this issue. The bounty will be paid out to the developer who closes the issue.

[paroski]

We can probably get this implemented some time in the first half of 2014 if no one else gets around to it.

[milesj]

Just ran into this issue today. Probably the only error popping up in my hhvm unit tests.

[vlucas]

I need support for this. Bullet uses it extensively.

[jails]

No news on this subject ?

[lisachenko]

@ptarjan, @paroski, @sgolemon ping ) This issue probably the main stopper for many libraries to run under HHVM.
Even the bounty of $200 can not help to solve this issue :)

[Ocramius]

@lisachenko IIRC, someone at FB was working actively on this (read about it last week or so)

[LiraNuna]

Sadly this has become a large task for me, here's a gist if anyone is interested in continuing my work: https://gist.github.com/LiraNuna/b17303992428a34dad57

Mind you that not all tests pass right now, while all the tests that answer this regex should pass:

hphp/test/zend/good/Zend/tests/closure_0*.php

[Ocramius]

@LiraNuna argh, you were my only hope here :-( Is there anyone experienced with the JIT that can actually help on this? It is really the last major blocker for moving a lot of libraries to full HHVM support :\

[danielstjules]

Looks like @Ocramius upped the bounty to $300!

[Ocramius]

@danielstjules it's not like it will help much :-)

[fredemmott]

I think @paulbiss is currently working on this.

[Majkl578]

@paulbiss: Great job! One huge incompatibility less. :)
Any info whether this will land in 3.3.0 or not?

[simonwelsh]

@Majkl578 It won't. 3.3.0 has already been delayed 4 days, and that's for crashers rather than missing features.

[paulbiss]

/cc @fredemmott
There's zero chance of this making our internal release, but I would support cherry-picking it for 3.3.0.

[Ocramius]

W00t! Going to compile and try it out later today \o/

[lisachenko]

Great job, @paulbiss! And jackpot! :) I will also try a nightly-build later

[Ocramius]

@paulbiss also, please claim https://www.bountysource.com/issues/1037082-support-for-closure-bind-is-missing :-)

[fredemmott]

Sorry, this won't be going into 3.3.0 - our internal and external releases don't diverge, except for build system fixes etc.

[paulbiss]

@Ocramius that's very generous of you, but I can't accept a bounty for doing my job. :-P It would be a conflict of interest, and I worry it sends the wrong message about how we prioritize issues from the community. Would it be possible to apply it to another issue you care about? I think it's a terrific way to involve others in the project.

Also, please open issues for any bugs you find with the feature in your testing. We don't have great test coverage for it so I wouldn't be surprised if bugs slipped through. Thanks!

[Ocramius]

Would it be possible to apply it to another issue you care about? I think it's a terrific way to involve others in the project.

Fine by me, I can simply revoke the bounty if it's allowed by that site. I really don't care about other issues right now :-)

Also, please open issues for any bugs you find with the feature in your testing. We don't have great test coverage for it so I wouldn't be surprised if bugs slipped through. Thanks!

Will do as soon as I get it built.

[Ocramius]

Before:

Tests: 384, Assertions: 1046, Failures: 17, Skipped: 24.

After:

Tests: 403, Assertions: 1207, Skipped: 2.

I'd like to call that a great success :-)

[bobthecow]

👍