Re: function autoloading v4 RFC

On Sun, Aug 18, 2024, at 11:36, Stephen Reay wrote: > > Hi Rob, > >> On 18 Aug 2024, at 04:33, Rob Landers <rob@bottled.codes> wrote: >> >> I wouldn't consider it a BC break, no. But (ironically?), Symfony crashes with this >> change. > > > I wasn't aware of that specific code before but it's exactly the type of issue I was > talking about earlier. > >> Ah, good catch. I've updated this and gone through other relevant functions. > The RFC now says "The spl_autoload function will not be updated.", but that will > *also* break if it isn't updated to at least *account for*, even if it doesn't *use* the > second argument given. > > However I'm also curious why you would *specifically* make it *not* support function > loading? > The current implementation should work unmodified, once the signature is changed to accept an > int as the second parameter (and move the current 2nd parameter to 3rd), There is nothing > "class specific" in the existing implementation except for a couple of variable names. > > > I would imagine you also need to update spl_autoload_unregister[1] - it also needs to be able > to identify the type of autoloader it's operating on (because the same autoloader might be > defined for both types). > > And lastly I think you'd need to adapt spl_autoload_functions[2] too - perhaps the same as > the others, introduce a parameter int $type = SPL_AUTOLOAD_CLASS, so > existing code works as-is, otherwise it'd be impossible to know how a given autoloader was > registered. > > > 1: > https://www.php.net/manual/en/function.spl-autoload-unregister.php > 2: > https://www.php.net/manual/en/function.spl-autoload-functions.php > > > Cheers > > Stephen Hello again internals, The implementation has now reached a stable point and things discovered during the implementation have been reflected in the RFC text itself. I did my best to assess the impact to Opcache, but I suspect someone much more familiar with how opcache works will need to take a look. From my understanding of the changes needed in zend_execute, it looks like there are some changes needed to the JIT helpers, but there may be more changes required that aren't so obvious. I've added a helper that can be used as a drop-in replacement for looking up functions from the function table directly, which should help. Lastly, I do plan to open a docs PR in the coming week that will probably trigger some smaller updates to the RFC; mostly to smooth out the wording and make it more friendly for non-technical people, but the technical aspects shouldn't change (barring the discussion here, of course). > The RFC now says "The spl_autoload function will not be updated.", but that will > *also* break if it isn't updated to at least *account for*, even if it doesn't *use* the > second argument given. I've updated the text to reflect exactly that, it did require updating. ;) > However I'm also curious why you would *specifically* make it *not* support function > loading? > The current implementation should work unmodified, once the signature is changed to accept an > int as the second parameter (and move the current 2nd parameter to 3rd), There is nothing > "class specific" in the existing implementation except for a couple of variable names. I mostly decided not to support it to avoid the inevitable bikeshedding of how a "default function autoloader" would work. I really want to push that to a separate RFC, unless there was a general consensus of an implementation. If we are fine reusing the existing default class autoloading, then I am fine with that. > And lastly I think you'd need to adapt spl_autoload_functions[2] too - perhaps the same as > the others, introduce a parameter int $type = SPL_AUTOLOAD_CLASS, so > existing code works as-is, otherwise it'd be impossible to know how a given autoloader was > registered. I've also added those functions as well. — Rob