On Mon, Aug 19, 2024, at 14:03, Gina P. Banyard wrote:
> On Thursday, 15 August 2024 at 17:22, Rob Landers <rob@bottled.codes> wrote:
>> Hello internals,
>> 
>> I've decided to attempt an RFC for function autoloading. After reading 
>> hundreds of ancient (and recent) emails relating to the topic along with 
>> several abandoned RFCs from the past, and after much review, I've decided to 
>> put forth a variation of a previous RFC, as it seemed the least ambitious 
>> and the most likely to work:
>> 
>> https://wiki.php.net/rfc/function_autoloading4
>> 
>> Please let me know what you think. An implementation should be along before 
>> opening it for a vote (now that I realize how important that is).
> 
> I had a quick glance at the RFC, and I really don't think this is a good 
> approach or good API.
> Having autoloading in SPL causes dumb issues, for example ext/session must 
> register it has a dependency on ext/spl just for the autoloader. [1]
> This means that currently ext/session depends on ext/spl, ext/spl depends on 
> ext/standard, and ext/standard depends on ext/session, a glorious circular 
> dependency.
> 
> This main problem has pushed me again to rebase my PR [2] for the Core 
> Autoloading RFC. [3]
> The wording of the RFC hasn't been updated, as this is frankly my least 
> favourite part of any RFC.
> The PR passes CI and has also produced benchmark results from CI. [4]
> 
> I would rather have some collaboration on a proper solution than, IMHO, this 
> suboptimal solution.
> I still need to get opinions from other people if it makes sense to remove 
> the zend_autoload_class function pointer API and have the VM directly call 
> zend_autoload.
> Because from what I remember 2 years ago, some profiling tools hook into it 
> to track autoloading time.
> This might be improved by introducing new observer hooks.
> 
> 
> Best regards,
> 
> Gina P. Banyard
>> 
> 
> [1] https://github.com/php/php-src/pull/14544#issuecomment-2294907817
> [2] https://github.com/php/php-src/pull/8294
> [3] https://wiki.php.net/rfc/core-autoloading
> [4] https://github.com/php/php-src/actions/runs/10441267948
> 

Hey Gina,

I’d love to collaborate on this feature. For what it’s worth though, I did a 
ton of research on it (mostly reading every discussion I could find on the 
topic, and prior RFCs) and I felt that this API was the most likely to be 
accepted. You even have a comment on your PR asking why not an API similar to 
this one (!) though your reasoning is sound for why it is a bad idea, and I 
believe it is the superior API.

There are some key differences between our two RFC’s that I think are worth 
discussing (besides the obvious API differences):
 1. in your RFC, it calls the autoloader with the global function it isn’t 
found in both scopes. In mine, it calls it once not found in the local scope 
and calls the autoloader once (for the local scope). This seemed to be a highly 
liked proposal in one of the older discussions (2013-ish?) that appeared to not 
result in a new RFC, as it would bypass a lot of perceived performance issues 
in earlier RFCs. If an autoloader so desires, the autoloader can check the 
global scope (by getting the “base name” of the function), but autoloading the 
global scope should be a niche application these days.
 2. I like the “pinning” aspect. I haven’t seen your code yet, but I suspect it 
just registers the global function in the current namespace? If so, does this 
affect the __namespace__ global? Probably not, but I am just curious. What 
happens if I manually require a file with a pinned function in it?
 3. Should we update function_exists like I did in mine to include an autoload 
argument?
 4. You mention no default autoloader for classes and functions, while I agree 
that this should be the case, will the spl library still provide the default 
class autoloader that can be registered?
As far as performance for ambiguous functions go, I was thinking of submitting 
an RFC, where ambiguous function calls are tagged during compilation and always 
resolve lexically, sorta like how it works now:

echo strlen($x); // resolves to global, always
require_once "my-strlen";
echo strlen($x); // resolves to my strlen, always

This works by basically rewriting the function name once resolved and may make 
the code more predictable. If I can pull it off, it can be relegated to a 
technical change that doesn’t need an RFC. Still working on it. 

In other words, maybe pinning could be solved more generally in a future RFC, 
decrease your RFC’s scope and chance for sharp edge cases.

In any case, if you are up for a high bandwidth conversation to collaborate, we 
can join a call, collaborate in the open, or whatever you feel most comfortable 
with. I’m very excited to see this feature.

— Rob

Reply via email to