Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error message provokes panic #5615

Closed
p6rt opened this issue Aug 25, 2016 · 15 comments
Closed

Error message provokes panic #5615

p6rt opened this issue Aug 25, 2016 · 15 comments

Comments

@p6rt
Copy link

p6rt commented Aug 25, 2016

Migrated from rt.perl.org#129088 (status was 'resolved')

Searchable as RT129088$

@p6rt
Copy link
Author

p6rt commented Aug 25, 2016

From 1parrota@gmail.com

An error in a sprintf format (program attached) generates a "panic" message​:

Your printf-style directives specify 0 arguments, but 1 argument was supplied
  in any at /home/guru/rakudo/rakudo-star-2016.07/install/share/perl6/runtime/CORE.setting.moarvm
line 1
  in any panic at
/home/guru/rakudo/rakudo-star-2016.07/install/share/nqp/lib/NQPHLL.moarvm
line 1

Apologies if it should have been reported as a moarvm bug

perl6 -v
This is Rakudo version 2016.07.1 built on MoarVM version 2016.07
implementing Perl 6.c.

@p6rt
Copy link
Author

p6rt commented Aug 25, 2016

From 1parrota@gmail.com

ch_02_01

@p6rt
Copy link
Author

p6rt commented Aug 26, 2016

From @timo

This sound much more severe than it is.

panic is just a convenience method that throws an exception that
contains the cursor's location during parsing.

it then just uses nqp​::die to throw that message, which is as harmless
as any other "die".

@p6rt
Copy link
Author

p6rt commented Aug 26, 2016

The RT System itself - Status changed from 'new' to 'open'

@p6rt
Copy link
Author

p6rt commented Aug 26, 2016

From 1parrota@gmail.com

"Panic" brings to mind "kernel panic" - perhaps there's a less
alarming way to express it; "failure", "error", or something? (Of
course, we're used to "Fatal error - aborting" meaning "something's
wrong - the program's stopping".)

Presumably " in any at" should have something between "any" and "at",
to judge from " in any panic at".

On 8/26/16, Timo Paulssen via RT <perl6-bugs-followup@​perl.org> wrote​:

This sound much more severe than it is.

panic is just a convenience method that throws an exception that
contains the cursor's location during parsing.

it then just uses nqp​::die to throw that message, which is as harmless
as any other "die".

@p6rt
Copy link
Author

p6rt commented Aug 27, 2016

From @timo

we could call the method "parsefail", or maybe just "fail", or "abort".
how does that sound?

@p6rt
Copy link
Author

p6rt commented Aug 27, 2016

From 1parrota@gmail.com

On 8/26/16, Timo Paulssen via RT <perl6-bugs-followup@​perl.org> wrote​:

we could call the method "parsefail", or maybe just "fail", or "abort".
how does that sound?

Just plain "fail" seems most appropriate.

Even the reference to the VM seems more confusing than useful to the
average user. All the message really needs to say is that the printf
mask is wrong, with optional extra awesomeness points if it can
identify where and why.

@p6rt
Copy link
Author

p6rt commented Aug 27, 2016

From @timo

On 27/08/16 17​:45, Parrot Raiser wrote​:

Even the reference to the VM seems more confusing than useful to the > average user. All the message really needs to say is that the printf >
mask is wrong, with optional extra awesomeness points if it can >
identify where and why.

Huh? What do you mean "reference to the VM"?

@p6rt
Copy link
Author

p6rt commented Aug 27, 2016

From 1parrota@gmail.com

Huh? What do you mean "reference to the VM"?

in any at /home/guru/rakudo/rakudo-star-2016.07/install/share/perl6/runtime/CORE.setting.moarvm
line 1
  in any panic at
/home/guru/rakudo/rakudo-star-2016.07/install/share/nqp/lib/NQPHLL.moarvm
line 1

moarvm is mentioned at the end of both lines, (which say nothing
useful about the bug, and could be omitted completely)

@p6rt
Copy link
Author

p6rt commented Aug 27, 2016

From @timo

are you aware that that's just the filename? and the whole thing is just
the backtrace that shows where things are going wrong?

not necessarily a very useful stacktrace; usually we'd want to have the
source file name rather than the effective filename of the bytecode
loaded. also, no useful line numbers, either. that's probably just an
artifact of that stuff living in NQP space rather than in perl6 space,
and should also see some kind of fix at some point, though.

On 27/08/16 22​:11, Parrot Raiser wrote​:

Huh? What do you mean "reference to the VM"?
in any at /home/guru/rakudo/rakudo-star-2016.07/install/share/perl6/runtime/CORE.setting.moarvm
line 1
in any panic at
/home/guru/rakudo/rakudo-star-2016.07/install/share/nqp/lib/NQPHLL.moarvm
line 1

moarvm is mentioned at the end of both lines, (which say nothing
useful about the bug, and could be omitted completely)

@p6rt
Copy link
Author

p6rt commented Aug 27, 2016

From @geekosaur

On Sat, Aug 27, 2016 at 4​:22 PM, Timo Paulssen <timo@​wakelift.de> wrote​:

are you aware that that's just the filename? and the whole thing is just
the backtrace that shows where things are going wrong?

I think you're getting an end user's-eye view of what internal backtraces
look like. (Yes, really. See this in MacPorts support too; it errs on the
side of providing copious detail in its logfiles.)

--
brandon s allbery kf8nh sine nomine associates
allbery.b@​gmail.com ballbery@​sinenomine.net
unix, openafs, kerberos, infrastructure, xmonad http://sinenomine.net

@p6rt
Copy link
Author

p6rt commented Aug 28, 2016

From 1parrota@gmail.com

I think you're getting an end user's-eye view of what internal backtraces look like.

Right! If the problem is simply a user error, (an invalid entry in an
sprintf format string), the messages are irrelevant and confusing.

I thought I was reporting some problem in the processing or creation
of the error message. In that case P6 is broken in some way, and the
messages presumably help the language's developers fix that.

On 8/27/16, Brandon Allbery via RT <perl6-bugs-followup@​perl.org> wrote​:

On Sat, Aug 27, 2016 at 4​:22 PM, Timo Paulssen <timo@​wakelift.de> wrote​:

are you aware that that's just the filename? and the whole thing is just
the backtrace that shows where things are going wrong?

I think you're getting an end user's-eye view of what internal backtraces
look like. (Yes, really. See this in MacPorts support too; it errs on the
side of providing copious detail in its logfiles.)

--
brandon s allbery kf8nh sine nomine
associates
allbery.b@​gmail.com
ballbery@​sinenomine.net
unix, openafs, kerberos, infrastructure, xmonad
http://sinenomine.net

@p6rt
Copy link
Author

p6rt commented Aug 31, 2016

From @jnthn

On Sun Aug 28 10​:16​:31 2016, 1parrota@​gmail.com wrote​:

I think you're getting an end user's-eye view of what internal
backtraces look like.

Right! If the problem is simply a user error, (an invalid entry in an
sprintf format string), the messages are irrelevant and confusing.

I thought I was reporting some problem in the processing or creation
of the error message. In that case P6 is broken in some way, and the
messages presumably help the language's developers fix that.

Improved things a good bit in dc7f27988. The sprintf format strings are parsed using a grammar in NQP. When we can't parse it, we call the panic method (a standard name used by the Perl 6 grammar also). This is all fine. What wasn't so fine is that​:

* We let that leak into the userspace backtrace, where it...well, apparently makes people panic

* We accidentally trimmed the userspace backtrace below that line due to some not so smart logic to avoid showing unhelpful stuff.

I've fixed those up so that you get a decent backtrace of where the sprintf was now and it doesn't mention the panic method - which is an implementation detail.

We should probably write some kind of test for this; I'll tag it testneeded. :-)

@p6rt
Copy link
Author

p6rt commented Sep 9, 2016

From @zoffixznet

Tests added in Raku/roast@71aab20a9d

@p6rt
Copy link
Author

p6rt commented Sep 9, 2016

@zoffixznet - Status changed from 'open' to 'resolved'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant