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

Execute while and until inside macros #10959

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Blacksmoke16
Copy link
Member

Resolves #9091 (comment)

@asterite
Copy link
Member

Note that I excluded these from the macro language because they allow the compiler to hang. I'm not sure we should add this.

@lbguilherme
Copy link
Contributor

At some point I was experimenting with a macro that could generate SQL from almost arbitrary expressions and on a part of it I ended up with this piece of code:

  {%
    stack = ...
    forever = [nil] of NilLiteral
    forever.each do
      forever << nil
      top = stack.last
      if top.is_a?(NilLiteral)
        forever.clear
      else
        # do stuff that might involve pushing to "stack"
      end
    end
  %}

Yeah, it hurts my eyes too. But the point is: it is already possible to make infinite loops and consume unrestricted memory from macros. This PR doesn't make things worse, just cleaner and more obvious.

@asterite
Copy link
Member

It's true

@Blacksmoke16
Copy link
Member Author

This should be good for review.

@straight-shoota
Copy link
Member

I'm not sure about this. For example, I would expect such macro loops to have a block macro body similar to the for loop:

{% while v < 5 %}
  {% v += 5 %}
{% end %}

That doesn't exclude the inline syntax implemented in this PR. But it's definitely more limiting in what can be expressed.

I'd rather look at the bigger picture with a focus on implementing loops with a block body. The short form might be an enhancement, but it's not a priority.

@beta-ziliani
Copy link
Member

I have a working prototype of a MacroWhile node here: https://github.com/beta-ziliani/crystal/tree/feat/macro-while

At the moment, I was just playing with this small thing, but at some point I'll make a proper PR with everything that is missing (until and specs).

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

Successfully merging this pull request may close these issues.

5 participants