Implement enums

[iluuu1994]

RFC: https://wiki.php.net/rfc/enumerations

[stof]

shouldn't this also add a enum_exists function or is this covered by class_exists ?

[TysonAndre]

• probably need to update autoloading to autoload NS\X instead of NS\X::Y and add a test, e.g. in calls to unserialize()
• newInstanceWithoutConstructor may also be an issue

[TysonAndre]

The overall plan seems reasonable, I had some minor comments/questions about the implementation

[iluuu1994]

@TysonAndre As always, thanks for the review! We're almost feature-complete but there's definitely some cleaning up to do 🙂

[nikic]

This looks good apart from the questions re reflection, as well as broken persist / file cache. I'm not sure how to address that part yet.

[dstogov]

I don't see big problems. See my minor comments.

[dstogov]

How does this work with inheritance cache? It's going to persist them anyway.

[nikic]

Yes, they will be persistent in the inheritance cache. Persistence of internal methods is already supported for preloading. The important part is that a) this doesn't happen on Windows (caching for that case is disabled above) and is not persisted in file cache (which is not used for inheritance cache).

[dstogov]

The comment is unclear. Is it outdated or something still has to be fixed? May be handle this in enum::free_obj() handler.

[nikic]

The problem here is that currently we only destroy constants and class constants after freeing the object store, but now constants can contain objects, so we get a leak report. This requires some small changes to shutdown order. I have a patch for this already and plan to apply it afterwards.

[TysonAndre]

What's the intended behavior of define()? As this was a large RFC documentation and implementation I'd overlooked it and assumed it would work, but the tests and rfc document don't mention anything about define( that I saw skimming over them again.

I believe it'd be reasonable and non-controversial for define() to allow anything that const foo = expr; would allow, i.e. to allow enum objects in array keys and as top-level values, e.g. this was probably just overlooked

Ideally, Zend/zend_builtin_functions.c would be changed to "cannot be/contain an object that is not an enum" or something along those lines

php > enum X: string { case FOO='bar'; }
php > const foo = X::FOO;  // this works
php > var_dump(foo);
enum(X::FOO)
php > define('foo2', X::FOO);  // this throws a TypeError

Warning: Uncaught TypeError: define(): Argument #2 ($value) cannot be an object, X given in php shell code:1
Stack trace:
#0 php shell code(1): define('foo2', Object(X))
#1 {main}
  thrown in php shell code on line 1
php > define('foo3', [X::FOO]);

Warning: Uncaught TypeError: define(): Argument #2 ($value) cannot be an object, X given in php shell code:1
Stack trace:
#0 php shell code(1): define('foo3', Array)
#1 {main}
  thrown in php shell code on line 1

[iluuu1994]

Ideally, Zend/zend_builtin_functions.c would be changed to "cannot be/contain an object that is not an enum" or something along those lines

Is there any good reason to not just allow objects in general? Given the general direction of PHP to allow objects in constant expressions.

[TysonAndre]

Is there any good reason to not just allow objects in general? Given the general direction of PHP to allow objects in constant expressions.

The former would be less controversial due to very likely being an oversight - and the workaround of eval('const MY_CONST = ' . var_export(EnumClass::ENUM_CASE, true)) would be worse.

The latter seems more controversial, and many would be opposed to the shift it'd allow for const to contain data that isn't actually constant (e.g. if reading or reviewing code) - the exact direction of that RFC already changed

• (i.e. whether one would want const to mean the object and its properties won't change, or just that the object handle won't change)