Skip to content

Add co_stacksize to code objects #4554

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

Merged
merged 1 commit into from
Mar 10, 2023
Merged

Conversation

howjmay
Copy link
Contributor

@howjmay howjmay commented Feb 22, 2023

Closes #4545

@fanninpm
Copy link
Contributor

I need to remind myself to update Lib/test/test_compile.py to the version from CPython 3.11.

@DimitrisJim
Copy link
Member

wow, this makes a lot of tests pass 😄

The following tests in test_compile can be marked as passing (i.e remove the # TODO: RUSTPYTHON comment and the expectedFailure decorator from the test function.

test_async_for (test.test_compile.TestStackSizeStability) ... unexpected success
test_async_for_else (test.test_compile.TestStackSizeStability) ... unexpected success
test_async_with (test.test_compile.TestStackSizeStability) ... unexpected success
test_for (test.test_compile.TestStackSizeStability) ... unexpected success
test_for_break_continue (test.test_compile.TestStackSizeStability) ... unexpected success
test_for_break_continue_inside_async_with_block (test.test_compile.TestStackSizeStability) ... unexpected success
test_for_break_continue_inside_except_block (test.test_compile.TestStackSizeStability) ... unexpected success
test_for_break_continue_inside_finally_block (test.test_compile.TestStackSizeStability) ... unexpected success
test_for_break_continue_inside_try_finally_block (test.test_compile.TestStackSizeStability) ... unexpected success
test_for_break_continue_inside_with_block (test.test_compile.TestStackSizeStability) ... unexpected success
test_for_else (test.test_compile.TestStackSizeStability) ... unexpected success
test_if (test.test_compile.TestStackSizeStability) ... unexpected success
test_if_else (test.test_compile.TestStackSizeStability) ... unexpected success
test_return_inside_async_with_block (test.test_compile.TestStackSizeStability) ... unexpected success
test_return_inside_except_block (test.test_compile.TestStackSizeStability) ... unexpected success
test_return_inside_finally_block (test.test_compile.TestStackSizeStability) ... unexpected success
test_return_inside_try_finally_block (test.test_compile.TestStackSizeStability) ... unexpected success
test_return_inside_with_block (test.test_compile.TestStackSizeStability) ... unexpected success
test_try_except_as (test.test_compile.TestStackSizeStability) ... unexpected success
test_try_except_bare (test.test_compile.TestStackSizeStability) ... unexpected success
test_try_except_qualified (test.test_compile.TestStackSizeStability) ... unexpected success
test_try_finally (test.test_compile.TestStackSizeStability) ... unexpected success
test_while_else (test.test_compile.TestStackSizeStability) ... unexpected success
test_with (test.test_compile.TestStackSizeStability) ... unexpected success

@DimitrisJim
Copy link
Member

Heya @howjmay, if there's confusion on how to do it or you don't have enough time to do so let me know and I'll fix the tests as needed.

@howjmay
Copy link
Contributor Author

howjmay commented Mar 4, 2023

Hi @DimitrisJim I was busy in the past week. I am starting to fix it once again

@howjmay
Copy link
Contributor Author

howjmay commented Mar 7, 2023

@DimitrisJim I am feel sorry that I still can't solve this. I have no idea how to get the correct stack size

@fanninpm
Copy link
Contributor

fanninpm commented Mar 7, 2023

@howjmay try going into Lib/test/test_compile.py and removing the lines that say

# TODO: RUSTPYTHON
@unittest.expectedFailure

@DimitrisJim
Copy link
Member

your previous commit (c80802d) was perfectly fine here! Like @fanninpm mentions, just go through the tests in test_compile and remove the decorators marking the functions as failing.

If the stack depth is calculated incorrectly in some cases, that's another subject. This PR just takes care of exposing it for now so don't worry about it.

@howjmay
Copy link
Contributor Author

howjmay commented Mar 8, 2023

@fanninpm @DimitrisJim Ah thanks! I was totally misunderstood. Let me fix it!

@DimitrisJim
Copy link
Member

DimitrisJim commented Mar 8, 2023

yeah, sorry if I didn't make myself clear enough. The idea here is to remove any decorators (and their preceding comment) for the tests that now pass. If you revert the changes you've made to test_compile.py and then run:

cargo run --release --features ssl,jit -- -m test -j 1 -u all --slowest --fail-env-changed -v test_compile

this will execute the test runner for this specific file. After you do that you'll notice that for some of the functions (the ones in #4554 (comment)) the test actually passed (there will be a message saying 'Unexpected success'). For these you'll just need to remove the # TODO: RUSTPYTHON comment and @unittest.expectedFailure so the test runner can now accept them as succeeding.

Hope I made it clear! Feel free to ask if something is confusing.

@howjmay
Copy link
Contributor Author

howjmay commented Mar 10, 2023

Thank you @DimitrisJim. I just pushed a commit to fix

Copy link
Member

@DimitrisJim DimitrisJim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @howjmay! Hopefully this now satisfies the test runner.

@DimitrisJim DimitrisJim merged commit 17c5361 into RustPython:main Mar 10, 2023
@howjmay
Copy link
Contributor Author

howjmay commented Mar 10, 2023

Thanks!

@howjmay howjmay deleted the co_stacksize branch March 10, 2023 17:08
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.

Add co_stacksize to code objects.
3 participants