Skip to content
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

Allow adding fields to a record during an update #3673

Closed
HKalbasi opened this issue Jun 11, 2019 · 14 comments

Comments

Projects
None yet
3 participants
@HKalbasi
Copy link

commented Jun 11, 2019

The inferred type of function

f x = x { a = x.b + 2 }

is

forall t7 t8.  
  { a :: t7    
  , b :: Int   
  | t8         
  }            
  -> { a :: Int
     , b :: Int
     | t8      
     } 

but it can be more general :

forall t7 t8.  
  { forall possibilities of a  
  , b :: Int   
  | t8         
  }            
  -> { a :: Int
     , b :: Int
     | t8      
     } 

I don't know is that type possible, but i think it is ( if it isn't already supported ).
Use cases:
1- write general and reuse able functions:
for example bodyparser is a function that add a body field in a pretty structure and cookieparser is a function that add cookie field to its input. for write some function like this , i can don't use the great record assignment syntax and rebuild record that lead to diffrent bodyparser for every project, or use it but when want to use the bodyparser , fill body with some garbage that is ugly.
2- simplicity in use of records:
this example:

x = { a: 2, b: { c: 5, d: 2 } }
-- y is really close to x
y = x { b { c = 3 , e = 8 } }

leads to compile error, because compiler expect x.b.e to be exist, but it does not need it and
can do it work without x.b.e . but for bad type of _ { e = _ } , it does not let me to do this and i should use this:

y = x { b = { c : 3 ,  e : 8 , d : 2 } }

and rewrite value of d.

@hdgarrood

This comment has been minimized.

Copy link
Member

commented Jun 11, 2019

So you'd like to be able to add fields to a record during a record update? There's probably a way of expressing the type you want at the moment with some of the builtin row type classes, like Union, but I suspect type inference would suffer, so I think the current behaviour is probably the right default.

@HKalbasi

This comment has been minimized.

Copy link
Author

commented Jun 11, 2019

Work with row types is painful ( maybe i don't know the correct way ) and i think equal of _ { x : 5 } that is a function setting field x with row classes will be long and confusing.
That inferred type let us use beatiful record update syntax in adding field. Update and insert fields are not really distinct, and all older usages of update stay valid and work.
Type of record update with now row types will be a mess and will be confusing when we print the inferred type, but it can be clean using some syntax sugar, such as i mentioned above.

@natefaubion

This comment has been minimized.

Copy link
Contributor

commented Jun 11, 2019

See purescript-record for examples of what the type of an insertion would look like: https://pursuit.purescript.org/packages/purescript-record/2.0.1/docs/Record

Because we allow duplicate labels in rows, but records can only have a single value at a label at runtime, insertion and deletion require a Lacks constraint, where a record update does not.

@HKalbasi

This comment has been minimized.

Copy link
Author

commented Jun 11, 2019

Yes, this is not support in now purescript, and need some new feature like iflacks or something i said.
But i think this is needed and meaningful and useful, isn't ?

@HKalbasi

This comment has been minimized.

Copy link
Author

commented Jun 11, 2019

Or maybe possible with some combination of union and nub, but a compiler level support is still preferred ( see @hdgarrod comment )

@hdgarrood hdgarrood changed the title type of record assignment Allow adding fields to a record during an update Jun 11, 2019

@hdgarrood

This comment has been minimized.

Copy link
Member

commented Jun 11, 2019

If we were to generalize record update syntax in this way such that the type of such a function became more complicated, I wouldn't consider adding syntax sugar to address that to be such a good idea; if the type is complex then type inference suffers, regardless of how pretty the type looks when you write it out. In my view, type inference is really, really important for language ergonomics.

But i think this is needed and meaningful and useful, isn't ?

I can get on board with it being useful, but I don't think it's needed; I don't remember seeing this feature requested anywhere else. I think the union, disjointUnion, and merge functions from the purescript-record library cover this use case sufficiently well.

@HKalbasi

This comment has been minimized.

Copy link
Author

commented Jun 12, 2019

Oh, i think "type inference suffers" means the inferred type will annoy user.
I think this is possible to design a type that don't break the type inference. We can't use current nub of row types to do this, but i think some way to type it "type inference friendly" should exists.
Is it impossible?

@hdgarrood

This comment has been minimized.

Copy link
Member

commented Jun 13, 2019

I can’t see how, I think these things are necessarily in opposition to each other. I’m going to close this for now but we can reconsider if someone puts together a detailed proposal which addresses the type inference issue.

@hdgarrood hdgarrood closed this Jun 13, 2019

@HKalbasi

This comment has been minimized.

Copy link
Author

commented Jun 21, 2019

I implement it in this example.
You can test it, I think its type inference works well.
There is a little probability that example isn't compatible with purescript type system, but I think there is a little different and the problem is not insoluble.
@hdgarrood is it enough for reopening issue?

@hdgarrood

This comment has been minimized.

Copy link
Member

commented Jun 21, 2019

I don't think that design is suitable, because if we were going to add something like this to PureScript, it would need to make sense for row types in general, not just records. That is, if we have { foo :: ?, bar :: String } meaning a record which may have a foo property of any type, but must have a bar property of type String, that seems clear enough, but then what does Variant ( foo :: ?, bar :: String ) mean? Also, what kind would ? have? If ? can only exist as a row label and not in other contexts, that seems messy.

This proposal is also missing a more detailed explanation of why the existing language features are not sufficient. It's easy to write a function which adds a particular property to a record if that property doesn't exist, or overwrite it if it does, with the Record.union function.

Sorry if it seems like I'm being difficult, but we're at a point in the evolution of the language where we really need to be saying "no" a lot; my impression is that language stability is very desirable for many users of the language, based on the response to e.g. Phil's post on discourse.purescript.org suggesting that we stop adding features.

@HKalbasi

This comment has been minimized.

Copy link
Author

commented Jun 22, 2019

That make sense for all row types. for example:

b_ :: SProxy "b"
b_ = SProxy

dropB :: forall t3 t5. Variant t3 -> Variant ( b :: t5 | t3 ) -> Variant t3
dropB c v = on b_ (\x->c) (\x->x) v

can clearly be more general:

dropB :: forall t3. Variant t3 -> Variant ( b ? | t3 ) -> Variant t3

( b ? ) has a hidden forall in it, so it is meaningful in functions.

Also, what kind would ? have? If ? can only exist as a row label and not in other contexts, that seems messy.

This was a printing mistake from me. ? is not a type. This is a special state that a label in row type can get. So I replaced { b :: ? } with { b ? } to resolve this misunderstanding.

This proposal is also missing a more detailed explanation of why the existing language features are not sufficient.

The union function is a messy function, which break the "every label should appear once in a record" rule. That makes problems like this:

setAto5 x = union { a : 5 } x
setAto5 { b : 2 } == { a : 5 , b : 2 } -- OK!
setAto5 { a:3 , b : 2 } == { a : 5 , b : 2 } -- Compile Error :(

Its sister function, Record.merge solves this, but its type signature has a Nub constraint, which is not type inference friendly. We should declare type of every expression and it is annoying.

language stability is very desirable

I am strongly agree that stability is important, but it has no conflict with adding features. Languages that have been made for decades ( like c++ and js ) are still adding backward-compatible features and no one is annoying.

? feature is backward-compatible but changing the type of record update syntax may cause some problem. I think every code that compiled before should compile after changing that, but I'm not sure. We can test it and if it has serious problems, we can leave it.

? feature alone is also good, and it allow us to get rid of Record.union and Record.merge and write a type-infrence-friendly updateOrInsert function.

Please reopen the issue, to more people say their opinion.

@hdgarrood

This comment has been minimized.

Copy link
Member

commented Jun 23, 2019

I'll give a more complete response later, but for now just a couple of thoughts:

It may be worth looking at the "scoped labels" paper as well: https://www.microsoft.com/en-us/research/wp-content/uploads/2016/02/scopedlabels.pdf although note that these can already be implemented as a library: https://github.com/paf31/purescript-scoped-labels.

The union function is a messy function, which break the "every label should appear once in a record" rule.

There isn't actually a rule enforcing this; in fact there was a deliberate decision to allow duplicate labels in rows. If a row r has duplicates, then the meaning of Record r is based on the first occurrence of any given label in r. See https://pursuit.purescript.org/builtins/docs/Prim#t:Record

Please reopen the issue, to more people say their opinion.

You're free to continue discussing it on this issue; the fact that the issue is closed doesn't mean that we want people to stop commenting on it, it just means that it's not being considered as something to be worked on right now.

@HKalbasi

This comment has been minimized.

Copy link
Author

commented Jun 24, 2019

We can break that rule ( maybe rule is not a good word for that ) but it will cause problems, specially where we need to unify types. It is not a good idea that updateOrModify function use Record.union, because it will make a record that behaves same as we want in some cases, but not always ( for example , using in if statements, putting in list or array, equality i mentioned in my previous comment , and ... make compile error that we don't expect from updateOrModify function ).

I think that paper confirm me. It says that { x :: Bool } is not { x :: Bool , x :: Int } , so updateOrModifyAto5 is not Record.union { a : 5 } .

@HKalbasi

This comment has been minimized.

Copy link
Author

commented Jun 24, 2019

From that paper I found this paper that is very close and more general to my ? . Is it possible in purescript as a library or as a feature?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.