Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:125022
Content-Type: text/plain;
	charset=utf-8
Precedence: bulk
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3696.120.41.1.8\))
Subject: Re: [PHP-DEV] Should there be a `get_declared_enums()` function ?
In-Reply-To: <66BFFC0C.1090900@adviesenzo.nl>
Date: Sat, 17 Aug 2024 00:54:07 -0400
Cc: PHP internals <internals@lists.php.net>
Content-Transfer-Encoding: quoted-printable
Message-ID: <DF2F5FC7-0F5F-4000-95CF-1FBC6CDA7745@newclarity.net>
References: <66BD5F05.8010409@adviesenzo.nl>
 <8dc51539-20bb-4017-b859-76066e2287e5@gmx.de>
 <4AA66A3E-1ACF-48F5-81A2-C6AAAE8B3FD0@newclarity.net>
 <66BFFC0C.1090900@adviesenzo.nl>
To: Juliette Reinders Folmer <php-internals_nospam@adviesenzo.nl>
From: mike@newclarity.net (Mike Schinkel)

Hi Juliette,

> On Aug 16, 2024, at 9:25 PM, Juliette Reinders Folmer =
<php-internals_nospam@adviesenzo.nl> wrote:
>=20
> On 17-8-2024 2:00, Mike Schinkel wrote:
>> To everyone on this thread and thus nobody in specific (hence my top =
post):
>>=20
>> Given that get_declared_*() populates a new ZEND_HASH each time the =
function is called[1] =E2=80=94 which for classes can easily be hundreds =
of strings =E2=80=94 would there be any appetite to:=20
>>=20
>> 1. Add an optional parameter =E2=80=94 e.g. =
`get_declared_classes($startingFrom)` =E2=80=94 that can limit the =
number of symbols starting with "n"  where if n=3D=3D100 then it would =
return array_slice($symbols,100), and=20
>>=20
>> 2. Add a function =E2=80=94 `get_declared_class_count()` =E2=80=94 =
that returns the current number of declared symbols of whichever kind of =
symbol, so we can get the "n" before calling `include`/`require` to use =
with get_declared_*() after calling  `include`/`require`?
>>=20
>>=20
>> This would cut down on creating hashes with 100+ strings that get =
immediately thrown away, and also cut down on memory =
fragmentation/garbage collector churn.=20
>>=20
>> -Mike
>>=20
>> [1]=20
>> =
https://github.com/php/php-src/blob/18d41502da0da1bb3928e60c41f1b821974c2c=
01/Zend/zend_builtin_functions.c#L1371
>=20
> Mike,
>=20
> While your suggestion does not directly relate to what triggered my =
mail to the list, I very very much like the idea.

Excellent!

> It's as if you are reading my mind or rather, reading one of the =
autoloaders [1] I need to maintain, in which your above outlined =
proposal should definitely make for more efficiency.

Thank you for sharing that.  Yes, your use-case is very similar to mine.=20=


> Having said that, the same parameter + extra function would then also =
be needed for `get_declared_interfaces()`, `get_declared_traits()` - =
and, if it would be added, `get_declared_enums()`.

That was my assumption and intention, but I did not say so explicitly. I =
should have.

Thanks to Ayesh's excellent PR, I was able to recognize that all of =
those functions in-fact are implemented via one internal function =
`get_declared_class_impl()` so it only made sense =E2=80=94 to me, at =
least =E2=80=94 that modifying it would enable the same functionality =
for all four (4) userland functions.

> There is one problem I can see with this approach though: since PHP =
7.4, the return value of `get_declared_classes()` (and friends) does not =
guarantee any particular order anymore [2].
> So, an `array_slice($symbols, $n)` may not get you the _latest_ =
classes loaded, so I think this would only work if the order of classes =
is guaranteed in some way.

That is a super relevant insight and one I was actually not aware of.

Do you happen to know if it was triggered by an explicit RFC to that =
effect, or if it resulted because of a side-effect of some other =
implementation change?  =46rom my perspective, that is definitely a BC =
break and one that I am surprised I never noticed before?  Do you also =
know if it is a case of "not guaranteed, but in-fact actual fact it =
always works that way, or not?" =20

I have not used my lib that needs that functionality to work in a few =
years, so maybe it is broken now and those who are now maintaining those =
apps have had to deal with it?

Given that you see value in the same use-case I had which was an =
autoloader, maybe it would be better to achieve the functionality we =
need in a different, more efficient way? =20

What if instead PHP were to implement an optional 2nd callback parameter =
to `include()` / `require()` / `include_once()` / `require_once()` to =
allow us to capture the symbols loaded and their paths? The callback =
function could return `void` and accept an array of `$symbols` with the =
following guaranteed minimum structure?

$symbols =3D array(
   'classes' =3D> [],
   'interfaces' =3D> [],
   'traits' =3D> [],
   'enums' =3D> [],
);

With that you could reimplement all that PHP code you linked =E2=80=94 =
at least for versions of PHP supporting the callback =E2=80=94 to look =
like this:

function loadFile($path)
{
   if (strpos(__DIR__, 'phar://') !=3D=3D 0) {
     $path =3D realpath($path);
     if ($path =3D=3D=3D false) {
       return false;
     }
   }
   if (isset(self::$loadedClasses[$path]) =3D=3D=3D true) {
     return self::$loadedClasses[$path];
   }
   include($path, function($symbols) use ($path) {
     foreach ($symbols['classes'] as $className) {
       self::$loadedClasses[$path]    =3D $className;
       self::$loadedFiles[$className] =3D $path;
     }
   });
   return self::$loadedClasses[$path];
}//end loadFile()

This approach would be a less disruptive than my prior suggestion which =
would have added four (4) new `*_count()` functions and new optional =
parameters for four (4) existing function vs. this proposal which just =
adds new optional parameters for four (4) other existing functions. =
Further, and the signature(s) required for parameters would be much =
easier to settle on. =20

I checked the code[2] and if I understand it correctly would (only?) =
require adding a Zend Observer[3] callback to be notified in =
`zend_bind_class_in_slot()` although I still cannot figure out where =
that observer would need to be added.

Going this approach would also make the userland code in PHPCodeSniffer =
a lot more efficient than the code you currently must run for every =
included file simply to determine what symbols were loaded.

What do you think if this alternate approach?

-Mike=20

[1] =
https://github.com/php/php-src/blob/18d41502da0da1bb3928e60c41f1b821974c2c=
01/Zend/zend_builtin_functions.c#L1371
[2] =
https://github.com/php/php-src/blob/php-8.3.10/Zend/zend_compile.c#L1231-L=
1269
[3] =
https://www.linkedin.com/advice/3/what-some-best-practices-common-pitfalls=
-using-zend