esl / ice

Apache License 2.0
2 stars 4 forks source link

Support unary operators '+'/'-' #54

Closed lucafavatella closed 10 years ago

lucafavatella commented 10 years ago

Main changes:

Also:

lucafavatella commented 10 years ago

This depends on pull request #10 of teaparser (already merged at the time of writing).

fenollp commented 10 years ago

And what about just calling the unary primops on nodes like {'+', _, A} and leave the old code for other {'+', _, A, B} nodes? The tuples represent the arity already, why do you need to explicitly write it?

lucafavatella commented 10 years ago

And what about just calling the unary primops on nodes like {'+', , A} and leave the old code for other {'+', , A, B} nodes? The tuples represent the arity already, why do you need to explicitly write it?

The AST in tea/ice supports whatever number of arguments for a primop with only one type of primop AST node, putting the arguments in a list. I could infer the arity of the primop from the lenght of the list of arguments but I found that hackish, as a buggy change in the length of that list (e.g. in a transformation) could change the semantics of the primop. So I am explicitly writing the arity for using it at the following line: https://github.com/esl/tea/pull/54/files#diff-72d161e0e0066656434bcf35d5187c8dR29

lucafavatella commented 10 years ago

Tomorrow I plan to change tcore from:

eval({primop, Primop, Eis}, I, E, K, D, W, T) ->
  ...
      F = tprimop:f(Primop),
      {apply(F, Dis1), MaxT}

to:

eval({primop, Primop, Eis}, I, E, K, D, W, T) ->
  ...
      {apply(tprimop, Primop, Dis1), MaxT}

following feedback by Ed.

Full conversion below just for tracking:

[18/12/2013 16:21:54] Edward Tate: Luca for PR 54 I tend to agree with both of your points
[18/12/2013 16:22:07] Edward Tate: the 'hack' is that + in Erlang doesn't support 1 argument natively as in you have to call 2 separate functions in erlang for + which is just bad and it is due to how you specify functions..
[18/12/2013 16:24:31] Luca Favatella: I am not sure I am following you
[18/12/2013 16:24:53] Edward Tate: in Erlang there are functions defined both for unary and binary +
[18/12/2013 16:24:59] Edward Tate: erlang:'+'/1, erlang:'+'/2
[18/12/2013 16:25:10] Edward Tate: in a dynamic language which is sane, you should never need to specify the arity
[18/12/2013 16:25:13] Edward Tate: erlang:'+'
[18/12/2013 16:25:24] Edward Tate: so the hack is that you have to specify the arity in the first place
[18/12/2013 16:25:33] Edward Tate: you can however get around this by doing
[18/12/2013 16:26:00] Edward Tate: fun (X) -> X;
      (X,Y) -> X + Y
end.
[18/12/2013 16:26:18] Edward Tate: which is less hacky that either solution
[18/12/2013 16:26:22] Edward Tate: (imo)
[18/12/2013 16:26:57] Luca Favatella: isn't that head mismatch?
[18/12/2013 16:27:22] Edward Tate: should be: 
add(X)    -> + X.
add(X,Y) -> X + Y.
[18/12/2013 16:27:35] Edward Tate: similarly with minus
[18/12/2013 16:27:52] Edward Tate: subtract(X) -> - X.
subtract(X, Y) -> X - Y.
[18/12/2013 16:28:12] Edward Tate: hmm
[18/12/2013 16:28:19] Edward Tate: same problem though since you have to export these
[18/12/2013 16:28:21] Edward Tate: :P
[18/12/2013 16:28:42] Luca Favatella: What you are saying is that you do not like how tcore interacts with tprimop
[18/12/2013 16:29:00] Edward Tate: no, what I am saying is that I don't like the fact that in Erlang you have to specify an arity to functions
[18/12/2013 16:30:02] Edward Tate: and that that is the real hack
[18/12/2013 16:30:11] Luca Favatella: There are 2 ways for avoiding that:
1) Give operators distinct names that include the arity; or
2) Apply the arguments to the operator, and dinamically select the right arity based on the number of arguments
[18/12/2013 16:30:13] Edward Tate: but your solution adheres to Erlangs hackiness better than Pierres
[18/12/2013 16:30:41] Edward Tate: number 1 is the same as specifying the arity...
[18/12/2013 16:30:43] Luca Favatella: Erlang's hackiness :)
[18/12/2013 16:30:48] Luca Favatella: exact
[18/12/2013 16:30:59] Edward Tate: number 2 is inefficient
[18/12/2013 16:31:19] Edward Tate: we don't want to have to call length every time we apply a function
[18/12/2013 16:31:22] Luca Favatella: so what would the proper solution be?
[18/12/2013 16:31:28] Luca Favatella: nono
[...]
[18/12/2013 16:32:15] Edward Tate: in lisp: (apply #'+ '( 0 )) or (apply #'+ '(0 1 2))
[18/12/2013 16:32:38] Edward Tate: works
[18/12/2013 16:33:12] Luca Favatella: eval({primop, Primop, Eis}, I, E, K, D, W, T) ->
  ...
      F = tprimop:f(Primop),
      {apply(F, Dis1), MaxT}

could become

eval({primop, Primop, Eis}, I, E, K, D, W, T) ->
  ...
      {apply(tprimop, Primop, Dis1), MaxT}
[18/12/2013 16:33:23] Luca Favatella: same here
[18/12/2013 16:33:58] Luca Favatella: of course functions should be exported accordingly in tprimop
[18/12/2013 16:33:59] Edward Tate: hmm much better
lucafavatella commented 10 years ago

@Ed: Please review this version of the PR. I hope I integrated your feedback. @Pierre: Could you integrate the 'not' unary operator in the parser?

fenollp commented 10 years ago

Done: https://github.com/esl/teaparser/pull/12

lucafavatella commented 10 years ago

@Ed: I think I addressed your concerns re list of primops in the tea module. Can you please review?

lucafavatella commented 10 years ago

Ed, As discussed - rebased, squashed, polished. Please consider merging.