Hi Michał Marcin Brzuchalski, > Heads up, I plan to open the vote tomorrow morning.
Sorry for the late comment - I'd been busy/occupied with various other things and this RFC fell off my radar, and I'd assumed that anything I'd end up noticing when looking at the RFC would have already been pointed out by other reviewers. (In general, I've only reviewed or locally compiled the code of a small fraction of recent RFCs mentioned on the mailing list.) I had some comments on the implementation in https://github.com/php/php-src/pull/5820#pullrequestreview-452045885 - How much does this change elapsed time in typical use cases? This had completely skipped my mind when I saw this RFC, but I'd generally assume that stack traces aren't kept along for longer than needed to process them, and that most stack traces would be at most hundreds of frames in typical applications. So I'd consider CPU time elapsed more important than RAM. I'd had similar issues with the tradeoffs in PhpToken (and forgot). Those issues were that the applications using the array would use less memory and be faster if I used the returned array multiple times, but less so if I only processed the array once and immediately freed it. PHP also seems faster at fetching dimension from arrays than properties from objects. Still, though, this might help with a bit with developer productivity (e.g. IDEs being better at inferring types of $frame->getFile() in some cases, or providing help text for the method), or in being able to check types when passing individual frames - I found a bug: The ArrayAccess magic methods are implemented, but not the real methods. - I found some memory leaks, possibly related to tracking argument arrays in the object being stored - Casting getFile() to the empty string is unexpected for me ($frame['file'] and $frame->file are null in the implementation). - Forbidding modification/deletion on the ArrayAccess magic API (e.g. `$frame['trace'] = ...`) seems inconsistent when modification and deletion are allowed through the real properties, (and throwing there may affect code written to process the array returned by Throwable->getTrace()) - The typed property default was incompatible with the typed property declaration (`class StackFrame { string $file = null; /*...*/}` for internal stack frames) With the feature freeze nearby, more of a heads up would have been useful. Aside: The guidelines in https://wiki.php.net/rfc/howto have been followed, but I think it'd be useful for those guidelines to say "consider *at least* one days heads up mail on the mailing list" instead of "consider one days" to avoid cases where review comments are made on short notice. Normally, any issues found the day before voting starts would have more time to reschedule the vote to be addressed. - I don't know how amendments to the howto get proposed or decided on Regards, - Tyson -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
