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

same-name default argument values lead to self-referencing variables or unexpected behavior #2515

Closed
jtratner opened this issue Aug 27, 2012 · 5 comments

Comments

@jtratner
Copy link

When you set a default value with the same name as a parameter to the function, it leads to one of the following on a null value:

  1. a self-referencing variable assignment (e.g. a=a)
  2. null value-variable is assigned the value of a different function parameter (see badDefault CoffeeScript in Rails #1 and Bug with assigning function and calling it immediately. #2)

Examples

coffeescript

a = "outer a"
bDefault = "outer bDefault"

selfRefDefault = (a=a, b=bDefault) ->
  console.log("a = #{a}; b = #{b}")
  return

badDefault = (a, b=a) ->
  console.log("a = #{a}; b = #{b}")
  return

badDefault2 = (a=c, b, c) ->
  console.log("a = #{a}; b = #{b}; c = #{c};")
  return

becomes this javascript

var a, bDefault, dummyDefault, dummyDefault2, dummyDefault3;

a = "outer a";

bDefault = "outer bDefault";

selfRefDefault = function(a, b) {
  if (a == null) {
    a = a;      // does nothing!
  }
  if (b == null) {
    b = bDefault;
  }
  console.log("a = " + a + "; b = " + b);
};

badDefault = function(a, b) {
  if (b == null) {
    b = a;         // b becomes first argument!
  }
  console.log("a = " + a + "; b = " + b);
};

badDefault2 = function(a, b, c) {
  if (a == null) {
    a = c;         // a becomes third argument!
  }
  console.log("a = " + a + "; b = " + b + "; c = " + c + ";");
};

The selfRefDefault example just fails to give a default at all, but the second two (badDefault) could cause more subtle errors, particularly because they don't get picked up by code validating tools for javascript (I tried JSLint, JSHint, and CoffeeLint).

Should this be a compiler error? Maybe it should be mentioned as a note on default values? (or maybe this is just obvious to everyone else :P)

Coming from a Python background, I (not thinking) wrote something similar to the above and, when looking at the compiled output, was surprised to see that this leads to self-referencing variables because of how the default arguments are converted. I guess I expected the compiler to just automagically handle the name overlap, just like it creates temporary variables for list comprehensions.

(If it matters: JSLint and InspectionJS will catch the first function as an error, JSHint and CoffeeLint miss it. They all miss the rest. )

@satyr
Copy link
Collaborator

satyr commented Aug 28, 2012

badDefault seems useful at times. It's valid in Ruby too:

irb(main):001:0> f = -> a, b = a { b }
=> #<Proc:0x00000100859958@(irb):1 (lambda)>
irb(main):002:0> f.call 42
=> 42
irb(main):003:0> f.call 42, 43
=> 43

Coming from a Python background

Note that Python's default argument is entirely different than ours.

@jtratner
Copy link
Author

@satyr note that if you reference a variable that comes afterwards (e.g. badDefault2), it still takes on the value of the inner scope variable and not the outer scope variable (which, to my understanding, is different than what ruby does).

@satyr
Copy link
Collaborator

satyr commented Aug 28, 2012

True, though consistent with the hoisting nature of JS scope.

irb(main):001:0> puts x; x = 1
NameError: undefined local variable or method `x' for main:Object
    from (irb):1
    from /usr/local/bin/irb:12:in `<main>'

vs

coffee> console.log x; x = 1
undefined
1

@davidchambers
Copy link
Contributor

Note that Python's default argument is entirely different than ours.

… and not something we should strive to imitate. ;)

> python
>>> def f(a=[]): a.append('wat'); return a
...
>>> f()
['wat']
>>> f()
['wat', 'wat']
>>> f()
['wat', 'wat', 'wat']

@jashkenas
Copy link
Owner

Yep, agreed -- I think it's correct behavior.

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

No branches or pull requests

4 participants