-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Update syntaxerror highlight from CPython v3.10.5 #3870
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Welcome to RustPython, and thanks for volunteering your time and effort. There is an issue with the CPython test suite that you may or may not have noticed by now.
"end_lineno" => ctx.none(), | ||
"offset" => ctx.none(), | ||
"end_offset" => ctx.none(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suspect that the positioning of these is currently incompatible with some of the exceptions that are derived from SyntaxError
. On your local machine, can you run the following command and figure out why the test suite is hanging?
cargo run --release --features ssl,jit -- -m test -u all --slowest --fail-env-changed -v
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your help in solving the problem.
In fact, it was not confirmed whether the test file was runable because it was a pull request only until the commit where the test file was just copied.
I added the expectedFailure tag and skip tag to make test runable, and re-commit
The biggest problem is that the test_no_hang_on_context_chain_cycle1,2,3 test (which are all new) in Lib/test/test_exception.py is stuck in an infinite loop, so the test suite have not ended.
(Still don't know why they are stuck, so I added skip tags)
I'm reviewing the entire test to check if there are any other problems, so I'll leave a comment again when the results come out.
I didn't know that the order of attributes could be a problem. Could you explain a little bit more about the mechanism that could be a problem?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't know that the order of attributes could be a problem. Could you explain a little bit more about the mechanism that could be a problem?
My mistake. I was caught off guard by the Unicode-related errors, as they have similarly-named attributes.
I'd recommend taking a look at the definition of fn new_syntax_error()
(in vm/src/vm/vm_new.rs
) as well as where that method is invoked. (You may need to reach into RustPython's compiler
crate to implement those new attributes properly.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First, I checked other test on current branch and compare it to the results of current main branch and of the case when I switched order of attributes.
There are no difference of results. Hence, until now, the order of attributes seems not a big problem.
And I have concerned because there are a lot of things to change for the SyntaxError update. But thanks to you, now I know where to start. Thank you for the recommendation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the easiest way to diagnosing hanging problem is checking callstack when it hanged. I usually use debugger but there will be other ways too.
tough you may not need it right now
c4631a6
to
bf2e2db
Compare
add 'end_lineno' and 'end_offset'
bf2e2db
to
4c94439
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Though the actual functions are not implemented yet, this patch is enough to resolve blocking issue for test_traceback.py
No description provided.