paiq / blackcoffee

CoffeeScript + hygienic macros
MIT License
105 stars 9 forks source link

.subst() does not descend into parameter lists #2

Closed ngn closed 10 years ago

ngn commented 10 years ago

Example:

macro m (f) -> f.subst x: macro.codeToNode -> y
m (x) -> x + 1

compiles to

(function(x) {
  return y + 1;
});

I was expecting

(function(y) {
  return y + 1;
});
vanviegen commented 10 years ago

Hmm, that does look somewhat unexpected. The reason it works this way, is that the x in function(x) is not an expression. So it would create a pretty broken AST to put an arbitrary expression in its place.

I can't think of any good reasons why you'd want to do something like this though. What is it that you're trying to accomplish?

ngn commented 10 years ago

I wanted to use (U+237A) and (U+2375) as identifiers in CoffeeScript, even though they are not valid JS identifiers (these are APL symbols, not Greek letters). My first attempt was:

macro withAlphaAndOmega (f) ->
  f.body.subst
    '⍺': macro.codeToNode -> alpha
    '⍵': macro.codeToNode -> omega

withAlphaAndOmega -> # enclosing a section with this macro makes it possible to use ⍺ and ⍵ inside
  # ...
  someFunction = (⍺, ⍵) -> ⍺ + ⍵
  # ...

Anyway, I managed to do it with an ugly hack.

However, there are other valid reasons for .subst()-ing parameter names. E.g. if you want hygienic macros (and I mean truly hygienic):

macro multiple (f) ->
  (macro.codeToNode ->
    (n) -> for [0...n] then fBody
  ).subst fBody: f.body

greet = -> console.info 'hi'
greet() # prints 'hi'
multiGreet = multiple -> console.info 'hi'
multiGreet 3 # prints 'hi' three times

n = 'nick'
greetByName = -> console.info "hi, #{n}"
greetByName() # prints 'hi, nick'
multiGreetByName = multiple -> console.info "hi, #{n}"
multiGreetByName 3 # prints 'hi, 3' three times, oops

This could be fixed by .subst()-ing every local variable that occurs in the macro with some tmp variable:

macro multiple (f) ->
  (macro.codeToNode ->
    (n) -> for [0...n] then fBody
  ).subst fBody: f.body, n: macro.csToNode "tmp#{macro.tmpCounter++}"

Unfortunately, not descending into parameter lists makes the latter impossible.

vanviegen commented 10 years ago

Hmm, your second example is pretty nasty indeed. Perhaps substitution of local variable variables with something guaranteed to be distinct should be a feature of codeToNode? Manually replacing variable names by some tmp name doesn't feel like something I want to be worrying about every time. But then again, what if we want to use the same variable, how would we do that when it's auto-renamed out of the way..... Hmm.. :-S

How would you have subst work on parameters? Just throw when the replacement is not an identifier and it is to be used as a parameter? I'm still not really sure we need this. The thing about a local variable is that it's local, so what should we care how it's called in the generated code, right? :-)

vanviegen commented 10 years ago

As you can see in the patch above, I reconsidered. The previous behavior was probably more inconsistent then the new behavior is lacking. :-)

ngn commented 10 years ago

Excellent! Thank you. I'll think about what could be done to ensure macro hygiene, maybe Scheme can give some inspiration. Practice with blackcoffee will show what's most convenient.