Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
clarify some edge cases around constructors #3737
base: main
Are you sure you want to change the base?
clarify some edge cases around constructors #3737
Changes from 1 commit
491a76b
4f9a4e1
868ced6
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I understand what's going on here but I could probably benefit from more elaboration and motivation. Maybe something like:
Generative constructors are unlike other functions because they:
Allocate a fresh instance of the class before any other work happens.
If there is a superclass, execute the superclass constructor's initializer list (recursively).
Execute this constructor's initializer list.
Execute this constructor's body.
If there is a superclass, execute the superclass constructor's body (recursively).
Note that initializer lists are run "down" the inheritance chain from superclasses to subclass while constructor bodies are run "up" the chain from subclass to superclasses.
This order is fixed by the language and, importantly, we require all initializer lists to be executed before any constructor body is run so that all final and non-nullable fields are definitely initialized before the new instance can be seen.
The parameters to a constructor are visible both in the initializer list and in the constructor body. The initializer list runs before the body which means the parameters are already potentially seen and used before the augmented constructor body begins.
That's why when calling
augmented()
, the augmenting constructor doesn't pass any parameters to the original body. Those parameters are already available before the augmenting constructor began and thus the augmenting constructor can't interfere with them.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's the other way around, actually. Superclass bodies run before subclass bodies. (The order of pre-body initialization doesn't really matter, but it's defined in case there are visible side effects.)
An object creation expression creates an object, then it invokes the constructors to initialize that object.
Invoking a non-redirecting generative constructor does the following:
Object
class.this
bound to the new object.That ensures that all fields are initialized before the first body executes, and then it ensures that all super-class bodies have been executed before the subclass body runs, so the object is at least "fully initialized as the superclass" before the subclass sees it.
But 'nuff nitpicking.
Does that matter wrt. whether we can pass arguments to
augmented
?When augmenting function bodies, we can invoke the parent body using
augmented(args)
because nothing happens after (or during) binding actuals to formals and before executing the body. Calling "as a function" is safe because it does nothing except set up an environment and then execute the code we wanted in that environment.Why can we not do that for constructors. Because we can't invoke the constructor without side effects.
But if we only want to invoke the body, then we probably could.
Take:
This
augment
would invoke the body as if it was a function with a parameter list derived from the parameter list of the constructor (same types and names, but never any initializing or super parameters).An equivalent desugaring (not how it's specified!!) could be:
That would allow invoking the augmented constructor body with new arguments, like what we do for normal functions.
We can say that we don't want to allow that, and that the body must use the same argument list as the augmented construtor. (I'm fine with that. But then we should consider why we don't say that for functions too.)
And as written, the augmented body is evaluated with the same variables as the augmented body, which is scary. That means a function call-looking expression like
augmented()
can change local variables. (Also, "the same variables" probably does not extend to local variables, so at most the same parameter scope.)Generally, we can't just assume that the augmented body has its arguments already passed, because we are not chaining through "parent constructors" in the augmentation chain the same way we are between classes.
(If we were, new initializing formals would be prepended, not appended.)
Which means that we never invoke the augmented constructor. We probably don't even evaluate its parameter list, because we use the one from the last augmentation instead, which should replace the original (or match it, in which case we don't know which one we evaluate). That mean no binding actuals to formals for the augmented constructors formals. (Which is also a problem, because we do need to do that to set up the correct names for the initializer list. And if we do that, we can probably do the same for the body.)
All in all, this is a mess. I'll have to think 🤔.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have thought a little about it.
Here is one possible approach: https://gist.github.com/lrhn/47d4161c4743a09659732952b21591f7
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't allow altering parameters via augmentation (even names of positional parameters), which simplifies a lot of this I think?
I did change this up a bit, to specify that local variables are not in scope, it is only the parameter scope which is shared. It does mean that a call to
augmented()
can change a non-final parameter variable in the augmenting scope. That is weird, to be sure, but I think it is probably acceptable.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've thought some more (far too much, likely): https://gist.github.com/lrhn/47db7dd136bb5ed388b0cd1c8260001d
And couldn't find any reasonable way to execute the
augmented
body of the augmented declaration in any other scope than the same parameter scope that the current body is executed in. That was the scope that the parameter list introduced, and which the initializer list and super constructor invocation has already been run in, which means those variables might be captured in closures already stored inside the object. Introducing a new scope just to execute the augmented body in, would not be consistent with the augmented constructor's initializer list and body sharing a scope.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think what I have specified here is a subset of that? Basically it doesn't allow bashing completely over anything pre-existing, or changing something to/from redirecting or not, if it was clearly one or the other.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can an augmenting constructor add a super initializer or redirecting initializer?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe!
Porbably not a redirecting initializer, unless the constructor already was redirecting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed the part about redirecting initializers here.
I am allowing a constructor with no body and no initializers to be augmented with a redirecting initializer. This means it can be ambiguous if a constructor is redirecting or not until after augmentation.
We could remove that, but I think code generators will sometimes want to implement a constructor as redirecting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a section clarifying that you can add a super initializer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be safe!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As in, you think we should allow converting constructors between generative and factories?
Or, you think it is good that this is specified as an error?