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

3.24: add `safe_pass_by_ref` migration #21

Closed
fredemmott opened this Issue Oct 27, 2017 · 2 comments

Comments

Projects
None yet
2 participants
@fredemmott
Contributor

fredemmott commented Oct 27, 2017

This option will be supported in HHVM 3.23+; it requires that callsites specify '&' when passing an argument by ref; for example:

$foo = vec[1,3,2];
sort(&$foo);

.hhconfig:

enable_experimental_tc_features=safe_pass_by_ref

With the option off, in 3.23+, the '&' is ignored - but it is an error in master. This can be worked on now if you build HHVM master, or brew tap hhvm/hhvm; brew install hhvm-preview if working on a mac.

This requires building support for migrations that query and fix typechecker errors, rather than purely operating on the AST

@ignacvucko

This comment has been minimized.

ignacvucko commented Nov 23, 2017

This ends up highlighting hundreds of issues within hhvm source code itself when used with composer.

composer.json
{
"require":
{
"facebook/xhp-lib": "2.x",
}
}

$ hh_client
... dozens of errors ...
vendor/hhvm/hsl/src/c/select.php:180:21,32: This argument should be annotated with & (Typing[4168])
... dozens of errors ...

As far as I know, there isn't a way to ignore the vendor directory by adding it to your .hhconfig file.

So, how is the above best handled?

@fredemmott

This comment has been minimized.

Contributor

fredemmott commented Nov 23, 2017

For now, you can use sed or similar to replace /*ctpbr:&*/ in the HSL with &; I'll start talking to people next week about when we should switch what versions of HHVM HSL master should be targeting.

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