brownplt / pyret-lang

The Pyret language.
Other
1.06k stars 109 forks source link

Functional extension of data values should be disallowed #307

Open shriram opened 9 years ago

shriram commented 9 years ago
data Posn:
  | posn(x :: Number, y :: Number)
end

fun move-up(w):
  w.{x: w.x + 1}
end
> posn(2, 3)
posn(2, 3)
> move-up(posn(2, 3))
{x: 3, y: 3, $fieldNames: [
  "x",
  "y"
], _match: <method>}

(Yes, brands.)

jpolitz commented 9 years ago

Should we just make this be an error (e.g. functional update of things with brands) until we have a good answer?

blerner commented 9 years ago

Can we limit it to if the field being updated shows up in $fieldNames? Or is that making it too confusing?

Also -- we should fix the printing in general so that it avoids any $compiler-named fields...or move them out of the dict of the object and into some more private space. (I think the only reason it's in dict is because I didn't see a "prim-field-access" to go along with "prim-app", and didn't feel like creating one at the time.)

jpolitz commented 9 years ago

We can limit it, but IMO it's really weird that

> l :: List = [list:].{x: 5}

works.

blerner commented 9 years ago

Mrph, maybe. No contest that it's weird, but I think everyone's taste for what's acceptably weird here will differ ;-)

Another possibility is to restrict it from working on any values with a $fieldNames -- i.e. on data-derived values. But custom-branded values would/could still work.

To be clear: I'm not averse to making this be an error until we have a good answer. I'm just brainstorming potential good answers :)

jpolitz commented 9 years ago

@shriram had an interesting idea for this a while ago, which was simply compiling functions that do the update for data values that would check that the update was OK and then rebrand the new value.

That does give contexts that don't have access to the constructors the ability to create branded instances of the datatype when they couldn't before, so it changes guarantees around data. But that might be worth it for how good the example Shriram wrote above it.

blerner commented 9 years ago

IIRC, that suggestion ran into issues where 1) we couldn't use the .{} syntax, and 2) we couldn't give types to those updating functions, and 3) it lowered the guarantees about data.

jpolitz commented 9 years ago

Sorry, wasn't clear, mixed levels.

Data instances would not get Pyret-level functions on them for this. They'd just have different semantics on update than normal objects, probably implemented as a JS-level function on the representation. Those implementation-level functions would check the incoming value with the same annotation as the constructor, do the update, and rebrand.

The different guarantees on data aren't a clear win or lose, but they do change.

sorawee commented 9 years ago

Clearly,

l :: List = [list:].{x: 5}

ought not to work, but sometimes I want to update only one field, and the extend expression would help a lot

data Test: test(a, b, c, d, e, f) end
a = test(1, 2, 3, 4, 5, 6)
b = a.{d: -1}

Currently

With data Test2: test2(a :: String) end

shriram commented 7 years ago

This output has arguably gotten even worse:

image