2016-05-24 17:32 GMT+02:00 Rasmus Schultz <ras...@mindplay.dk>: > > it would be more logical to collect the trace in the ZEND_THROW Opcode > instead of in the create_handler of the Exception class > > I've been pondering a means of doing this with relatively few BC issues. > > There are four use-cases right now: > > 1. Direct: throw new Exception() > 2. Indirect: throw $factory->createException() > 3. Re-throwing: throw $exception > 4. Not throwing: $exception = new Exception() >
You missed the "5. Throw later" case, e.g. for coroutines using Generator::throw. http://amphp.org/docs/amp/managing-concurrency.html#generators Regards, Niklas > The first use-case (throw new) makes up the large majority, and would > be unaffected by this change. > > The second use-case (indirect throw, factory) and third use-case > (re-throwing) are both broken as-is - in either case, it isn't > reporting the correct stack-trace now. This change corrects those > cases. Even if that's a minor BC break, it corrects an error. > > By my analysis, that leaves only the 4th case (not throwing) as a > potentially serious BC break. If somebody is presently constructing an > Exception just as a means of extracting a stack-trace, this would no > longer work. But there is a more direct way of doing that, which is > debug_backtrace(), so likely the number of cases of this in the wild > are few, and easy to correct, and likely with backwards-compatibility > of the corrected code in all cases. > > I can't think of any other case where this change affects anything, > but please correct me and point out any other case you can think of? > > As for Exception::getTrace(), I would propose the signature be changed > to Exception::getTrace($index = 0) which would be backwards compatible > in API terms - the default argument of 0 for $index would return the > most recent stack trace, whereas an index of 1 would return the > previous stack trace, e.g. after a re-throw. > > Exception::getTraceAsString() could either change in the same way, or > render out all collected traces (which might be a bit much) or could > simply render the most recently collected trace - I'm not sure which > is best or most compatible. > > Exception::getLine() could either return the line-number of the first > or last collected stack-trace - returning the first collected > line-number is more compatible, consistent with "return new" and > re-throwing, but returning the last line-number might be more correct, > I'm not sure. > > Would a change like this require an RFC (and a vote) or is it arguably > a bug? (I'm guessing this change is too substantial to be considered a > "bug fix"?) > > > On Mon, May 23, 2016 at 10:12 AM, Julien Pauli <jpa...@php.net> wrote: > > On Sat, May 21, 2016 at 8:16 PM, Rasmus Schultz <ras...@mindplay.dk> > wrote: > >> Alright, so forget my comparison with other languages. > >> > >> My other points remain: > >> > >> Presently, "throw new" is treated as though it was one statement. > >> That's not the case. We have deferred throws and factory methods for > >> exceptions, and we have re-throws, so collecting the stack-trace at > >> construction time doesn't work. > >> > >> The construction site would only be relevant if "throw new" was in > >> deed a single statement. > >> > >> Recording the actual throw site is clearly the goal - the current > >> implementation is betting on "throw" and "new" happening at the same > >> site, which is merely circumstance. > >> > >> Ideally, an Exception should collect another stack trace for each > >> successive throw, which would enable you to trace not only the > >> original site, but the flow through any exception-handlers that might > >> have re-thrown the same Exception. > >> > >> As is, there is no information collected on throw, and thereby no > >> evidence or record of possible re-throws - on top of the fact that you > >> may be collecting and looking at bogus stack-traces from factory > >> methods or exception mappers. > >> > >> > >> On Fri, May 20, 2016 at 11:06 AM, Rowan Collins < > rowan.coll...@gmail.com> wrote: > >>> On 20/05/2016 08:22, Niklas Keller wrote: > >>>> > >>>> 2016-05-20 4:13 GMT+02:00 Jesse Schalken <m...@jesseschalken.com>: > >>>>> > >>>>> The top frame is the construction (get_error) and the site of the > throw > >>>>> (do_throw) doesn't appear in the stack at all. > >>>>> > >>>> > >>>> The comparison with JavaScript isn't a good one, since you can throw > >>>> everything in JS. If they didn't provide the stack trace upon throw, > you > >>>> would not have a stack trace at all if you throw a plain string. > >>>> > >>> > >>> > >>> That explanation justifies completely the opposite behaviour to what > Jesse > >>> described. > >>> > >>> According to MDN [1] the "stack" property is completley > unstandardised, and > >>> some engines may indeed populate it on throw, but there's no hint on > that > >>> page that they'll attach it to anything not constructed as an Error. > >>> > >>> So it's not a great comparison for either side (note that it was > originally > >>> brought up by Rasmus as an example where it *does* come from the throw > site) > >>> because the language doesn't actually guarantee you a stack trace at > all. > >>> > >>> [1] > >>> > https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error/stack > >>> > >>> Regards, > >>> -- > >>> Rowan Collins > >>> [IMSoP] > >>> > >>> -- > >>> PHP Internals - PHP Runtime Development Mailing List > >>> To unsubscribe, visit: http://www.php.net/unsub.php > >>> > >> > >> -- > >> PHP Internals - PHP Runtime Development Mailing List > >> To unsubscribe, visit: http://www.php.net/unsub.php > >> > > > > Hi. > > > > I explained that in my article detailing Exceptions from internal , > > http://jpauli.github.io/2015/04/09/exceptional-php.html > > > > I admit it would be more logical to collect the trace in the > > ZEND_THROW Opcode instead of in the create_handler of the Exception > > class. > > > > That would break backtraces, but we already broke them in PHP 7.0 > > So we could think about it for a 8.0 ideally , or a 7.1 ; I'm not > > sure. Anyway, that needs more debate ;-) > > > > > > > > Julien.Pauli > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: http://www.php.net/unsub.php > >
