-
Notifications
You must be signed in to change notification settings - Fork 28
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
[RFC] Generalized finalization #114
Conversation
A few thoughts on
About generalized finalization, it looks a bit heavy to me to introduce 6 new aspects for that functionality. Why not do like for aspect For types that allow exceptions in finalization, I suppose The RFC does not say which types can be finalizable. I suspect it only makes sense for objects passed by reference? Thus, we could decide that this aspect makes the type a by-reference type. And also forbid it on by-copy types. The "unresolved questions" of the RFC states that "a Finalize raising an exception will make the program abort by default` which is not what the RFC currently says. |
Note that in SPARK, |
I agree, good suggestion, I'll amend the RFC to reflect that
I don't think that's necessary, IMO
Hmm, it's true that falling out of the fact that they're tagged, every finalizable type so far in Ada is by-reference. I do think however that Rust and C++ allow by-copy finalizable types. Let me think on this.
Good catch. That was another possible alternative (corresponding to what C++ does).
@sabaird Sent me similar comments about Assertion Error vs Program Error, Ill try to explain the rationale of the choice I made here. First, for a bit of context:
What I get out of this and the Rust discussion is that
To further this point, some libraries in Ada (like LAL but not only LAL) use the pattern of exceptions being regular errors that can happen (
So, this is the context for the design in the RFC. The exact kind of the exception doesn't matter much to me, but it's very important IMO to make this "crash early" behavior alterable for production builds in an easy fashion, and assertions behave like this in Ada, which did seem like a good fit. Also, if you conceptualize No Except as a contract (and it is what it is IMO) then I really don't see the problem having subprograms who breach the contract raise an 'Assertion_Error`. In light of the above:
I think this is last resort and should be avoided if we can. Having a well documented behavior is important IMO for the reasons described above. This is also why, after consideration, I put the C++ "terminate the program" option off the table. |
I meant that in that case,
The issue is that, for a by-copy type, the insertion of the calls to Thanks for the rationle on your choice of strategy for when an exception is raised from a |
After some live discussion with @yannickmoy, we can agree on |
Program_Error still seems more natural to me. Constraint_Error generally implies there is some numeric range that is being violated, or a pointer is null. Raising an exception when none is expected seems very close to the return from a non-returning subprogram, and hence Program_Error. |
Ok @sttaft sounds good to me. What do you think about making this subject to a check category ? |
For sure. We try to make all checks subject to some check category. The existing " |
Sounds great, thanks Tuck 👍 |
* Unify the two configuration aspects into one * Specify the behavior in terms of permission for the compiler * Use Program_Error rt. Assertion_Error in the specification of No_Raise * Enforce that this can only be specified on record types or private view of record types
I pushed an update taking into account all the comments. In particular:
|
* Style/typo fixes. * Removes the TODO for the SPARK team as they answered it. * Rename the check category to `Raise_Check`, as per Tuck's comment. * Explicitly mention that the aspect cannot be set on a derived type.
@raph-amiard I removed the TODO for the SPARk team as Claire confirmed this was the default, and Yannick mentioned it could be a useful shortcut to explicitly state the default. Also I did the renaming of the category from |
minor comment: |
I think there is a typo when you say:
I think here you want to say that Finalize is only called for heap-allocated objects when the object is explicitly deallocated by calling an instance of Unchecked_Deallocation, or before it is assigned a new value, but not when exiting the scope of the corresponding access type. Is it what you had in mind? |
Thanks @Roldak and @yannickmoy and @sttaft , and everyone else who contributed. I think we're getting there |
This work is based upon the work of @Roldak on Lightweight finalization #65.
The idea the same, eg. to provide a way to have finalization in Ada that is:
The additions of this version of the RFC on top of what Romain did earlier is:
Ada.Finalization.Controlled
in terms of the current model.This feature is made of two RFCs:
Warning
This is not an RFC about object orientation. This is a redefinition of the underlying finalization model, with some provisions to make it usable out of the box. In particular, it is expected that prototyping of destructors in #106 should be done on top of this.