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

get_class(Closure) !== "Closure" #3281

Closed
legoktm opened this issue Jul 25, 2014 · 8 comments
Closed

get_class(Closure) !== "Closure" #3281

legoktm opened this issue Jul 25, 2014 · 8 comments

Comments

@legoktm
Copy link

legoktm commented Jul 25, 2014

$ php -r "echo(get_class(function() {}) . PHP_EOL);"
Closure
$ php -v
PHP 5.4.20 (cli)

In hhvm on Ubuntu 12.04 with the latest nightly:
hphpd> echo(get_class(function() {}) . PHP_EOL);
echo(get_class(function() {}) . PHP_EOL);
Closure$;0$1ba8b96f61946da7de5f3a4457710c1d$

legoktm@legoktm:~$ hhvm --version
HipHop VM 3.1.0 (rel)
Compiler: tags/HHVM-3.1.0-0-g71ecbd8fb5e94b2a008387a2b5e9a8df5c6f5c7b
Repo schema: 88ae0db264d72ec2e2eb22ab25d717214aee568b

Noticed[1] when working with MediaWiki.

@swtaarrs
Copy link
Contributor

This is something we're aware of and were waiting to see if it affected anyone. It shouldn't be hard to fix but I don't remember if we ever decided if it's something we're ok diverging from PHP5 on. @ptarjan, @jdelong you guys touched this recently-ish; any thoughts?

@ptarjan
Copy link
Contributor

ptarjan commented Jul 25, 2014

I thought @LiraNuna just fixed this

@LiraNuna
Copy link
Contributor

No I did not, I fixed the php5 tests to comply. See 8c42809.

@LiraNuna
Copy link
Contributor

Didn't mean to close.

@DmitrySoshnikov
Copy link
Contributor

Any update on this since Jul 25th? Did we decide whether we want to have it as the "Closure" as in PHP, or is it a breaking change?

@paulbiss
Copy link
Contributor

Not that I'm aware of. As @swtaarrs mentioned the fix should be pretty simple, but it wasn't clear that this was impacting any existing software.

@paulbiss paulbiss self-assigned this Nov 8, 2014
@paulbiss
Copy link
Contributor

paulbiss commented Nov 8, 2014

Fix is up internally D1669072

@paulbiss
Copy link
Contributor

paulbiss commented Jan 9, 2015

It seems to be the consensus here that the current behavior is preferable, as it's easier to track down the origin of a particular closure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants