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.
Add initial draft of static immutability #126
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
Add initial draft of static immutability #126
Changes from all commits
8d7cb43
e923165
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.
This is still dangerous because auto-generated code can only rely on fields, not logical properties (unless we start annotating the logical properties, getters, which should be used for the hash code/equality, and that's just ugly).
We cannot define an automatic equality based on private fields, so any class with a private field and public getter would not work with automatic equality. The class needs to be sealed to ensure that other objects of the same type has the same private members.
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 was a bit concerned about this. @munificent seemed to feel it was a non-issue. Something to think about.
I don't understand why not? Can you elaborate?
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 you can, but since nobody knows the order or names of the private members, it is nigh impossible to make another implementation of the same interface that has the same equality/hash-code. So, rather than "we can't", it's probably "we shouldn't". It exposes the (arbitrary) internal implementation in a public method.
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 don't really understand this. Suppose I have class. It has private fields. I want to put it in a hash table. What is it that you suggest I do, if not hash the private fields? Make the fields public so that I can hash them? And if it's reasonable for me to do it, why is it not reasonable for a compiler to do it for me?
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.
True, it works. That class interface has private fields, so it is effectively unimplementable, so my concern that nobody else can implement the same equality is probably not a real problem.
My concern about hash code based on private fields may (I don't remember any more) have come from subclassing. If you subclass an immutable class with automatic hash codes and private fields in the superclass from another library, then you can't access the private field. You can call
super.hashCode
, and you might even be able to callsuper.==
, so it could work.Then the hashCode/equality of
D
would have to be something like:That could work.
It's still not symmetric,
C(1) == D(2, 1)
andD(2, 1) != C(1)
. That's a separate, and general, problem with a equality and subclassing a concrete class. You should never subclass a concrete class, but I guess that rule is hard to sell :)(Or maybe my concern was that the author might not want private fields to be part of the equality, because they are implementation details that may vary without affecting logical equality. I guess they can write their own equalities then).
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.
Again assumes that these classes are simple data objects which only represent their public fields. So far everything has suggested that these are full Dart classes, not data-types.
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.
What does "shares" mean?
Is it sent on the port, or is it just prepared for sending.
What class does the object have on the "other side"?
Can it access static state? Other types? Can an immutable object have a method like:
If the receiving isolate has not imported the library declaring the class of the object, can you send it? If you do, what is its run-time type? Which static state can it access?
If the receiving isolate has imported the same library, what is the run-time type of the received object? We have two different instances of the library (they have different static state), so which static state will the object access?
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 is sent on the port.
I would expect that it can access static state, and every isolate has its own instance of the static state.
Broadly, to your questions, I would expect isolates to share the same code, hence have the same classes loaded, etc. This is what the (very minimal, vague) docs for Isolate.spawn seem to describe? If there are other ways to create isolates that don't share this property, I would suggest that they not be able to access the shared heap.
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.
You can use
Isolate.spawnUri
to run arbitrary code that doesn't need to have anything in common with the spawning isolate. You can usually only send JSON-like structures between such isolates.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.
See dart-lang/sdk#36648 .
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.
So we have "mutate until frozen" collections.
Do we allow that for other, user written, immutable types too?
(We can perhaps get away with this by saying that you can only mutate the object using cascades on the object creation expression, and that mutating methods on an immutable object may not leak
this
in any way. Still sounds complicated).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 raised this question below - if there's need for it we can do it. It's basically a limited use of typestate. How limited depends on how much power we need or don't need.
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.
Likely not needed, and it's a very fragile functionality, so I'd prefer to avoid it.
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. @munificent agrees with you. After years of struggling with vector initialization in functional languages.... I'm a little more skeptical.
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.
If you need computation when creating the object, then you can use a factory constructor.
For collections, I do believe comprehensions ("control flow") will be sufficient.
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.
That's my hope too.
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.
This needs to be considered in the context of
built_collection
and protocol buffers.They use the builder pattern;
built_collection
currently copies only once when changing to a builder and back again, I believe this would force it to copy twice.Protocol buffers currently copy zero times--the 'builder' is 'frozen' and becomes the actual instance--I believe this would force it to copy once.
To support these without extra copying you'd need a way to explicitly call 'freeze' on a mutable collection to make it immutable.
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.
Yes, please! immutability is really useful for collections outside of supporting issolates, etc
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.
True, but if we get the semantics wrong we may find ourselves trying to retrofit immutables onto isolates. So I really prefer that we consider isolates immediately.
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.
Sure. Just saying that it's nice on its own, too.
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.
There is a difference between shallowly unmodifiable and deeply immutable objects, and we probably want both.
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.
What does shallow immutability give you that we don't already have with final fields?
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 you say below, it can't be a supertype if subtypes of immutable types must be immutable. If that isn't true, then the type system won't be the thing enforcing immutability.
(It's also a good reason why you should never name a type negatively, because a restriction is usually something you can remove in a sub-class, and
new MutableList() is ImmutableList
being true is just weird).We can add a supertype of
List
calledSequence
, and then letImmutableList
implementSequence
. We probably won't because it introduces a new super-type ofList
that code authors should then change all their read-onlyList
accepting functions to using.So
ImmutableList
shoud either be a subtype ofList
or be unrelated toList
.The former allows it to be used with code that currently accepts a list.
The latter allows it to have a different API, say with functional updates. I don't think introducing a completely new API is worth it.
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.
The Dartino approach was to have one bit on each object telling whether it's immutable.
(That could also be based on which GC page it is allocated in, or something similar).
When you create a new object using a const constructor, or an
.unmodifiable
constructor of a system collection, then the bit gets set if all the fields/elements/entries are immutable.You never need to do a deep check, but you do one more computation for each component object when creating a composite object.
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.
Yeah, I thought about writing this up as an alternative proposal, may still do (or feel free). I'm not very enthusiastic about it though. It makes every allocation in the program a little slower (or potentially a lot slower in JS), even if no immutability is used.
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 not clear how this would work. A type annotation of
ImmutableList
should accept the subtype_GrowableList
.If it's not type based, then it should probably not show up in the type system at all.
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.
Yes, I don't see a coherent story here, but added it based on discussion for future reference.
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 assume that all
Immutable
types will work fine in JS – just the isolate APIs won't be available.I'm sure there will be places where we can generate better JS if we know things are immutable, though – so I still consider this a "feature" – even for web-only code.
CC @rakudrama @jmesserly for comments...
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.
Yes, it's the communication that wouldn't work out of the box.
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 cloning objects should be a sufficient polyfill. You don't get the performance benefits from memory sharing, but you still get the benefit of a second isolate (i.e. web worker).