Skip to content

Internal errors in third pass go uncaught #1341

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

Closed
gnprice opened this issue Apr 7, 2016 · 3 comments
Closed

Internal errors in third pass go uncaught #1341

gnprice opened this issue Apr 7, 2016 · 3 comments
Labels

Comments

@gnprice
Copy link
Collaborator

gnprice commented Apr 7, 2016

We have a nice report_internal_error mechanism for catching unexpected exceptions and reporting them explicitly as internal errors with the source file and line number that triggered the issue.

In the crash reported as #1319, that mechanism didn't fire. That was in the third pass of semantic analysis, and from the code it looks like we invoke that mechanism around the main (second) pass of semantic analysis and the type checker itself, but not the third pass.

Because it seems somewhat tricky to get this error reporting into every context where we know a specific line number, it might be helpful, in addition to any fix specifically for ThirdPass, to put a backstop layer of reporting that makes sure we at least identify the file. (As #1319 illustrates, it can be tricky to identify that from the outside!) It looks like the State.process call in the main build loop in BuildManager.process could be an effective spot for that.

@JukkaL
Copy link
Collaborator

JukkaL commented Apr 7, 2016

A good idea!

@JukkaL JukkaL added the feature label Apr 7, 2016
@gvanrossum gvanrossum added this to the 0.3.2 milestone Apr 7, 2016
@gvanrossum
Copy link
Member

Doing this to State.process() would require me to rewrite the logic once incremental lands, so I propose to wait for that (it's also targeted for 0.3.2).

@gnprice
Copy link
Collaborator Author

gnprice commented Apr 8, 2016

Sounds good. And thanks for the fast fix!

On Thu, Apr 7, 2016 at 11:15 AM, Guido van Rossum [email protected]
wrote:

Doing this to State.process() would require me to rewrite the logic once
incremental lands, so I propose to wait for that (it's also targeted for
0.3.2).


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#1341 (comment)

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

No branches or pull requests

3 participants