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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a Const<T> immutable wrapper type #41

Closed
wants to merge 5 commits into
base: master
from

Conversation

Projects
None yet
5 participants
@bendmorris
Copy link

bendmorris commented Mar 9, 2018

Provides a way to create a recursive read-only view of a value.

Just want to note that as I went to submit this, I noticed an "immutable" branch which has a nearly identical proposal from @nadako, down to the examples used. I didn't know about or reference @nadako's proposal in constructing this one...great minds think alike I suppose 馃槈 In fact I came to this idea over the past several days when thinking about a few concrete use cases, including one in HaxePunk: HaxePunk/HaxePunk#574 and tried out a macro solution which wasn't totally satisfactory.

Thought I would submit anyway to get the ball rolling.

Rendered version

@bendmorris bendmorris force-pushed the bendmorris:const branch from a84b603 to 48e4097 Mar 9, 2018

@bendmorris

This comment has been minimized.

Copy link

bendmorris commented Mar 13, 2018

@EricBishton

This comment has been minimized.

Copy link
Member

EricBishton commented Mar 13, 2018

What is the advantage of this over the final keyword (which is already implemented in Haxe 4)?

@bendmorris

This comment has been minimized.

Copy link

bendmorris commented Mar 13, 2018

The use cases are pretty different. final simply disallows reassignment of a class field. Const prevents any modification of any value, recursively - so e.g. an Array that you can iterate over but not push to or modify its members. It doesn't prevent reassignment, and isn't limited to fields.

@EricBishton

This comment has been minimized.

Copy link
Member

EricBishton commented Mar 13, 2018

final isn't (or isn't supposed to be) limited to fields. That's just the obvious first case -- and may be the only thing implemented at the moment. IIRC, when folks were discussing it, it was for any variable (locals and fields) and could be applied to classes and methods as well. (Though the concept of final for classes and methods is a bit murky to me since it isn't about inheritance as it is for e.g. Java.)

I think that simply applying Const to an object will not be able to do as you desire. Yes, for Array and Map, we can disallow use of functions that may modify the underlying object, but that cannot be enforced for random methods on generic objects, unless you now add some new metadata to each method that describes the enforcement policy. (I'm pretty sure that people want to avoid "'const' hell" as it is known in C.) And, even if you do add such metadata, it has to become a run-time construct because we cannot guarantee that the object won't be passed around to some other code that will call functions with side effects. (I would be amazed if the typer looked any further than the current method when performing its function; certainly it doesn't follow deep code paths.)

That is, unless I'm misunderstanding the whole concept here.

@Justinfront

This comment has been minimized.

Copy link

Justinfront commented Mar 13, 2018

Is Const needed for Ocaml target?
Can you provide me with a really simple practical and useful use of Const - in relation to Array<Sprite>
Is this a feature like 'Abstract' that I use all the time or something like 'Options' which for me is quite marginal and I would not really care if it was not there tomorrow, seems more like the 'Options' feature?

@bendmorris

This comment has been minimized.

Copy link

bendmorris commented Mar 13, 2018

final prohibits reassignment; that's an entirely orthogonal concept here.

unless you now add some new metadata to each method that describes the enforcement policy

This is specifically in the proposal. I have a working prototype which I linked above, which adds the metadata to Array. Yes, it specifically only works when the metadata is there; otherwise it only allows field access by default. Languages like Rust also require declaring mutability; it's not a new concept. The main difference here is that everything is mutable by default and immutability is opt in.

And, even if you do add such metadata, it has to become a run-time construct because we cannot guarantee that the object won't be passed around to some other code that will call functions with side effects.

I don't think it has to at all. It's explicitly adding a set of compile time checks, which you have ways to get around. It doesn't have to be exhaustive or apply at runtime to be useful.

@bendmorris

This comment has been minimized.

Copy link

bendmorris commented Mar 13, 2018

Can you provide me with a really simple practical and useful use of Const - in relation to Array

I'll give you one example that triggered this proposal: in HaxePunk each scene maintains a list of entities in the scene which have a specific type (a Map<String, List<Entity>>). We have a public method to get a list of entities that have a specific type. Currently I have two options:

  • Get the List from our internal Map, copy it and return the copy. This is a performance hit and isn't recursively immutable (so you could, say, take one of the entities and change its type, invalidating our scene state.)
  • Return the same List stored in our internal Map. This bypasses the copy but is dangerous because behavior is undefined if the caller modifies the list; it could no longer be an accurate list of active entities with the given type.

This proposal adds a third option: return the list as a Const<List<Entity>>. This returns the same list that's already allocated, but the compiler will prevent the user from modifying the list or its contents, throwing a compiler error if they do so.

@EricBishton

This comment has been minimized.

Copy link
Member

EricBishton commented Mar 14, 2018

It seems to me that your example problem could be solved, at least in part, via composition (a wrapper object). But that's neither here nor there.

final prohibits reassignment; that's an entirely orthogonal concept here.

Const also prohibits reassignment. It's not orthogonal if they do the same thing. 馃槃 However, final cannot be applied to a return value (yet), so I understand your desire here. Even if it did, it would apply to the assignment of the variable, and not to the internals of the object.

One trouble I see is that we then need to add @:const to the StdLib in order to make this useful -- even to you. All of a sudden, it's not optional for the compiler team.

It doesn't have to be exhaustive ... to be useful.

Ah, the bugs that will be generated! If it's not exhaustive, it's a maintenance headache.

Languages like Rust also require declaring mutability; it's not a new concept.

Correct, it was added as a similar (opt-in) concept to C89, almost 30 years ago. And engineers have hated it ever since.

Const prevents any modification of any value, recursively

In your example above, a Const<List> is returned -- but you want the Const-ness to apply to SomeType (and its methods and return values) recursively without explicitly declaring it. (After all, you're passing back a list that is active -- and not const -- in another context.) So how is the compiler supposed to know that there should be an error here?:

var i : Const<List<SomeType>> = MyLib.getSomeType();
for (type in i) {
   var otherStuff = type.getOtherStuff();
   var extraStuff = otherStuff.getExtra();
   extraStuff.setProperty(newValue);    // <<<--------  Should create an error!
}
@bendmorris

This comment has been minimized.

Copy link

bendmorris commented Mar 14, 2018

It seems to me that your example problem could be solved via composition (a wrapper object). But that's neither here nor there.

It could. This is a generic way to construct such wrappers automatically, so that I don't have to create one per type myself. I could also do this with a macro, which I addressed under "alternatives."

Const also prohibits reassignment.

It doesn't - I'm not sure where you're getting this idea. It prohibits mutating the object, not reassigning references:

final x:{x:Int, y:Int} = {x:1, y:2};
// this is invalid; I can't reassign a final
x = {x:3, y:4};
// this is fine; finals aren't recursively immutable, only the reference is immutable
x.y += 2;

var x:Const<{x:Int, y:Int}> = {x:1, y:2};
// this is fine; I can reassign a Const
x = {x:3, y:4};
// this is invalid; I can't mutate a Const
x.y += 2;

These are entirely different features. Const prohibits reassigning fields within the object, and final does not - it's a big difference.

All of a sudden, it's not optional for the compiler team.

And that's why changes like this go through a proposal process. Given that @nadako was thinking along similar lines, I'm hoping the compiler team will see the value added by this change as greater than the cost of maintaining it in the standard library. I think this will be appreciated by people coming from a functional programming background.

In your example above, a Const is returned -- but you want the Const-ness to apply to SomeType (and its methods and return values) recursively without explicitly declaring it.

I don't know where I'm missing you, but I addressed this exact problem and proposed a solution. Members accessed from the List are automatically Const, so they can't be modified. If Entity also declares immutable methods, I can use them; if it doesn't, I can only access its fields. But obviously I'm also going to mark up Entity's fields, or I wouldn't be using this construct in the first place.

Notably, your example works correctly, right now in my prototype! 馃槃

@bendmorris

This comment has been minimized.

Copy link

bendmorris commented Mar 14, 2018

@Justinfront in your case it seems like a custom abstract would be a better solution if you want limited mutability.

@Justinfront

This comment has been minimized.

Copy link

Justinfront commented Mar 14, 2018

Ben there is certainly a problem that abstracts can't be inherited, but for the Fixel use case they could do something like this:
https://try.haxe.org/#6af57
try haxe does not currently have final so I can't really explore mixing it in within a try example and I am not sure I have final in my working haxe version, there are quite a lot of options and variations that allow a lot of flexibility without java restrict type approach.

@EricBishton

This comment has been minimized.

Copy link
Member

EricBishton commented Mar 14, 2018

So, I've figured out two things: The first is that I am conflating a variable (e.g. a reference) and what it contains (a value), even though they are distinct and I know it. I believe that it comes from the way I think when I am writing code -- and I'm trying to observe this solution from that perspective. Thus, I'm sorry if I have sounded belligerent here. I'm definitely NOT trying to be.

Second, I've read the code, and while I only have a passing understanding of OCaml, I think I see how you are applying const-ness across sub-element access. That can only apply within a single scope (and its contained scopes), though. I think it still breaks down when the sub-entity is passed into another routine -- unless that receiving routine's parameters are marked Const as well. (And I think you intend them to be.) However, that breaks interoperability between libraries, doesn't it? Suddenly, you either a) can't use a companion library that doesn't conform (because the compiler errors at the call point), or b) you cast away or subvert all of that const-ness and break the library writers assumptions. (Believe me, you will get support calls from people who break your assumptions.) And, then, for interoperability's sake, everybody has to conform and it's no longer optional.

@bendmorris

This comment has been minimized.

Copy link

bendmorris commented Mar 14, 2018

The intention behind returning a Const value in the first place is to limit what the caller does with it. That's by design.

@back2dos

This comment has been minimized.

Copy link
Member

back2dos commented Mar 14, 2018

With Const<T> from T it should not be named Const, but ReadOnly which is a different thing:

typedef HasX = { var x:Int = 0; }
var o:X = { x: 0 }
var c:Const<HasX> = o;
var initial = c.x;//this should not change, because it's recursively *constant*, right?
o.x = 12;
assert(initial == c.x);//oooops

This is not just a nuance: treating read only data structures as immutable ones can fail phenomenally in concurrent code.

@skial skial referenced this pull request Mar 14, 2018

Closed

Haxe Roundup 423 #487

1 of 1 task complete
@bendmorris

This comment has been minimized.

Copy link

bendmorris commented Mar 14, 2018

//this should not change, because it's recursively constant, right?

The value itself isn't recursively constant, only the view of the value is immutable, i.e. in your example you can't modify o from c. It's an "immutable reference." Naming is up for discussion.

Solving problems with concurrent code will take more than this and is beyond the scope of this proposal, but I think this is a step in the right direction.

@bendmorris bendmorris force-pushed the bendmorris:const branch from b530f75 to cf6b00d Mar 14, 2018

@bendmorris bendmorris force-pushed the bendmorris:const branch from 6808a16 to 751abb9 Mar 18, 2018

@Simn

This comment has been minimized.

Copy link
Member

Simn commented Apr 17, 2018

I don't really like these wrapper types, mostly because Null<T> is such a mess. On the other hand, I'm not sure how else to handle something like this because our internal type instance handling is a mess as well.

@bendmorris bendmorris referenced this pull request Apr 18, 2018

Closed

Complex type expressions #44

@bendmorris bendmorris closed this May 1, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment