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

Add macro verbatim blocks #6105

Closed
wants to merge 1 commit into from
Closed

Add macro verbatim blocks #6105

wants to merge 1 commit into from

Conversation

asterite
Copy link
Member

What

This adds the ability to specify verbatim blocks in macros, so macro code isn't immediately execute.

Why

Take a look at how the macro def_clone is implemented:

  macro def_clone
    # Returns a copy of `self` with all instance variables cloned.
    def clone
      clone = \{{@type}}.allocate
      clone.initialize_copy(self)
      GC.add_finalizer(clone) if clone.responds_to?(:finalize)
      clone
    end

    protected def initialize_copy(other)
      \{% for ivar in @type.instance_vars %}
        @\{{ivar.id}} = other.@\{{ivar.id}}.clone
      \{% end %}
    end
  end

Note all the \{% and \{{ in there. It's needed because we don't want to execute @type right away: we want to defer it until the method is executed, so it works like a macro method. The \ is ugly, but it works.

With this PR, we can write it like this:

  macro def_clone
    {% verbatim do %}
      # Returns a copy of `self` with all instance variables cloned.
      def clone
        clone = {{@type}}.allocate
        clone.initialize_copy(self)
        GC.add_finalizer(clone) if clone.responds_to?(:finalize)
        clone
      end

      protected def initialize_copy(other)
        {% for ivar in @type.instance_vars %}
          @{{ivar.id}} = other.@{{ivar.id}}.clone
        {% end %}
      end
    {% end %}
  end

verbatim basically won't treat the block inside it as macro code, but instead as just a string that's being pasted. Then when the clone and initialize_copy methods are invoked, the pasted code that's actually macro is executed.

So this is just a better way to write and do what we can already do.

I also need this for a complete implementation of #6082 . The JSON::Serialize module must inject an initialize in the included type so that types referenced in attributes (like converter) work well. For example:

class Foo
  class MyConverter
  end

  @[JSON::Field(converter: MyConverter)]
  property x : Int32
end

If MyConverter is pasted in the included JSON::Serialize module, it will give an error because MyConverter can't be reached from that location (that's the reason why I had to remove all the private types in the specs in that PR). If initalize is injected with an included macro, MyConverter can be accessed.

But since that injected initialize method already uses @type in macros, I'll have to escape with \ all of that. But it's so much simpler to just surround everything with a verbatim block.

Copy link
Contributor

@RX14 RX14 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A formatter spec?

@bew
Copy link
Contributor

bew commented May 19, 2018

And can you add a spec?

@asterite
Copy link
Member Author

Oh... the formatter will definitely not work with how it's implemented. Damn...

@asterite
Copy link
Member Author

Yeah, this will have to wait. It's too much work to make the formatter work, I'll probably need to introduce a new AST node. I'll close this PR until then.

@asterite asterite closed this May 19, 2018
@oprypin
Copy link
Member

oprypin commented May 19, 2018

Just another reason for me to consider the formatter harmful :/

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.

None yet

4 participants