Closed TrevorBurnham closed 10 years ago
Please please please can we kill super
now? It's not at all intuitive what it should be doing here.
At least disable it in static methods :(.
This seems like a compiler bug, in that I would expect the latter form to fail the same way as the following:
-> super
[stdin]:1:4: error: cannot call super outside of an instance method.
-> super
^^^^^
@variable = -> ...
is not an instance method (neither, of course, is @variable: -> ...
, but it does at least use property syntax in a class body, which implies some kind of CoffeeScript magic...).
I think it makes sense that you can write
class Parent
@classMethod: ->
# ...
class Child extends Parent
@classMethod: ->
super
and calling Child.classMethod
will invoke Parent.classMethod
. It's just inconsistent that you can't do this with @classMethod = ->
, which is otherwise equivalent.
For the record, a similar bug appears when using normal functions inside the class body:
class X
fun = ->
super
var X;
X = (function() {
var fun;
function X() {}
fun = function() {
return X.__super__.fun.apply(this, arguments);
};
return X;
})();
And referencing super
in the class body triggers a weird compilation error:
$ cat super.rb
class X
super
$ coffee super.rb
super.rb:2:3: error: Class bodies shouldn't reference arguments
super
^^^^^
Wow, until today I thought the only way to create static methods was this way:
@staticMethod = ->
Then I found this https://github.com/jashkenas/coffee-script/issues/279 with
@staticMethod: ->
How did we get into this state where both are possible? That will be I guess impossible to remove from the language, in which case the behavior should match. super
makes sense here as much as it makes sense in instance methods (unless OOP == Java in your mindset).
@xixixao: Yes, super
only makes sense in classical OOP (Java/C++/etc.). In prototypal OOP, we have prototype chains and nothing more. I have never once used super
, and it's not because I'm trying to avoid it; it's just not idiomatic.
@michaelficarra
@xixixao CoffeeScript doesn't add class-based OOP, it just offers syntactic sugar that makes JavaScript's standard prototypal inheritance look like classes—and, regrettably, adds that super
business that has caused so much trouble and confusion. Probably should've used a keyword other than class
to make that clear since it seems to confuse the hell outta people.
I don't see the problem with writing Foo::method.call(self, argle, bargle)
if you really need to call a method from further up the inheritance chain. That makes it explicit what's being called.
@erisdiscord I didn't say that CoffeeScript adds class-based OOP and every language in the end is just a syntactic sugar over
00000101 00100110
...
I assume Jeremy didn't change his mind over the last three months, so super
is probably here to stay. If you don't like classes, I don't see the problem with writing
object = {data: 4}
method = (n) -> this.data + n
alert method.call object, 3
Makes it more explicit what's being called.
@xixixao alright, mr. smartypants. I think you know what I meant. CoffeeScript expands on JavaScript's semantics, but it doesn't somehow automagically change the native prototypal inheritance into classical inheritance.
And that's a cute example, but it's not really analogous, now, is it? Besides, there are actually valid cases where you'd write code (kind of) like that. :unamused: Calling a known ancestor's implementation of a method you've consciously shadowed with your own implementation ain't quite the same situation as calling methods on arbitrary objects, though.
@erisdiscord And again, I never said it did.
It is quite analogous.
a = new A
a.method()
What function am I calling here? Method declared in class A, or its super class, or somewhere else where someone overwrote A.prototype.method? The point is, what all this "sugar" gives you is an easier, idiomatic way of expressing programs, in certain paradigm, in this case classical OOP. And back to the topic, if we have super
for instance methods and nothing is stopping us from having it work for static methods as well, it should work across the syntax variations (unfortunate) that are part of the language. It can be useful. Like a.method()
. If we want to support only prototypical OOP then we should throw out super and static methods and instance methods and classes altogether.
If we want to support only prototypical OOP then we should throw out super and static methods and instance methods and classes altogether.
Baloney, my friend. Pure baloney ;)
Prototypes and classes are both ways of accomplishing inheritance with objects. If you have inheritance, then you have super
— it's an implied concept. If you have inheritance, then you have instance methods (duh). If you have inheritance, and an "abstract" thing that you inherit from, then you have a class — no matter what you decide to call it.
That was sarcasm.
If you have inheritance, then you have super — it's an implied concept.
I totally agree.
Glad to hear it ;) Ah, GitHub tickets, where nuance is lost, body language is invisible, and smirks go unseen...
In case it helps the discussion, this block of code illustrates the various possible combinations of variable assignment (@,=,:) and super
class XXX
bar4 : ->
alert 'XXX : bar4'
bar5 : ->
alert 'XXX : bar5'
return 'bar5'
bar6 : ->
alert 'XXX : bar6'
return 'bar6'
@bar7 : ->
alert 'XXX @bar7'
class Foo extends XXX
#bar1 = alert 'test1' # run by Foo()
bar2 : 'test2' # bar attribute of Foo.prototype
@bar3 = 'test3' # @bar : 'test'
bar4 = ()-> #
alert 'Foo bar4'
super # runs XXX:bar4
bar4() # run fn internal to Foo
bar5 : ()->
alert 'Foo bar5'
super # runs XXX:bar5
@bar6 = ()->
alert 'Foo = @Bar6'
super # runs XXX:bar6
@bar7 : ()-> # adds constructor
alert 'Foo : @bar7'
super # runs XXX@bar7
class Bar extends Foo
constructor : ->
super
alert new Foo().bar2
alert Foo.bar3
new Foo().bar5()
Foo.bar6()
Foo.bar7()
In node.coffee
superReference
, a constructor
is included if method.static
.
In addProperties
if assign.variable.this
method.static = yes
I haven't traced this earlier, but I'm guessing that @bar7 :
sets this, but @bar6 =
does not.
https://github.com/jashkenas/coffee-script/issues/1598#issuecomment-26737133
looks like the same issue, revisiting 'Support super
in static/class methods?' from several years ago.
@connec is quite right. The source code seems to intend exactly this behaviour, but METHOD_DEF does not do what you might think when you just glance over it. Edit: Way of the mark, we are actually missing a bit of logic in Class
. And it looks like it's simpler to make the 2nd case work (instead of throwing an error).
@epidemian
referencing super in the class body triggers a weird compilation error
This is caused by the somewhat clunky node construction in the grammar and the unavoidable order of validity checks (The search for illegal arguments
happens while constructing the class body and illegal super
is detected when compiling the super
call.)
Compare:
$ bin/coffee -bce 'class then super arg'
[stdin]:1:12: error: cannot call super outside of an instance method.
class then super arg
^^^^^^^^^
A dedicated Super
node would prevent this and also make for much nicer code. (A PR for that should appear in the not too distant future...)
Please lets not disable super
in static methods. The intuitive behavior for me is the one that compiles from @foo : -> super()
. Does anyone know the original intent? http://stackoverflow.com/questions/19850865/coffeescript-static-inheritance-of-child-classes
https://github.com/jashkenas/coffee-script/issues/627 Static functions and the '=>' parameter
may be relevant. The method used there to identify 'static' functions appears to work just as well for @static=
as for @static:
, where as the one currently used with 'super' only works with the :
version.
I just learned another interesting thing :
class Foo
@bar = =>
@thing = 'wack'
this results in a _this = this;
that is never used in the compiled javascript. Might be a related issue.
The var _this = this;
declaration is generated by
else if not @static
o.scope.parent.assign '_this', 'this'
So it looks it is sensitive to the same method.static
setting.
In the development version the approach to binding for the 'fat arrow' has been changed substantially https://github.com/jashkenas/coffee-script/issues/3143
There is no longer a difference between @:
and @=
. This code using @static
is no longer needed.
@marchaefner I was going to start tackling this, but your "dedicated super
node" sounds like a lovely approach. Should I look at other things while awaiting your PR, or take a stab at it?
@jashkenas: I already got a quick'n'dirty fix for this issue and #2949. Just need to work on a proper description since this has some side effects. (I think only positive ones.)
The Super
node itself would just fix the confusing error message. It also allows for a nicer fix for #2367, though that should also be fixable by a well placed ?
-- haven't tested this one thoroughly. (If you feel like writing pretty code have at it.)
Looks like marchaefner
has come up with a solution. But just in case it might be useful for further discussions, here 's a fix that works for me. I implemented it in the 1.6.3 release, but it should work in the development master
In nodes.coffee
class Class
I add a testStatic
method that is a stripped down version of addProperties
, and call that in walkBody
. It is designed to add value.static
to a @foo=
node. addProperties
is never called on that type of node.
11/13 - I rewrote testStatic
so it handles both the :
and =
forms.
# test whether node is a static method
# extracted from addProperties
# handles both @foo: and @foo=
testStatic: (node) ->
if node instanceof Assign
if node.variable.base.value isnt 'constructor'
if node.variable.this
func = node.value
func.static = yes
# dont set bound context here
else if node instanceof Value and node.isObject(true)
# handle '@foo:' case
props = node.base.properties[..]
while assign = props.shift()
@testStatic(assign)
# Walk the body of the class, looking for prototype properties to be converted.
walkBody: (name, o) ->
@traverseChildren false, (child) =>
cont = true
return false if child instanceof Class
if child instanceof Block
for node, i in exps = child.expressions
@testStatic node # ADDED TEST FOR '@foo='
if node instanceof Value and node.isObject(true)
cont = false
exps[i] = @addProperties node, name, o
child.expressions = exps = flatten exps
cont and child not instanceof Class
super
compiles differently in these two snippets:and
The way it compiles in the second snippet seems like a bug to me. See http://stackoverflow.com/a/19865440/66226.