# Throwable patch(es) and finalizing the story - Thread: https://externals.io/message/86265 - Original archive: https://news-web.php.net/php.internals/86265 - Started by Stanislav Malyshev on 2015-05-16T20:57:52+00:00 - Messages: 7 - Reply by email to internals@lists.php.net - More for agents: search with https://externals.io/search.md?q={query} · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt --- ## #86265 — Stanislav Malyshev — 2015-05-16T20:57:52+00:00 URL: https://externals.io/message/86265#86265 Hi! I am assuming we have broad consensus about the move from BaseException to Throwable. I've looked at the patch at https://github.com/php/php-src/pull/1282 and it seems pretty close to what we want to me. The only difference is that https://wiki.php.net/rfc/throwable says Throwable is an interface and this patch makes it abstract class - which I guess makes some sense as both Exception and engine errors need the same properties - but I'm not sure if it doesn't mess something up. Sebastian, could you comment on this? Of course, other comments are welcome too. There's a question left about the naming - particularly, renaming EngineException to Error. I'm not sure I am a fan of this. I think it's better to leave it as EngineException, but I'm open to arguments here. One thing seems to be clear though - a) there's a broad support for introducing Throwable and b) we need to do this before we release PHP 7 alpha. So I think we need to start converging on something really soon. I'd like to take https://github.com/php/php-src/pull/1282 as a baseline, and once we close open questions about interface/abstract class and the name of the Error class, merge it - targeting sometime within the timeframe of the next week. If someone thinks it's impossible or should not be done, please tell. -- Stas Malyshev smalyshev@gmail.com --- ## #86273 — Yasuo Ohgaki — 2015-05-16T21:21:09+00:00 URL: https://externals.io/message/86265#86273 · In reply to: #86265 Hi All, On Sun, May 17, 2015 at 5:57 AM, Stanislav Malyshev wrote: > I am assuming we have broad consensus about the move from BaseException > to Throwable. I've looked at the patch at > I would like to add note that the name "BaseException" breaks too many apps. Someone researched this in previous thread. Regards, -- Yasuo Ohgaki yohgaki@ohgaki.net --- ## #86275 — Nikita Popov — 2015-05-16T21:56:39+00:00 URL: https://externals.io/message/86265#86275 · In reply to: #86265 On Sat, May 16, 2015 at 10:57 PM, Stanislav Malyshev wrote: > Hi! > > I am assuming we have broad consensus about the move from BaseException > to Throwable. I've looked at the patch at > https://github.com/php/php-src/pull/1282 and it seems pretty close to > what we want to me. The only difference is that > https://wiki.php.net/rfc/throwable says Throwable is an interface and > this patch makes it abstract class - which I guess makes some sense as > both Exception and engine errors need the same properties - but I'm not > sure if it doesn't mess something up. Sebastian, could you comment on > this? Of course, other comments are welcome too. > > There's a question left about the naming - particularly, renaming > EngineException to Error. I'm not sure I am a fan of this. I think it's > better to leave it as EngineException, but I'm open to arguments here. > > One thing seems to be clear though - a) there's a broad support for > introducing Throwable and b) we need to do this before we release PHP 7 > alpha. So I think we need to start converging on something really soon. > I'd like to take https://github.com/php/php-src/pull/1282 as a baseline, > and once we close open questions about interface/abstract class and the > name of the Error class, merge it - targeting sometime within the > timeframe of the next week. If someone thinks it's impossible or should > not be done, please tell. > Sorry for going a bit off-topic here, but why does this need to be resolved for the first alpha? If I'm not mistaken, we do feature freeze on first beta, not first alpha, right? I'd like to see an alpha release ASAP without waiting for the last few features to trickle in. Btw, the author of PR #1282 also has an implementation that makes Throwable an interface (with semantics similar to Traversable, so cannot be directly implemented). Nikita --- ## #86276 — Stanislav Malyshev — 2015-05-16T22:15:27+00:00 URL: https://externals.io/message/86265#86276 · In reply to: #86275 Hi! > Sorry for going a bit off-topic here, but why does this need to be > resolved for the first alpha? If I'm not mistaken, we do feature freeze Because it's a pretty deep language change and we don't want to release stuff that has such deep change pending. However, I don't really want to spend time on arguing about alpha/beta timelines but rather on getting the consensus around this change and getting through with it ASAP. > Btw, the author of PR #1282 also has an implementation that makes > Throwable an interface (with semantics similar to Traversable, so cannot > be directly implemented). Where is it? I'd like to see it. -- Stas Malyshev smalyshev@gmail.com --- ## #86284 — Sebastian Bergmann — 2015-05-17T05:07:28+00:00 URL: https://externals.io/message/86265#86284 · In reply to: #86265 > I am assuming we have broad consensus about the move from BaseException > to Throwable. I've looked at the patch at > https://github.com/php/php-src/pull/1282 and it seems pretty close to > what we want to me. The only difference is that > https://wiki.php.net/rfc/throwable says Throwable is an interface and > this patch makes it abstract class It is supposed to be an interface. It it's not then it's a mistake in the patch. I strongly believe that the hierarchy for exceptions thrown by the engine must be separate from the hierarchy for exception thrown by userland code (and "regular" extensions such as Reflection, etc.). The whole purpose of the Throwable interface is to mark objects of both hierarchies as throwable. Whether we name the base class of the hierarchy for exceptions thrown by the engine Error or EngineException, I don't really care. But Error feels more right to me than EngineException. I do care, though, about the fact that userland code (and "regular" extensions such as Reflection, etc.) must not throw engine exceptions. --- ## #86285 — Sebastian Bergmann — 2015-05-17T05:08:16+00:00 URL: https://externals.io/message/86265#86285 · In reply to: #86276 > Where is it? I'd like to see it. https://github.com/php/php-src/pull/1284 --- ## #86287 — Aaron Piotrowski — 2015-05-17T05:25:17+00:00 URL: https://externals.io/message/86265#86287 · In reply to: #86284 I tried to keep the implementation simpler in the first PR, but I think the second I submitted is much better and should be closer to what you had in mind. The second PR does not prevent user space code from instigating Error, but I do have another branch that does prevent it (same behavior as trying to write `new Closure()`) that I could merge if it is decided that the behavior is desired. Aaron Piotrowski (Sorry for originally sending this to you directly Sebastian, I’m new to the list and thought I could just hit reply.) > > > > > […] > > It is supposed to be an interface. It it's not then it's a mistake in the > patch. > > I strongly believe that the hierarchy for exceptions thrown by the > engine must be separate from the hierarchy for exception thrown by > userland code (and "regular" extensions such as Reflection, etc.). > The whole purpose of the Throwable interface is to mark objects of > both hierarchies as throwable. > > Whether we name the base class of the hierarchy for exceptions thrown > by the engine Error or EngineException, I don't really care. But Error > feels more right to me than EngineException. I do care, though, about > the fact that userland code (and "regular" extensions such as Reflection, > etc.) must not throw engine exceptions.