If a function has a try/catch block nested inside a try/finally, then the finally clause will not be run if the inner block throws an exception without catching it.
For instance, the following code does not run print Cleanup.
We've seen this behaviour in the wild with ndscan, which has several functions with nested try statements.
This occurs because we skip adding the cleanup block to the landing pad if the try block does not have an exception. This patch changes the logic to only skip cleanup if we have no other try blocks in scope.
I've an alternative (though slightly more complex) patch that tracks whether we have any finally blocks in scope, and uses that instead — happy to submit that if preferred.
Type of Changes
Type
✓
:bug: Bug fix
Steps (Choose relevant, delete irrelevant before submitting)
All Pull Requests
[x] Use correct spelling and grammar.
[ ] Update RELEASE_NOTES.rst if there are noteworthy changes, especially if there are changes to existing APIs.
[ ] Close/update issues.
[x] Check the copyright situation of your changes and sign off your patches (git commit --signoff, see copyright).
Code Changes
[ ] Run flake8 to check code style (follow PEP-8 style). flake8 has issues with parsing Migen/gateware code, ignore as necessary.
[ ] Test your changes or have someone test them. Mention what was tested and how.
[ ] Check, test, and update the documentation in doc/. Build documentation (nix build .#artiq-manual-html; nix build .#artiq-manual-pdf) to ensure no errors.
Git Logistics
[ ] Split your contribution into logically separate changes (git rebase --interactive). Merge/squash/fixup commits that just fix or amend previous commits. Remove unintended changes & cleanup. See tutorial.
[ ] Write short & meaningful commit messages. Review each commit for messages (git show). Format:
topic: description. < 50 characters total.
Longer description. < 70 characters per line
Licensing
See copyright & licensing for more info.
ARTIQ files that do not contain a license header are copyrighted by M-Labs Limited and are licensed under LGPLv3+.
ARTIQ Pull Request
Description of Changes
If a function has a
try/catchblock nested inside atry/finally, then thefinallyclause will not be run if the inner block throws an exception without catching it.For instance, the following code does not run print
Cleanup.We've seen this behaviour in the wild with ndscan, which has several functions with nested
trystatements.This occurs because we skip adding the cleanup block to the landing pad if the
tryblock does not have an exception. This patch changes the logic to only skip cleanup if we have no othertryblocks in scope.I've an alternative (though slightly more complex) patch that tracks whether we have any
finallyblocks in scope, and uses that instead — happy to submit that if preferred.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.Documentation Changes
nix build .#artiq-manual-html; nix build .#artiq-manual-pdf) to ensure no errors.Git Logistics
git rebase --interactive). Merge/squash/fixup commits that just fix or amend previous commits. Remove unintended changes & cleanup. See tutorial.[ ] Write short & meaningful commit messages. Review each commit for messages (
git show). Format:Licensing
See copyright & licensing for more info. ARTIQ files that do not contain a license header are copyrighted by M-Labs Limited and are licensed under LGPLv3+.