Re: PR for minor bugfix in compact()

Ah I see, thanks for the clarification! I only did it this way because when I did my RFC for fsync, I recall one or two people mentioned they weren't keen on the bit where it could raise a warning rather than a TypeError - but I appreciate that one was a completely new function and not a change to an existing one (and it went through with the warning condition in the end, anyway). On Wed, 28 Apr 2021, 12:18 G. P. B., <george.banyard@gmail.com> wrote: > On Wed, 28 Apr 2021 at 12:12, David Gebler <davidgebler@gmail.com> wrote: > >> Hi internals, >> I've opened a PR to cause compact() to throw a TypeError if its parameters >> are not valid, which I consider to be a fix for what is effectively a bug >> whereby logical errors in user code can be silently swallowed. >> >> GPB has done an initial review and left a comment >> https://github.com/php/php-src/pull/6921#pullrequestreview-646848902M�3 >> (��Ah{"���g in >> which he suggests I open this up to the floor, so here it is, seeking >> your feedback kindly. Also if anyone can clarify what is meant by a >> warning >> "will be promoted in PHP 9", I am not familiar with what changes are >> planned for the next major version? >> >> Regards >> David >> > > Hey David, > > What I meant is that we usually don't introduce an exception without prior > warning to existing functionality. > So making this a Warning in the PHP 8 series which gets promoted to a > TypeError in PHP 9, > similarly to how most of the internal functions went from returning null + > warning to throwing a TypeError, > or many of the warnings that got promoted to ValueErrors in 8.0 because > it's a major release. > > Personally I don't mind introducing the TypeError immediately in PHP 8.1, > because compact() should be rather rare > and mostly used on an array, but others might feel differently about this. > > Best regards, > > George P. Banyard >