« first day (4917 days earlier)      last day (256 days later) » 

02:58
@LeviMorrison It's not a clone (which already should be optimized), but array data (so already optimized by COW), but as you wrote, only half the story. So before falling over the root of all evil (Knuth) let's make sense of an iterator: It's a decorator. A lot of what you mention that would not make sense, is perfectly fine for that pattern.
Nevertheless, I'd recommend you talk with NikiC about your questions. dig the archives for the original docs (IIRC the user dirs aren't memorized since last re-mirrorization) and talk with Marcus about what you coined proxy. Marcus did introduce an internal class in PHP 5 (Traversable), a concept that AFAIK Nikita re-iterated on in 7 (Throwable).
 
3 hours later…
05:38
1.71 is min available version, see https://rpms.remirepo.net/rpmphp/zoom.php?rpm=rust
and BTW current dependency is already 1.70
05:55
@LeviMorrison paste.centos.org/view/ef17afae (so 1.70 is already required)
 
5 hours later…
10:36
o/
 
2 hours later…
12:24
https://github.com/php/php-src/pull/13800 @IluTov

I like the idea!

Some thoughts:
Currently this allows to acquire references to data class props, but this defeats CoW. Should this be denied?
Do you have an idea of how this will interact with hooks? Hooks deny $o->prop[$offset]=...; will the same restriction apply when calling mutating methods? E.g. will $o->prop->append!() be allowed if $o->prop is hooked?
Since using ! is required when calling mutating methods, can we make it part of the actual method name and remove the mutating keyword? E.g. function append!()
@ArnaudLeBlanc References in data classes should be fine if they are intentional, as in arrays. I think they should not generally be a problem, except for readonly properties. In that case, initializing the property should remove any references with a concrete value.
@ArnaudLeBlanc Good question. Yes, that's indeed the case. Otherwise, the hook has no way to observe changes made to the property. This is reasonable, imo. If this is undesired, you should avoid a set hook.
@ArnaudLeBlanc Sounds reasonable. Although I'm still open to an alternative syntax, because @bwoebi was hoping to use the same syntax for macros at some point in the future.
@ArnaudLeBlanc To clarify, it's only disallowed if the property has a set hook. It's allowed if you only have a get hook.
References are also a problem for hashing, because modifying the reference changes the hash and makes the value end up in the wrong bucket. The same solution is necessary here, flattening the object graph by removing any references.
12:52
@IluTov I think they are also a problem in arrays. The issue is that you can not just store an array/data class an assume it will not be modified later by the user, as they may hold references to members of the array/data class
@ArnaudLeBlanc Right, this already breaks down. 3v4l.org/sgKV2
Really, arrays should do the same thing. It's just that it might be expensive to check the entire array recursively.
@IluTov Yes! May we take the opportunity to fix this in data classes?
@ArnaudLeBlanc It would definitely make sense to support both, or neither.
13:23
@IluTov with the same technique as in github.com/php/php-src/pull/9979, we may be able to flag arrays containing references, so that only those need to be handled
@ArnaudLeBlanc That's true. Data classes themselves have similar guarantees, in that they may not be self-recursive without references.
So, we may try to flag them accordingly as well.
Anyway, that PR will become much more useful once dynamic properties are forbidden by default! Looking forward to that day. :P
@IluTov Ha, I was thinking the same thing when I saw the email last night before bed. Marking the method rather than the callsite seems a lot cleaner. Random thought: mut modifier? A bit more verbose but doesn't claim any syntax, and could parallel if we ever introduce a pure modifier.
@Crell Where? Call-site or decl?
@IluTov Decl. public mut function whatever() - Indicates that the function will be modifying the object in some way, so the engine can take appropriate steps to pre-clone or whatever.
And then it's still called the same as any other method: $foo->whatever()
@Crell That doesn't work, as explained in the e-mail. Call-site is the primary issue.
13:32
Drat.
I thought your reply to Arnaud implied that it was possible.
I'm going to come back to a lot of emails aren't I?
@Girgias Some threads you can safely skip.
Fair enough, please tell me which ones in 10 days :')
@Crell Nope. There might be complex solutions, but at least no simple ones.
Anyway, I think I don't mind the call-site indication. After all, this is what we're hoping to correct for references at some point.
"Correct for references"?
13:36
Passing by ref and passing by value shouldn't be the "same"
@Crell array_push(&$array, ...) vs. array_push($array, ...)
It's obvious in this case, but not in others.
I always have trouble with array_slice and splice.
Yeah splice is only useful because it is byref lol
Ah. That ancient debate. :-)
I don't understand why it got removed, instead of just deprecating and removing trying to pass by-ref when the function takes a by-val argument
That may even have been before my time, or early on when I started paying attention to Internals.
13:39
Yeah it's old, and this still plagues us >.>
Anyway, I'm going to sleep. tomorrow doing a lot of walking if no rain
 
3 hours later…
16:28
@hakre No, it's a real duplication on iterator creation. ArrayIterator::__construct will call this: github.com/php/php-src/blame/…
Note that in userland, the refcount of the array is effectively never 1, so it duplicates.
For instance the refcount here is 3:
<?php
$array = [1, 2, 4];
$iter = new ArrayIterator($array);
?>
If you used a literal, it would still be 2.
@RemiCollet Awesome, thank you!
16:49
public ArrayIterator::__construct(array|object $array = [], int $flags = 0)
The fact this signature has flags and allows for an object should also clue you into the fact that this class is doing way too many things.
 
5 hours later…
21:20
@LeviMorrison No idea if this was ever thought about, what if zend_array_dup has a flag to not increase the refcount ... apart from that there is also recursive iterator on top of iterator. would it be of benefit if I write an example that brings all three together in PHP userland? I have some poster case in mind which IMHO should not be affected (basically JSON.parse() reviver impementation on top of SPL).
@LeviMorrison No, actually I don't think so. Or better: and thinking, there is use for that. Normally for PHP array you don't have $this. With recursive iteration and passing in a holder (which in PHP is easiest to construct with an array), you can obtain the decoration on nodes and leafs via recursive iterator (by default in depth first order as in JSON.parse() reviver IIRC) and therefore treat arrays from json_decode() similar to object from the same function.
This is of benefit as array is much more stable and then the actual iterator (on each level) is an ArrayObject and we now can access the array as "$this" (self) as an object. I've not done enough research but by my current understanding is, we may require the copy (refcount +1) for that.
It would be great thought, this could be abstracted away. But it's kind of risky if in the subtiles it is leaking. E.g. breaks within foreach body/continue/break/goto. Would be great thought if it would work.
22:11
@hakre It's fine to have four use-cases. I just don't think that putting all four behaviors into one class, which means penalizing performance for the main one, is a good trade-off.
certainly NikiC did run actual perf tests? e.g. if putting together an example that is at least three levels deep (plus a holder) it would mean we should get duplication on each axis and run into the problem domain.if it's within foreach only, there would be no extra overhead for userland callbacks. we should be able to see if it's worth or if restoring the original procotol (in userland, should be easy) is introducing way more overhead after removal.
Doing a foreach on an array doesn't make an ArrayIterator. It does an internal iteration.
Sure, but not the example I was writing about: recursive iteration over an array object of array('' => array(...)) where ... is at least three levels deep.
The RecursiveArrayIterator extends from ArrayIterator, but IMO it shouldn't. But yeah, it has similar edges around duplication since it inherits.
I don't remember the exact details off the top of my head, but I think it only duplicates the outer array on construction (the inner ones will just get a refcount increase). The inner arrays get duplicated when the traversal actually occurs and a new RecursiveArrayIterator object gets made.
22:26
Yes, I think so, too. But have we tested those implications? The only problem I see with the raised questions is if we call into premature optimizations or not.
Ignoring Recursive stuff, and just looking at the basic ArrayIterator, I can definitely say it's not a premature optimization to avoid duplication. Nothing in the main use case requires it. The trouble is the other use-cases do, and to simplify the implementation (which is shared across way too many objects), it just inherits the requirement.
To iterate over an array, you shouldn't have to first duplicate it. That makes no sense, because to duplicate it, you have to iterate over it already.
I think due to all the inheritance and stuff, maybe it's best to leave ArrayIterator alone and make a different one but... what do you name it? Where do you put it?
Well, this is certainly true if you focus on that one aspect only. But as we already found out, there is a connection with the array iterator and henceforth with the recursive array iterator and If I got you right in the ongoing conversation, the copy only applies to the first instance being decorated, not the later during (recursive) iteration (decoration).
Nikita didn't like my Spl\ForwardArrayIterator idea.
I must admit I don't know it, so if you could share a link would be great.
22:37
you give me much to read at late time. but I'll read it and try to understand. just a question about those recursive parts, are they in or have you skipped the in your mental model?
@hakre I don't touch recursive stuff in the PR. Just adds new, simple iterators for basic use cases (iterating over an array forwards, and reverse).
how is that not recursive?
  1 <?php
  2 $array = [
  3     "one" => [0, 1, 2, 3],
  4     "two" => [0, 2, 4, 6],
  5     "three" => [0, 3, 6, 9],
  6 ];
  7
  8 $iterator = new Spl\ForwardArrayIterator($array);
  9 foreach ($iterator as $key => $current) {
 10     echo "$key\n";
 11 }
 12 /* outputs:
 13 one
 14 two
 15 three
 16 */
It doesn't visit the sub-arrays.
$current, the main subject for the foreach visits those.
No, the loop has 3 iterations.
$current will be [0, 1, 2, 3] on the first iteration, [0, 2, 4, 6] on the second, and [0, 3, 6, 9] on the third.
22:51
okay, you say, the iterator implementation does not visit those, true.
the foreach does. that's the reason why the decoration of the array has been chosen.
I don't understand what you are getting at, I guess.
you focus on the implementation, I focus on the use. (if we want to personalize things, don't take this too personal)
E.g. with your example, I would use foreach on $iterator to expose $current (visit it within the body of the foreach).
You could nest foreach, yes. With this implementation, you don't get duplication and instead you get regular copy-on-write. This is unlike spl's ArrayIterator, which would unnecessarily and aggressively duplicate it.
No, I don't want to nest foreach. I prefer it straight foward, therefhore $iterator needs to decorate even nested structures so that I can visit them with foreach. Otherwise we would waste abstractions for nothing.
You could build such an iterator using the ForwardArrayIterator and the RecursiveIterator interface.
23:03
I could also extend ArrayObject and set the iterator class IIRC.
It's perhaps how I'd do it (while the extension would perhaps not be necessary but a feature, no idea), nevertheless, the same example should also be done for your suggestion. I'd need to checkout the PR branch and compile it, should be feasible.

« first day (4917 days earlier)      last day (256 days later) »