-
-
Notifications
You must be signed in to change notification settings - Fork 113
Expose the name of the currently called item #150
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In a way this also would need to go into FastRecurse
and FastSuper
since there is a peephole optimization for these basic loop()
and super()
calls that immediately render.
At the same time those calls infos would only be observable by the engine itself, so paying the extra cost for that seems pointless?
So I think this either should be fixed or a comment should be left in FastRecurse
and FastSuper
about why they don't update the current call.
} | ||
Instruction::CallMethod(name, arg_count) => { | ||
state.current_call = Some(name); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would need to do something like let old_call = mem::replace(&mut state.current_call, Some(name));
since you can have things like {{ foo.bar(blub.blah()) }}
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this should just work as-is since foo.bar(blub.blah())
is really:
call blub.blah
pop
call foo.bar
I'll write some tests to confirm.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Confirmed (no bug) via tests. This change also means you can get rid of the name
argument in Object::call_method()
if the unwrap()
(which should always succeed) is acceptable.
2f3e83f
to
407fe75
Compare
Indeed, I didn't update the state where it's only visible internally since it provides no value. I'll leave a comment. |
Corollary to #146: so you can retrieve the dynamic name without having to clone it into the filter/function/test.