Add support for ::class to constant()

[dunglas]

This change allows retrieving the value of the ::class special constant using the constant() function:

var_dump(
  constant('\DateTime::class')
);

For instance, Twig's constant() helper internally uses this function. This patch allows writing code like that:

`myObject` contains a random object, retrieve its class:
{{ constant('class', myObject) }}

TODO: write an RFC?

[francislavoie]

Why not just use https://www.php.net/manual/en/function.get-class.php ?

[dunglas]

@drealecs @francislavoie Sure I could patch Twig (and I will), but my goal here is to make the language consistent. constant() allows accessing to all class constants except this one, that's weird.

[claudepache]

I have not checked whether the implementation handles those edge cases, but for sure tests are lacking:

• foo::class does never trigger the class autoloader, nor does it complain about missing class. Neither should constant('foo::class').
• As class is a keyword, it is case-insensitive: const("foo::CLaSs") must be supported.
• constant('\\A::B') is equivalent to constant('A::B'); it should be ensured that the same holds for constant('\\A::class') vs. constant('A::class').

And, of course, the implementation of the following is definitely missing:

• If you add support for ::class in constant(), you must also add support for it in defined().

And:

• You should consider (and justify) whether or not you add support in ReflectionClassConstant.

[dunglas]

Thanks for the feedback @claudepache!
I added tests covering the edge cases you mentioned (and for defined(), which is also supported).

I also updated the implementation to not trigger autoloading and not complain about missing classes, but my patch introduces a memory leak (https://github.com/php/php-src/pull/6763/files#diff-6f9b1b4ec76510c10bf1c606e8629eb6b4d5f9df8f21258fd73a3e76bcab4796R384) and I don't know how to fix it. The problem is that for non-existing classes we must create a new zval containing the class name, but the destructor of the returnedzval is never called by parent functions because they expect that we return an existing zval, not a new one. Regardless of if we merge this patch or not, I would appreciate some guidance on how to fix this, for my curiosity and to learn more about PHP internals :)

You should consider (and justify) whether or not you add support in ReflectionClassConstant.

This is a good one! On the one hand, adding support in ReflectionClassConstant would also make ::class less special and would increase the consistency of the language, but on the other hand, it can be annoying if RefelectionClass::getConstants() starts returning class in the list: this would be a BC break (and this wouldn't be very practical to have to always filter this constant). Maybe could we "hide" class behind a new filter, or support it when calling ReflectionClassConstan() or RefelectionClass::getConstant() directly but not include class in the return of getConstants().

As a side effect, adding class to ce->constants_table could fix my memory leak issue, but would consume more memory for an edge case.

[beberlei]

The larger question would be what to do abut magic constants, for example constant("__DIR__") et all also not work.

[nikic]

Thanks for the feedback @claudepache!
I added tests covering the edge cases you mentioned (and for defined(), which is also supported).

I also updated the implementation to not trigger autoloading and not complain about missing classes, but my patch introduces a memory leak (https://github.com/php/php-src/pull/6763/files#diff-6f9b1b4ec76510c10bf1c606e8629eb6b4d5f9df8f21258fd73a3e76bcab4796R384) and I don't know how to fix it. The problem is that for non-existing classes we must create a new zval containing the class name, but the destructor of the returnedzval is never called by parent functions because they expect that we return an existing zval, not a new one. Regardless of if we merge this patch or not, I would appreciate some guidance on how to fix this, for my curiosity and to learn more about PHP internals :)

It's not possible to fix this without changing the API. You'd convert zval *zend_get_constant_ex(zend_string *cname, ...) into bool zend_get_constant_ex(zval *result, zend_string *cname, ...) with the caller responsible for releasing the result.

[dunglas]

Thanks, @nikic! That's what I thought. An API change like this is probably too big to support such an edge case.

[nikic]

Thanks, @nikic! That's what I thought. An API change like this is probably too big to support such an edge case.

Changing the API isn't really a big deal. It's not used much and largely for internal use.

For reference, the internals discussion for this change: https://externals.io/message/113426 As the reception wasn't super positive, do you plan to pursue an RFC on the topic?

[dunglas]

Thanks for the explanation regarding how to change the API!

do you plan to pursue an RFC on the topic?

As there is no consensus on this and it provides only small benefits, no I don't think so. Let's close!