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

json module incompatible with checked exceptions #17399

Closed
arnetheduck opened this issue Mar 16, 2021 · 3 comments
Closed

json module incompatible with checked exceptions #17399

arnetheduck opened this issue Mar 16, 2021 · 3 comments

Comments

@arnetheduck
Copy link
Contributor

json module is broken with regards to checked exceptions (as is most of the std library, this is just one example) - Exception should no longer be raised per recent changes to the Exception hierarchy, yet json and many other modules provide inaccurate exception information in their API.

import json
func f() {.raises: [CatchableError].} = discard parseJson("test")
/usercode/in.nim(3, 58) Error: can raise an unlisted exception: Exception

nim 1.2.6, 1.4.4

@timotheecour
Copy link
Member

timotheecour commented Mar 17, 2021

this PR #16724 attempts to fix this and related issues but is blocked by upgrading csources nim bootstrap version in CI, because bootstrap nim can't support this.

links

@arnetheduck
Copy link
Contributor Author

the root cause in this case is that there are unannotated closure functions down the call stack - generally, the standard library doesn't reason about exception safety - just changing the default eh effect to CatchableError doesn't actually solve the issue - instead the std lib must be reviewed as to where exceptions can be raise, what needs to be released as a consequence and which callbacks and forward declarations actually should be allowed to raise exceptions.

for example, if a function calls a callback that raises, that function needs to be prepared to release resources it allocates (like files) - this rarely happens in the streams modules.

@ringabout
Copy link
Member

works on the devel branch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants