Fix throwing non-embedded builtin exceptions - #2663
Conversation
comm_kernel currently encodes all builtin exceptions with an id of 0. However, lookup for these exceptions happens from the artiq.coredevice.exceptions module. This means if you try to throw a builtin exception not in the embedding map, then it crashes when receiving it on the host. This also prevents you catching these exceptions in kernel code. We now encode only use an exn id of 0 if the exception is in the exceptions module (and thus already in the embedding map). Signed-off-by: Jonathan Coates <jonathan.coates@oxionics.com>
|
I was under the impression that we predefined the set of builtin exceptions we would use in |
|
So However, looking at this further, I think this is more complex/broken then I initially realised. 1) When we compile a kernel:
2) When an RPC method throws, we perform the following translation:
3) When translating a kernel exception back to a host exception, we do the following:
The main problem here comes from the mismatch between 1) and 2). Not all exceptions in from builtins import AssertionError
from artiq.language.core import kernel
def throw():
assert False
@kernel
def entrypoint():
try:
throw()
except AssertionError:
print("Caught")
In fact, we don't even need RPCs for this! When the ARTIQ IR generator generates assertions, it doesn't go through the embedding map, and so also generates different exception ids: from builtins import AssertionError
from artiq.language.core import kernel
@kernel
def entrypoint():
try:
assert False # 0:AssertionError
except AssertionError: # 2:builtins.AssertionError
print("Caught")I feel the cleanest fix here would be just to drop the strings mapping and special casing of ARTIQ exceptions/builtin exceptions (outside of the initial population of the embedding map) and lookup/store the type in the embedding map directly. I don't know — not familiar enough with the history and design decisions behind the current implementation to know if there are reasons against this. |
ARTIQ Pull Request
Description of Changes
comm_kernelcurrently encodes all (Python) builtin exceptions with an id of 0. However, lookup for these exceptions happens from theartiq.coredevice.exceptionsmodule. This means if you try to throw a builtin exception not in the embedding map, then it crashes when receiving it on the host. This also prevents you catching these exceptions in kernel code.We now encode only use an exn id of 0 if the exception is in the exceptions module (and thus already in the embedding map).
I believe this is a relatively recent regression — I noticed this while in the process of updating to latest ARTIQ. This was possibly introduced in #2526, but afraid I have not bisected this.
Type of Changes
Steps (Choose relevant, delete irrelevant before submitting)
All Pull Requests
git commit --signoff, see copyright).Code Changes
flake8to check code style (follow PEP-8 style).flake8has issues with parsing Migen/gateware code, ignore as necessary.