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

___ #8573

Closed
lexidor opened this issue Sep 21, 2019 · 5 comments
Closed

___ #8573

lexidor opened this issue Sep 21, 2019 · 5 comments

Comments

@lexidor
Copy link
Collaborator

lexidor commented Sep 21, 2019

The following issues are stale and keeping the issue count on hhvm high:

Issue reference Verdict Reason for closing
#1066 Stale This refers to the performance of HHVM back in 2013.
#1447 PHPism? #7926 (unmerged) addressed this issue. Unsure how to feel about modifying a file during a request. This does sound like a PHPism to me.
#1605 Implementation detail of PHP You should not be relying on the order of constants in which constants are defined in get_defined_constants.
#1589 Famously unmaintained It looks like the last non-cleanup commit in the code was about 22 months ago. (Jan 2014)
#1669 PHPism All hack type hints are strictly adhered to.
#2060 Programmer Error Registering a private function to a callback based api is not the fault of the api, but rather a programmer error.
#2108 Deprecated From a conversation with a Facebook employee -> "filters are gone, wrappers that can re-enter the VM are gone." Feels like it applies here too.
#2227 Fixed I can see an HHI for WeakRef, so it exists now.
#2260 Fixed Tried in nightly and it works there. Assume that this works everywhere else too.
#2268 PHP feature unsupported You can't use ArrayAccess<_, _> anyhow since hack will tell you that you are using a subscript operator on something that is not a KeyedContainer<_, _> $foo = new Foo(); $foo['a'] = 'b';``Typing[4005] This is not an object of type KeyedContainer, this is an object of exactly the class Foo.
#2406 hhvm-autoload, HH\fact_parse, and hhast exist It seems like @fredemmott was trying to do what hhvm-autoload is doing now. I presume that hhvm is providing the required functions and hooks to perform this, else how would hhvm-autoload do it?
#2560 Works? I tried the example from PHP docs on PHP authentication and it worked. It is unclear to me which fields may still be missing. I was able to see most of them using hhvm -m server and vardumping the keys of $_SERVER.
#2692 Fixed? My own website is serving some static .json files just fine. That might be because of how my op configured it though. It doesn't seem like this is an issue anymore.
#2730 Can be invoked using parens blog mentions ($this->callable_property)(...$args)
#2926 Untyped hack require_once (like echo and friends) is an untyped statement. This is expected behavior. You should not be requiring things yourself if you can help it. Autoloading is baked into the core of hhvm. The only thing that really needs to be required these days is /vendor/bin/autoload.hack.
#2957 Sample code unsupported It is hard to figure out what sql lib was used. It might have been mysqli. Even if it was something else, the given sample code is php that exploits many deprecated features. I don't think that this is something that we can realistically still use.
#2976 Stub #2976 (comment)
#3015 HHI change call_user_func_array takes Container<_>, stdClass is stopped at typecheck time.
#3193 PHPism We are strict about the difference between static methods and instance methods.
#3273 Updated This describes the current behavior of // strict.

Okay, this took waaay to long already. Will flag a couple of issues every time I create a new one. I want the issue count to slowly go down to something that is more manageable.

@ecreeth
Copy link
Contributor

ecreeth commented Dec 30, 2019

Issue Reason
#6954 PHP related
#6913 PHP 5 related
#6183 PHP 5 related
#6026 PHP 5 related
#5786 PHP 5 related
#5541 PHP 5 related
#3874...? PHP 5 related

@lexidor
Copy link
Collaborator Author

lexidor commented Dec 30, 2019

@ecreeth Could you explain your rationale for #3874? Is this okay to close because this behavior is also correct, but just different from PHP? Or are we merely closing this because of the PHP comparison?

(new ReflectionFunction($closure))->isClosure();

The issue says that this returns false. Does it still do that today? That seems like something worth fixing.

@ecreeth
Copy link
Contributor

ecreeth commented Dec 30, 2019

@lexidor This is due to the comparison with php and the year it was created

@lexidor
Copy link
Collaborator Author

lexidor commented Dec 30, 2019

The quirks that were mentioned in the issue are still accurate to this day. The example does not work out of the box, since ReflectionFunction::getDefiningFunction() is not a thing, you need ReflectionFunction::getDeclaringFunction().

HHVM + Hack 4.38.0-dev (rel)

bool(false)
NULL
object(ReflectionClass) (2) {
  ["name"]=>
  string(27) "Closure$Ship::getClosure;10"
  ["obj":"ReflectionClass":private]=>
  NULL
}

PHP + php 7.4.1 (cli)

bool(true)
object(ReflectionClass)#2 (1) {
  ["name"]=>
  string(4) "Ship"
}
object(ReflectionClass)#2 (1) {
  ["name"]=>
  string(4) "Ship"
}

I don't speak Reflectionista and I don't know if anyone uses this / cares.
It is worth noting that ReflectionFunction::getClosureScopeClass() does not exist in the hhi's.

<?hh // partial
//<?php

function main(): void {
  $ship = new Ship();
  $rf = new ReflectionFunction($ship->getClosure());
  $rf = $rf->getParameters()[0]->getDeclaringFunction();
  \var_dump($rf->isClosure());
  \var_dump($rf->getClosureScopeClass());
  \var_dump($rf->getDeclaringClass());
}

class Ship {
  public function getClosure() {
    return function(string $canon_to_fire) {
      return false;
    };
  }
}

main();

P.S. This doesn't need to be a PHP closure to have his behavior. I used one because of the <?php <?hh // partial dual operation of this script. This code will not work in the future if php closures are removed from the language. 4.30.0

@lexidor
Copy link
Collaborator Author

lexidor commented Jun 2, 2020

I am going over old issues on this repository, to see which ones apply to the current versions of hhvm.

We've come full circle.
I am closing my own issue about needing to close issues.

To all those issues that are on the list above, that I have not closed:
Sorry for marking your issue as one that needs to be closed.
With my far more conservative view on the issues list, I have closed some, but not all of them.
This issue was not made with the level of care that is required for working on open source.

I mainly focused on closing issues that relied on features that were removed from hhvm or relied on php mode.

I am updating the issue title of this issue to three underscores in order to not swing the discussion on whether to keep your issue open.

@lexidor lexidor closed this as completed Jun 2, 2020
@lexidor lexidor changed the title [Github] Close old issues that are no longer applicable ___ Jun 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants