-
Notifications
You must be signed in to change notification settings - Fork 0
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
Labeled Exceptions for pegen - Better error-reporting in CPython #123
Comments
Hi @lysnikolaou, I've slept on it and I think I understand better now. I'm changing terminology on you to make things clearer to myself (and hopefully to other Pythonistas not familiar with the terminology from the paper).
How much does this match your proposal? It seems I have some open questions regarding whether a thrown exception can by cleared by a successful match (without a catch operator) at an outer level. I also am not sure whether it ever makes sense to have a regular alternative following a catch operator. Finally, I worry that the proposed spelling of the operators may be confusing. We use |
Hey hey @gvanrossum, first of all thanks for taking the time to go through this and making all the improvements in terminology. Indeed, most of the operators are easier to digest using your names. Most of what you say is right, except for a few cases. Let me go through all the points, where my understanding of things differs from yours:
The analogy to a First of all, remember that the rule for
Let me re-order the alternatives (for the purposes of this example):
Now we annotate the
Going back to the
I hope this example makes the whole thing a bit more clear.
I'm not sure if I understand you correctly, but we currently use Some more remarks:
Exactly right!
because we can always do this, which would be the same thing:
Anyway, I don't think this is particularly important, so we can just disregard it altogether.
That is a great idea! We can indeed use these mechanisms to implement error recovery as well. |
Whoops, you're right, I misremembered our cut notation. I don't know what I was thinking. Your example has clarified things a lot, but now I have new questions.
Fascinating stuff! |
You're raising some very interesting points that are mostly up for discussion.
I did some thinking about this, before writing up the proposal. I also considered using
The idea was that we could catch the
That's a very very good question, which I haven't given much though, though I should. I'd really like to hear your opinions on this and also @pablogsal's. The way I see it, we really ought to go with the first of your alternatives (alternative with no caught exceptions is still being tried out). This seems like the most non-intrusive thing to do. The
Good point! We would have to use exceptions together with the cut operator in this case, in order to completely skip the third alternative:
Great idea! These blog posts and the way you were presenting PEG features using Python code made it easy to follow and understand everything. I could try and do something like that, but probably not before early next week. I could even try to write a small blog post for a change, which I haven't done and seems like a nice challenge.
Indeed! I'm really excited about this! |
I've been thinking more and more about this and I came to realise that allowing Let's consider the following rule we use to distinguish valid and invalid targets:
This currently accepts valid targets and disregards invalid ones, but we would really like it to be able to report the type of the invalid target, in order for the error message to be more descriptive. With the current design, we would do something like this:
That is currently not possible though, because But if we were able to do something like this (I'm omitting the actions, which would need a bit more work):
we would be able to unset the exception Another thought that's been on my mind, since I started working on this, is that it would somewhat significantly raise the bar for new contributors. I don't really know how much of a concern this should be (I don't think there are very many people that aspire to contribute to the parser, when they're starting out), but I'd like to hear your opinion on this. Another update: I opened lysnikolaou/lepegen, which serves as a "playground" where I'm trying stuff out with a Python hand-written parser, like @gvanrossum suggested above. Take a look at it, if you want, and feel free to push anything you want to it. |
Yeah, it makes sense to be able to throw exceptions from any point. Consider an alternative (the rule of which it is part doesn't matter):
We translate roughly that into code like this (skipping
Now we add a throw operator; there's no action:
I think the translation will be:
I'm assuming that we're not allowing to catch the error in the same rule, although maybe it doesn't even matter. Now some other rule (that directly or indirectly calls the rule containing the above alternative) can catch the exception. That rule's grammar has an alternative:
This could translate into something like:
I notice in your toy implementation (which I can't git-push to?) that throwing an exception basically erases any other exception that had previously been thrown, and catching an exception clears the pending exception state completely. I would like to propose to use a stack here, where throwing an exception pushes it onto the stack, and catching it removes it from the stack. Oh, and catching something that's present deeper in the stack should pop anything that's on top of it. Something like this (omitting the OO stuff):
What that should do when catching two errors that are both in the stack I don't know -- probably pop until a match on either one, but we should decide by considering an example and seeing what's the most useful. There's also the question of what to do if the same error occurs multiple times on the stack -- the above code only pops until the one nearest the stack top is revealed, but we could also consider popping until it's no longer found anywhere on the stack. An implication of my proposed behavior is that if One thought: If FWIW, I'm sticking to your notation for now, but I'm itching to write it more verbosely, e.g.
and
just so it's clear what's what. (Another possible set of terminology: fail and recover?) We also need to look into what we should report if multiple exceptions are pending on the stack. I guess we should report the error found at the very bottom of the stack? Because in the stack model that's the oldest (unrecovered) one. Maybe I should try to read that paper. :-) |
PS. Could throw just be a function called in the action? There can't be any other action when we throw. |
Huh, skimming the paper I realized that our current strategy of reporting an error at the farthest point the tokenizer has reached is the original approach to error reporting by Ford. I still have to understand the semantics the paper proposes for throw and catch though... |
Whoops, forgot to invite you and @pablogsal as collaborators. It's done now. |
I have an important question (I think). If we have a grammar like this:
And we parse this input:
When this is parsed, the first alternative ( Question: does the thrown error cause the second alternative for If the answer is yes, I have a follow-up question. Suppose the
This would fail on the input If the answer to the first question is no (i.e., if There's a minor follow-up question in this case: can an explicit throw be caught by a catch it the same rule? E.g.
@lysnikolaou Can you find the answers to my questions in the paper? There's a final edge case that I really hope is ruled out by the answers. Suppose we have this grammar:
And let's try this on the input
Suppose the answer to my first question was yes -- then the error thrown by All in all I am hoping the answer to my first question is "no" -- it would make it much simpler to understand the semantics. |
These are all very good question I've been pondering with in the last days.
First of all, if I understand everything correctly, the paper answers none of these questions. And that's because the paper does not propose the What we will have to do is decide on what the default behaviour should be, if an ordered choice operator is not followed by a
If we choose 1, then the answer to your question is yes. If we choose 2, it's no. I agree that 2 would have the clearest semantics of the three, but that would also include much more work for everything to work correctly, since we would have to include more |
I think I've figured out what the paper does. Labels are propagated unless caught. A "default" failure throws a special label So now I have my answers:
This makes me happy. |
There are some other details I glanced from the paper as well:
Here, if |
One comment from the paper worries me.
Assuming that even Typed Lua has a simpler grammar than Python, this worries me. And IIUC they don't catch any labels -- they only throw* them. At least the example in the paper doesn't. And it seems that catching labels is only useful once you want to do error recovery. I just looked at python#19911 ("produce specialised errors for del") and ISTM that we probably would have to do the same amount of work -- the only potential saving would be defining the Which is why I reviewed PR 19911 and said I'd approve and merge it once the conflict is resolved. |
Well, it seems that my understanding of the paper wasn't deep enough, although I had read it multiple times. Thanks for reading it through and clarifying all this stuff. It is very useful.
Indeed, this is somewhat worrying. It is certainly going to be lots of work and every time we discuss about it, it is even more apparent to me that it will need an exceptional amount of beforehand thinking on what labels to introduce, where to throw and where to catch them.
That is correct. I guess the most important question that we need to ask ourselves is this: Do we want to go through with it, in case we want to also implement error recovery in the future and maybe target Python 3.10? If not, we would need to do something similar to python#19911 for |
In this paper they mention something on those lines:
Seems that with actions, lookaheads, cuts and potentially with labels the grammar can be more challenging to read :S
Yeah, this is what worries me the most. Some times it is somehow acceptable, but some others is completely off :( This also connects with the problem of displaying the grammar in python#19969. |
I'm not a language jockey, merely a moderately experienced user. Here's my take writing code interpreted by Python. Call it the 10,000 foot user view.
NO, you don't. You need to hunt down and kill every occurrence of the SyntaxError: invalid syntax message, replacing each with a coherent detailed error message that says what the interpreter finds invalid. Every time that message pops up, you have the person at the terminal having to parse the line and area around it (lest the error was caused by previous line) from scratch. Consider:
Pardon me, but In short, don't say there is a problem, say what the problem is. Anything less costs the the developer (your end user) time. |
Sadly, is not that simple. The parser does not know that that is the thing you know is missing and the parser indeed can expect many other things there. For instance, take a look at this issue: https://bugs.python.org/issue40599 In there we explore some way for the parser to communicate what it was expecting and as you can see the errors don't get much better and some times it can be misleading. |
This is not news. 90% of programs are often error checking/reporting. I did scan https://bugs.python.org/issue40599. I'll note those more verbose messages, even the ones out in left field, give me more of a hint than the bare syntax error message. Quoting that issue:
That's only a good attitude if the interpreter authors are also the only end users. And they are not. To be plain: There is no beauty in that error message for an end user. When my spouse is working in Python as she fools around with the Google Voice Kit on a Raspberry Pi, she does not know or care how the parser works. (She's MIT Alum, but Mat Sci, not Comp Sci.) And the interpreter must generate informative messages both for her and for high freshmen. Here's another take on that error from a different interpreter:
That's from The Python team needs stop saying it "is not that simple". What needs to said is: Even though the language is more complex today, the Interpreter *will* issue messages as good (if not far better) as code from 35 years ago. |
Unfortunately, we are all volunteers here doing the best we can in our free time with no sponsoring of any kind. Python, on the other hand, is maintained by unpaid volunteers and the source is not only open but anyone can contribute. If someone thinks is fundamental that Python has much better error messages, is welcomed to contribute so we can have much better error messages. Telling unpaid volunteers that they need to do something is sadly not very encouraging and certainly not the best way of convincing. |
Paid versus OSS doesn't always make it better.
In any case, the topic of this defect is not software for hire versus OSS software, but rather Better error-reporting in CPython. I've offered the input of an experienced user. Do with it what you will. |
Thanks, we certainly will take into account. |
Even though I can understand the intention behind @swhobbit’s comment, I fear that it is unrealistic. The simple reason is that in order to help in case of a syntax error, the parser would need to know exactly what a coder wanted to achieve in terms of the AST. One way to do this would be to find common resolutions for syntax errors. So, I am thinking more in terms of a spelling correction/machine learning that tries to anticipate what the coder wanted in specific circumstances. I doubt, though, that there’s real data available to train some ML models. Another way to enrich the parsing would be the proposed exception handling for grammars but they are based on the gut feeling of the language designers only and quite verbose. |
This is incorrect. Zero information is worse than limited information. Don't let perfect be the enemy of good. Even unique error numbers would be an improvement over what is printed now. "Mr. Scott cannot give me exact figures, Admiral, so... I will make a guess." -- Mr. Spock (Star Trek IV) |
This issue has become toxic. Closing. We'll continue the discussion elsewhere. |
This proposal is about implementing labeled exceptions as a mechanism for better
SyntaxError
reporting in CPython. I first heard about labeled exception by @pablogsal in #43 (comment), where he includes a link to https://arxiv.org/pdf/1405.6646.pdf, which is a paper that completely describes these features.Labeled exceptions in Pegen
Labeled exceptions are the closest thing to an exception-handling mechanism that a PEG parser can support. At the moment, a rule can either succeed (the input can be parsed) or fail (the input can either not be parsed or there was an error in helper code). The
labeled exceptions
mechanism introduces some grammar constructs, which allow to fail with a certain label, that can later be used to denote what kind of failure caused the parser to abort and output a corresponding error message, thus giving a better description of the error to the user.In pegen we could introduce an additional C struct that would be part of the parser state, similar to the
perrdetail
struct, that the current parser uses. It would look something like this:This struct could later be used to find out what error occurred. For this, a mapping between labels and error messages would need to be implemented as well, probably in the form of an array of
LabelMessageMapping
structs. BUT, first things first!New grammar syntax
New
throw
operatorA new
throw
operator (spelled '^') has to be implemented, which accepts a label as a parameter and causes the rule to fail, while setting an exception and the corresponding label. An example inpython.gram
could be:If the parser were to exit with the
InvalidStatement
label set, it would denote that a statement could not be parsed, but the error was not caught by one of the rules in the call tree of eithercompound_stmt_rule
orsimple_stmt_rule
. This could then be used (together with the location information) for the error message.Parametrized
bar
operatorWe also need to introduce a new parametrized
bar
operator (spelled| ^ (label1, label2) alt
). This accepts a set of labels as input. If the parametrizedbar
operator is used, the alternative following it should only be tried, in case one of the prior alternatives failed with one of the listed labels. For example:Here, we know that the rule
invalid_assignment
only checks for invalid targets and thus should only be tried out, in case the first alternative ofassignment
failed with theInvalidTarget
label. Other possible labels could beInvalidAugassign
, in case the target was parsed, but the operator following it wasn't a validaugassign
operator.Parametrized terminal nodes
Terminal nodes should optionally accept a label as an argument, which is the label that needs to be set, in case the terminal node could not be parsed. Take the following rule for the
if
statements for example:In case a colon fails to be parsed after an
if
and anamed_expression
have both been successfully parsed, it should raise an error and set theNoColorAfterIf
label, which would need to be associated with an error message likeSyntaxError: expected ':' after an if expression
.Implementation details
I think that the easiest way to do all of this is to implement code that raises exceptions in
Parser/pegen/pegen.c
, which would be called by the parser for thethrow
operator and when a terminal node cannot be parsed. The helper code would need to:p->errors.error_set
to 1.p->errors.label
to the correct label (which would probably be a parameter to the function)Later, if the parser exits with a NULL node, the helper code in
_PyPegen_run_parser
will check to see ifp->errors.error_set
is equal to 1 and, if it is, it will call code that generates the correctSyntaxError
.Also needed is a helper function that checks to see if
p->errors.label
is one of the labels in a given set. The parser would call this function for the parametrised bar operator, in order to determine whether it should go on with the current alternative or not.Conclusion and things to consider
I think that if we implement all this, we should be able to make the new PEG-based parser much better in reporting correct (location-wise) and user-friendly error messages. But there is still one thing to consider. Even if all proves to be successful, I don't think this could ever be better than a hand-written parser, which is essentially what
Python/ast.c
is. There is going to be some error messages we cannot reproduce using these mechanisms, so we will soon have to answer this: Do we want to write additional helper code to check for corner cases and reproduce those error messages or do we want to go with the genericSyntaxError: invalid syntax
message, in case we cannot do this in the grammar itself?The text was updated successfully, but these errors were encountered: