Re: [Pre-RFC] Improve language coherence for the behaviour of offsets and containers

On Mon, Mar 11, 2024, at 12:52 PM, Gina P. Banyard wrote: > Hello internals, > > I would like to get some initial feedback on an RFC I've been working > on for the last 5–6 months. > The RFC attempts to explain, and most importantly, improve the > semantics around $container[$offset] as PHP is currently widely > inconsistent. > > The RFC is about six thousands words, so I would recommend a cup of > tea/coffee before starting to read it. > > The main things which are still in flux is the naming of the new > interfaces and methods, the phrasing of error messages, and the > behaviour around using null as a container. > Those are generally indicated with a TODO comment. > > Of note is that I will also be going on holiday for 3 weeks from the > 19th of March, so ideally most of the feedback would come during that > time frame so that I can finalize the RFC after my holiday. > > RFC: > > https://github.com/Girgias/php-rfcs/blob/master/container-offset-behaviour.md > > > Best regards, > > Gina P. Banyard Woof. That's my kind of RFC. :-) The extensive background helps a lot, thank you. I am generally in favor of this, and have wanted more fine-grained ArrayAccess interfaces for a long time. Thoughts in no particular order: * "Dimension" is clearly based on the existing engine hooks, but as a user-space dev, it's a very confusing term. Possibly documentable if there's an obvious logic for it we could present, but something more self-evident is probably better. * I am not sure combining get and exists into a single interface is right. I'm not certain it's wrong, either, though. What is the argument for combining them? * Do we have some guidance for what offsetGet() should do when offsetExists() is false? I know there isn't really any now, but it seems like the sort of thing to think about here. * You sort of skirt around but don't directly address the impact of this change on the long-standing desire to make array functions accept arrayified objects. What if any impact would this have there? Eg, could some array functions be made to accept array|DimensionRead without breaking the world? * How, if at all, does this relate to iterable? I think it has no impact, but since it's in the same problem space it's worth confirming. * You mention at one point applying shim code ArrayAccess to make it work like the new interfaces. Please expand on that. Do you mean the engine would automatically insert some shims, like how __toString() now magically implies implements Stringable? Or some trait that objects could use (either a user-space trait or an engine trait) that would provide such shims in an overridable fashion? I don't fully follow here. * Big +1 to removing the magic semi-silent casting when using weird key types. * I feel like some of the sections could benefit from more short code examples. Eg, What the heck does fetch-append on a null even look like? That would help illustrate why the current behavior is weird, or why some things need to stay non-obvious because they're used in odd cases. (Like how $a[1][2] is a by-ref fetch internally, something most people don't think about.) * What would the changes to ArrayObject mean for a backing object that uses hooks on some properties? It says __get/__set would be bypassed. Would hooks also be bypassed? (I have no clue what the preferred logic here is, honestly. Just thinking aloud and hoping hooks pass so that we have to think about this. :-) ) * If I read correctly, an internal object that implements one of the dimension handlers will automagically appear to user-land as having the corresponding interface, much like __toString()/Stringable. Is that correct? It seemed implied but not fully stated. If so, a brief code example would help to make it clear in such a long RFC. Again, thank you for your work on this, and I hope it passes easily. --Larry Garfield