-
Notifications
You must be signed in to change notification settings - Fork 423
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
Throwing initializers don't support throw
code inline
#26438
Comments
Note that it may make sense to get this right in dyno rather than fixing it in the production compiler, at least if it isn't needed by users / us before then. |
bradcray
added a commit
that referenced
this issue
Dec 20, 2024
[reviewed by @e-kayrakli and @cassella] This PR takes a first step toward throwing `postinit()` routines, similar to the current throwing support we have for `init()`. Specifically, it permits `postinit()` routines to be declared `throws` and to throw errors, which allows the user code to catch and respond to these errors at the `new myType()` call. The motivation for this was a desire to use richer throwing initializers in Jade's Python module work, where throwing postinit()s were a reasonable approach. Something this PR does not do, but that we ultimately need to, is to call the de-initializers of the fields that have been initialized in addition to deleting the memory that was allocated. This is behavior that's shared with the current throwing initializer implementation, and this PR adds some futures to demonstrate this lack in both `init()` and `postinit()` cases. The approach taken here is to: * remove the check that prevented throwing postinits * update the compiler-generated function representing the 'new' call to make it throw if its postinit call does * update compiler-generated postinit() functions to make them throw if the superclass postinit() they call does * disable `override` checking for postinit() routines which are not subject to `override` (they are not virtually dispatched; each class has one and explicitly or implicity, through the compiler, calls its parent class's postinit() In addition, the PR adds a new `postinit` field to the `AggregateType` type to simplify referring back to it throughout compilation, which we didn't seem to have a way of doing previously. This PR enables a postinit-focused throwing.chpl test to be de-futurized. I've also added several tests to lock in that the superclass/subclass behavior is correct if one or the other is declared throwing vs. not or not declared at all. In addition, I added a few futures and tests. Beyond the aforementioned futures showing that [field deinitializers are not called](#26437), I added one that shows that throwing initializers support calls to throwing procedures after their `init this;` but [don't support code that throws directly](#26438). This may be a known limitation, but I didn't see a mention of it in the documentation, so filed a future for it and will open an issue.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
In working on #26430 I found that initializers that throw can do so by calling a procedure that throws, but not by using
throw
statements themselves directly (or at least, the ones I tried). Support for throwing initializers is a WIP and it may be that this is a known limitation of the current implementation, but I couldn't find a mention of it in the technote about throwing initializers, so wanted to capture the issue here.It seems as though the implementation of this could be fairly straightforward:
init this;
into a nested procedure that throws, replacing it with a call to that proceduretest/classes/initializers/errors/errHandling/throwWithoutHelper.chpl
#26430The text was updated successfully, but these errors were encountered: