-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Can't call re-defined method. #1104
Comments
Yes, this is a known issue. Method redefinition is meant to be done before the "main" code is run. Once you invoke a method, that method is bound to that call and any subsequent calls, and redefining it has no effect. We could still make it work somehow, but that would mean invalidating some cached information when you redefine a method. I'll mark this as an enhancement. |
@makenowjust Why do you need this behaviour? |
@asterite For example, I want to call second previous method. http://play.crystal-lang.org/#/r/9oy def foo
p 1
end
$foo1 = ->foo
def foo
previous_def
p 2
end
foo
#=> 1
#=> 2
def foo
$foo1.call
p 3
end
foo
# expected
#=> 1
#=> 3 I think it is strange to change this behavior if redefined method has |
@makenowjust After a few similar bug reports, I tried to see how difficult it would be to implement this. It turned out to be really easy to do, here's a patch for that. So if you apply that patch to the compiler, compile a new compiler and try your sample program, it prints "1213" :-) The best thing is that all specs pass. We could just commit this, but I think it needs more thought. For example, right now if you have a method If we have many definitions for Now we have Maybe these names don't matter much because this kind of redefinition won't be very common (at least regular non-redefined methods are much more common). Another issue to think: you have |
@makenowjust See for example #123 , here @kostya wants to keep a reference to a method but he wants users to be able to redefine that reference. So you want one behaviour, @kostya wants the opposite. |
The most consistent behaviour in my eyes is that the last definition always rule. I would definitely expect the first example to print "2" twice. |
I'm closing this. The way the compiler works now, and will work like this forever, is:
This means that this is the output of this program: def a
p 1
end
a # => 2
# This is the definition that wins for the whole program
def a
p 2
end
a # => 2 In fact, since a couple of versions now you can invoke a method before defining it: foo # works!
def foo
puts "Hello!"
end If you want to somehow reuse a previous definition you can use |
I think it's a right place... module AA
def foo
puts "kek"
end
macro add_foo
def foo
previous_def
{{yield}}
end
end
end
struct DA
include AA
add_foo do
puts "lol"
end
end
DA.new.foo It's expected to output both "kek" and "lol", but |
I suggest you to open a new issue. |
previous_def finds in the same type. super finds in parents. You must use super here. |
@asterite it doesn't work if calling multiple |
@vladfaust defining |
Disclaimer: I never get help with short-history gitter, stackoverflow seems a little overhead for this particular issue (which is my personal issue); I hope someone would help me here. The code will be opensourced anyway. I'm implementing a versatile Callbacks system. I want module Callbacks
def before
true
end
macro before(&block)
def before
if previous_def
{{yield}}
end
end
end
def with_callbacks(&block)
before && yield # I omitted around and after callbacks
end
end
abstract struct Action
include Callbacks
abstract def call
def call_with_callbacks
with_callbacks { call }
end
end
abstract struct UsersAction < Action
before do
puts "It's UsersAction"
true
end
end
struct RegisterUserAction < UsersAction
before do
puts "It's RegisterUserAction"
true
end
def call
puts "#call from RegisterUserAction"
end
end
RegisterUserAction.new.call_with_callbacks
# Expected: It's UsersAction, It's RegisterUserAction, #call from RegisterUserAction
# Got: there is no previous definition of 'before' I'm sorry for disturbing you all... 😞 @Sija it hasn't worked for the whole issue. |
If you hadn't used macros and the fancy dsl you seem to be trying to create, you would already have a working program by now. Just stop using macros and magic so much and program what you need. |
@asterite I’m creating a shard which would make programming with Crystal even more joy. This callbacks issue is the final milestone. Check prism. If you find something useless that doesn’t mean it’s useless for everyone... |
http://play.crystal-lang.org/#/r/9ov
I think second
a
is wrong.In addition, there work fine.
http://play.crystal-lang.org/#/r/9ow
http://play.crystal-lang.org/#/r/9ox
Why?
The text was updated successfully, but these errors were encountered: