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

From: Date: Tue, 12 Mar 2024 07:19:16 +0000
Subject: Re: [Pre-RFC] Improve language coherence for the behaviour of offsets and containers
References: 1 2 3  Groups: php.internals 
Request: Send a blank email to internals+get-122620@lists.php.net to get a copy of this message
On Tue, Mar 12, 2024, at 02:36, Gina P. Banyard wrote: > On Monday, 11 March 2024 at 16:11, Larry Garfield <larry@garfieldtech.com> wrote: > > > > > 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 open to naming changes, but "dimension" is very much the standard and ubiquitous > term for this, see multidimensional arrays, n-dimensional array, etc. > > > > > * 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. > > I will answer both points together. > The reason that checking an offset exists is combined with being able to read it is that it > just makes sense to be together. > If not and only the "read" operation is supported, how do you know that an offset > actually exists before accessing it? You just access it and get an Exception that you need to catch? > It is also required for the null coalesce operator to function properly. > > What offsetGet() does if the offset doesn't exist is up to the implementation, you could > return null as a default value or throw an exception. > The only expectation is that *if* the offset exists, then reading the value should be possible. > > If you can only write to a container, then being able to check something exists is somewhat > meaningless. > > > > > > * 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. > > The former is actually also more related to iterable, as most array functions need to be able > to traverse the container to be able to do anything meaningful with it. > Or there would need to be a way to provide a "manifest" of what offsets are set for > things like array_search or array_flip. > > > > > * 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. > > > > * 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. > > Move around a later point to respond to them together. > Internal objects don't actually magically implement Stringable, this is something internal > objects must do themselves. > Moreover, internals objects can support string casts without implementing __toString(), see the > GMP object which is the only example of this in php-src (and I should fix this or if someone else > wants an easy PR to do feel free). > > To understand how the shim works, I first need to explain how interfaces being implemented in a > class work. > > Internal interfaces can have a special handler called interface_gets_implemented which gets > called during compilation of classes. > This is the mechanism used by the Throwable and the DateTimeInterface interfaces to prevent > userland classes from implementing them. > The ArrayAccess interface has (and all the other Dimension ones actually have) such a handler, > and the "shim" is to set the append, fetch, and fetch-append dimension handlers on the CE > to magically support those operations on the class for the time being. > No methods are created on the class, the new interfaces are not implemented. > To override the behaviour of append/fetch/fetch-append the relevant new interface should be > implemented. > > This is conceived as a temporary measure to ease migration for userland classes that support > those operations already. > > > > > > * 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.) > > > > I find having too many examples makes RFCs difficult to read and parse, and I prefer to use > them sparingly for when they are really needed. > Just for clarity but $a[1][2] is only a by-ref fetch for write operations, if it is a read > operation those are performed in sequence. > Fetch-append on null is not really weird, it just appends the given value (/null if no > assignments happens) and provides a reference to it. > See https://3v4l.org/UPg3P For what it’s worth, I think having some short one-liner examples sprinkled throughout the RFC is a good thing here. I had to constantly scroll back up to remind myself what each operation actually was in code. There are a lot of familiar concepts, just with new and particular language; having examples along the way to remind the reader what this new language resolves to would help, not hinder. — Rob

« previous php.internals (#122620) next »