-
Notifications
You must be signed in to change notification settings - Fork 0
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
Create an explicit immutable and cown region. #41
Conversation
7f513a2
to
9756865
Compare
I need to fix the cown stuff, before this change can land as I have made it stricter, and properly broken valid_01.vpy, rather than just in Asan |
9756865
to
ca13c10
Compare
ca13c10
to
8b67a92
Compare
I've created a cown region, and it fixes the Asan issue in the Pros
Cons
AlternativesCould create a new region for each cown. This would look nicer, but requires more complex checking. Questions
|
In our prototype they are currently the same. I added the Another thing I could think of would be closed regions. They would be opaque, until they're opened. (This opening could be implicit though) |
We could try not putting cowns into a sub graph, that would have them floating around, like the global objects initially did. Then we can just say that everything outside a "drawn" region is the cown region. Finding a good visualization will probably also require a bit of try and error :D We shouldn't block this PR on it, I can try to figure something out next week, if you don't want to :D |
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.
Very nice changes! Makes the whole thing a lot cleaner :D
} | ||
|
||
void freeze() | ||
{ | ||
// TODO SCC algorithm | ||
visit(this, [](Edge e) { | ||
auto obj = e.target; | ||
if (!obj || obj->is_immutable()) | ||
if (!obj || obj->is_immutable() || obj->is_cown()) |
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 means that cowns can't be frozen. It makes sense, but we should check that this is inline with what is written up. If not, we probably want to correct the text on this one, as I like this implementation
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 guess I viewed them as already in the "shared" world, so freeze doesn't do anything. A frozen cown can still be mutated in "my view" of the model.
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.
Yep, I agree. This was just meant as a reminder to check/sync the docs
No description provided.