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

Exception groups and except* #10

Open
wants to merge 21 commits into
base: main_old
Choose a base branch
from
Open

Conversation

iritkatriel
Copy link
Owner

@iritkatriel iritkatriel commented Feb 11, 2021

TODO:

  • Block except *(TypeError, ExceptionGroup)
  • Split to BaseExceptionGroup(BaseException) and ExceptionGroup(BaseExceptionGroup, Exception)
  • Make (Base)ExceptionGroup immutable

@iritkatriel iritkatriel changed the title Exception group stage5 Exception groups and except* Feb 13, 2021
@stestagg
Copy link

Hi!

On 58450cb, the following code:

try:
    1/0
except *Exception:
    print("Something went wrong")

Gives me the following traceback:

XXX lineno: 4, opcode: 47
ExceptionGroup
This exception has 1 sub-exceptions:
 ----------------- 1/1 -----------------
 Traceback (most recent call last):
   File "/home/sstagg/tmp/fuzztest/cpython/td.py", line 2, in <module>
     1/0
 ZeroDivisionError: division by zero
 ----------------- end of 1 -----------------

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/sstagg/tmp/fuzztest/cpython/td.py", line 4, in <module>
    print("Something went wrong")
SystemError: unknown opcode

This is just an FYI, to record the error. Please ignore if you know about it already!

Thanks

@iritkatriel
Copy link
Owner Author

I don't get that. Can you try a clean build? Or maybe bump the MAGIC NUMBER in Lib/importlib/_bootstrap_external.py and then a clean build?

@stestagg
Copy link

Hi

I've put together a Dockerfile that reproduces the issue. Can't see anything funky in what it's doing:

https://gist.github.com/stestagg/7c7094365ff328c5558c3d0ea4213099

But consistenly get unknown opcode.

It uses clang, gcc gives the same result, only slower.
I know the magic number bump is pointless in docker, but included it just for completeness.

uname:
Linux obsidian 5.10.16-arch1-1 #1 SMP PREEMPT Sat, 13 Feb 2021 20:50:18 +0000 x86_64 GNU/Linux

Thanks!

@iritkatriel
Copy link
Owner Author

Thanks, could be a system dependent thing I'm missing (I'm developing on windows).

@iritkatriel iritkatriel force-pushed the exceptionGroup-stage5 branch 3 times, most recently from ec3671f to cdcc93a Compare March 9, 2021 14:41
@stestagg
Copy link

stestagg commented Mar 15, 2021

The above issue I raised still exists on the latest version of the code.

I took a look, and it seems like you may need to run
make regen-opcode-targets
and commit the result.

This is because supported compilers (I'm guessing not vc) will use a fast-path vtable for opcodes that bypasses the big switch statement dispatch in ceval.c (hence the TARGET() macro). This has to be rebuilt for new opcodes.

You can see the difference in opcode_targets.h, the 47th entry is currently _unknown_opcode, and running the above changes it to this:

@@ -46,7 +46,7 @@ static void *opcode_targets[256] = {
     &&_unknown_opcode,
     &&_unknown_opcode,
     &&_unknown_opcode,
-    &&_unknown_opcode,
+    &&TARGET_RERAISE_STAR,
     &&TARGET_RERAISE,
     &&TARGET_WITH_EXCEPT_START,
     &&TARGET_GET_AITER,

Thanks

@iritkatriel
Copy link
Owner Author

The above issue I raised still exists on the latest version of the code.

I took a look, and it seems like you may need to run
make regen-opcode-targets
and commit the result.

This is because supported compilers (I'm guessing not vc) will use a fast-path vtable for opcodes that bypasses the big switch statement dispatch in ceval.c (hence the TARGET() macro). This has to be rebuilt for new opcodes.

You can see the difference in opcode_targets.h, the 47th entry is currently _unknown_opcode, and running the above changes it to this:

@@ -46,7 +46,7 @@ static void *opcode_targets[256] = {
     &&_unknown_opcode,
     &&_unknown_opcode,
     &&_unknown_opcode,
-    &&_unknown_opcode,
+    &&TARGET_RERAISE_STAR,
     &&TARGET_RERAISE,
     &&TARGET_WITH_EXCEPT_START,
     &&TARGET_GET_AITER,

Thanks

I see, thank you. I will do this once I finish some changes I'm making.

@github-actions
Copy link

This PR is stale because it has been open for 30 days with no activity.

@github-actions github-actions bot added the stale label Apr 19, 2021
iritkatriel added a commit that referenced this pull request Jul 4, 2022
iritkatriel added a commit that referenced this pull request Jul 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants