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

Interpreter: clear finished hooks after intepreting #12174

Merged
merged 4 commits into from
Jul 5, 2022

Conversation

asterite
Copy link
Member

Fixes #12172

Some explanation:

  • once the program is parsed and analyzed it computes a list of "finished hooks": code that must be analyzed and executed when the program ends
  • analyzing the code was actually happening already: that's why macro finished was able to declare vars, etc.
  • executing finished hooks is not done yet. That is, if you write puts "Hi" inside a finished hook, you won't see it in interpreted mode (I'm leaving that for a later PR)
  • however, the finished hooks were kept in memory and re-analyzed whenever we entered a new line in pry. And it seems analyzing finished hooks in pry doesn't work very well right now
  • so, far now, I'm just clearing the list of finished hooks right after interpreting some code. Ideally we'd also execute any finished hooks at the end of the program, but I played a bit with it and it's not so easy to do, so for now I'm not doing that
  • but at least this is a small step forward and you can debug apps that have a finished hook

@Blacksmoke16 Blacksmoke16 added kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:compiler:interpreter labels Jun 30, 2022
@asterite
Copy link
Member Author

Well, in the end I added the ability to execute finished hooks in interpreted mode.

So you can try this:

# foo.cr
macro finished
  puts "Bye!"
end

puts "Hello!"

If you execute that with crystal i foo.cr, previously the output was:

Hello!

Now it's:

Hello!
Bye!

You can also try this:

# foo.cr
macro finished
  puts "Bye!"
end

puts "Hello!"
debugger
puts "Hello again!"

Then with crystal i foo.cr this happens:

$ crystal i foo.cr
Hello!
From: foo.cr:8:6 <Program>#foo.cr:

    3:   puts "Bye!"
    4: end
    5:
    6: puts "Hello!"
    7: debugger
 => 8: puts "Hello again!"

pry> continue
Hello again!
Bye!

Which makes sense, because finished is executed at the very end of the program.

That said, I don't think executing code is common in finished hooks. It's usually used to define code. But I think it still makes sense to execute it, just like in non-interpreted mode.

@asterite
Copy link
Member Author

asterite commented Jun 30, 2022

Oh, and you can also try this:

$ crystal i
icr:1:0> macro finished; puts 1; end; puts 2
2
1
=> nil
icr:2:0> macro finished; puts 3; end; puts 4
4
3
=> nil

That is, you can actually define finished hooks in interactive mode, and they get executed.

@asterite
Copy link
Member Author

@cyangle If you want/can, you can check if the interpreter works fine in your particular case with this PR

@cyangle
Copy link
Contributor

cyangle commented Jun 30, 2022

@asterite Thanks a lot!

Now I can use the interpreter to debug my Lucky toy app:

$ cr i src/toy.cr 
Using compiled compiler at /home/chao/git/personal/crystal/.build/crystal
Listening on http://0.0.0.0:8080
2022-06-30 22:50:29 +00:00
From: expanded macro: setup_call_method:15:3 Api::V1::Users::Show#call:

    10:         begin
    11:   logger.info do
    12:     "user_id: #{id}"
    13:   end
    14:   debugger
 => 15:   json(UserTwin.new(current_user!))
    16: end
    17:       end
    18: 
    19:       __temp_139 = run_after_pipes
    20: 

pry> id
"774107312037625857"
pry> current_user

GET /api/v1/users/774107312037625857 (7fbb6ac9-6096-4da8-b1a1-33ff01e43c7c)
 ▸ user_id: 774107312037625857
#<User:0x7f4b24983840 id: 774107312037625857, name: "Erick Schmidt", email: "[email protected]", lock_version: 0, age: 18, created_at: 2022-06-27 05:53:23.634084000 UTC, updated_at: 2022-06-27 05:53:23.634085000 UTC, deleted_at: nil>
pry> current_user!
#<User:0x7f4b24983840 id: 774107312037625857, name: "Erick Schmidt", email: "[email protected]", lock_version: 0, age: 18, created_at: 2022-06-27 05:53:23.634084000 UTC, updated_at: 2022-06-27 05:53:23.634085000 UTC, deleted_at: nil>
pry> current_user!.id
774107312037625857
pry> current_user!.name
"Erick Schmidt"
pry> UserTwin.new(current_user!)
#<UserTwin:0x7f4b309a1460 @age=18, @id=774107312037625857, @name="Erick Schmidt", @email="[email protected]", @lock_version=0, @created_at=2022-06-27 05:53:23.634084000 UTC, @updated_at=2022-06-27 05:53:23.634085000 UTC, @deleted_at=nil>

I also tried it with the Lucky website, it worked. @jwoertink

This is awesome!

@jwoertink
Copy link
Contributor

whoa!! That's awesome @cyangle! Great work on this @asterite ❤️

@cyangle cyangle mentioned this pull request Jul 1, 2022
@asterite asterite marked this pull request as ready for review July 1, 2022 17:56
@beta-ziliani
Copy link
Member

Awesome! @asterite would it be hard to add specs? Do you want me to do that?

Also, for when this is merged, is this a reasonable change in the description? Thx!


Fixes #12172

Some explanation:

  • once the program is parsed and analyzed it computes a list of "finished hooks": code that must be analyzed and executed when the program ends
  • analyzing the code was actually happening already: that's why macro finished was able to declare vars, etc.
  • executing finished hooks is not done yet. That is, if you write puts "Hi" inside a finished hook, you won't see it in interpreted mode (I'm leaving that for a later PR)
  • however, the finished hooks were kept in memory and re-analyzed whenever we entered a new line in pry. And it seems analyzing finished hooks in pry doesn't work very well right now
  • so, far now, I'm just clearing the list of finished hooks right after interpreting some code. Ideally we'd also execute any finished hooks at the end of the program, but I played a bit with it and it's not so easy to do, so for now I'm not doing that
  • but at least this is a small step forward and you can debug apps that have a finished hook

@asterite
Copy link
Member Author

asterite commented Jul 1, 2022

Hi @beta-ziliani! There are no specs for pry yet and doing that is a huge task, and for now I won't have time to do that.

@asterite
Copy link
Member Author

asterite commented Jul 1, 2022

Another thing is the interpreter is mostly experimental. If it doesn't work well, it's not a big deal because compiled programs are the ones shipped.

But I promise I'll introduce tests for pry later on, otherwise it will be hard to make it evolve in a safe way. But I'd like to leave it for later.

@beta-ziliani beta-ziliani added this to the 1.5.0 milestone Jul 4, 2022
@straight-shoota straight-shoota merged commit 0af151f into master Jul 5, 2022
@straight-shoota straight-shoota deleted the bug/interpreter-finished-hooks branch July 5, 2022 17:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:compiler:interpreter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Interpreter: Defining class property in macro finished causes macro expanding errors in repl
6 participants