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

[mypyc] Add a more detailed MRO explanation. #10293

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

rtpg
Copy link
Contributor

@rtpg rtpg commented Apr 8, 2021

(This` include #10290)

This is inspired by Rust's --explain mechanism. Instead of having huge multi-line error outputs by default, you need to pass in explain (here through setting MYPYC_EXPLAIN=1 when running mypyc) in order to get a more detailed explanation for less obvious error messages from the mypyc compilation itself.

This is more of a discussion piece at this stage, but I think that having detailed explanations for most of the mypyc errors would help a lot with debugging errors quickly in general, especially since they're more restrictive than Python's normal rues.

rtpg added 2 commits April 7, 2021 18:55
 The vtable index is only needed for when we actually go through a
 vtable lookup. I don't believe this is actually a major performance
 improvement, but it should make it clearer what's improtant for each
 branch.
 This is inspired by Rust's `--explain` mechanism. Instead of having
 huge multi-line error outputs by default, you need to pass in
 `explain` (here through setting `MYPYC_EXPLAIN=1` when running
 `mypyc`) in ordre to get a more detailed explanation for less obvious
 error messages from the mypyc compilation itself.
@JelleZijlstra
Copy link
Member

You could leverage the existing --show-error-context mypy flag perhaps.

@JukkaL
Copy link
Collaborator

JukkaL commented Apr 8, 2021

This is a reasonable idea. It's quite a big change to our error reporting philosophy, so we should discuss it in an issue first and brainstorm different approaches and other error conditions where this might be useful. It's probably not worth it just for this particular message, but if we can come up with a few more common error conditions where we could use this, it could be worthwhile.

We should probably also have a shorter summary message corresponding to each detailed message. In this case, the short message that we'd show by default could be something like this:

... error: A native class can't inherit from two non-trait native/extension classes ("Base1" and "Base2")
... note: Consider using @trait to declare some base classes as traits
... note: See https:/<some doc link>

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

Successfully merging this pull request may close these issues.

3 participants