[Experimental] Allow user-defined macro methods

[asterite]

Implements #8835

This is a quick (but not dirty: there are a few comments and even specs) experiment to see how hard would it be to get user-defined macro methods.

Before this PR, macro methods, those that you can call while inside macros, were hardcoded inside the compiler. For example calling debug or puts inside a macro, or the method StringLiteral#downcase.

This PR allows users to extend these methods in these ways:

1. If you define a macro inside the Crystal::Macros module, it can be called as a top-level macro method inside macros. The debug macro method is documented here, inside the Crystal::Macros module, like any other top-level macro method, so I thought for consistency it would make sense for user-defined top-level methods to be defined inside that module too.
2. If you define a macro method inside, say, Crystal::Macros::StringLiteral, then when invoking that method on an actual string literal (inside macros) it will be found
3. If you define a macro method inside a type, for example a macro bar inside Foo, and you write Foo.bar inside macro code, it will be found and called.

You can see examples of all of these in the PR's specs.

There's a big difference between these new macros methods and regular macros

All of these macros that you can define work in a different way than regular methods. The reason is that normal macros generate code: by default the things that you write are the generated code, and to do some logic you use {% ... %}, and to interpolate code to generate you use {{ ... }}. However, the macro methods introduce here don't allow {% ... %} nor {{ ... }} because they are already operating on AST nodes and they produce AST nodes, not code. So calling those methods outside macros is allowed but doesn't make sense (they just produce static code).

For example:

module Crystal::Macros
  macro foo(x)
    x + "bar"
  end
end

macro bar(x)
  {{ foo(x) }}
end

bar("foo")

Now how the macro method foo computes a different value depending on the value of x. But this is only if you call it from inside other macros. If you call it like Crystal::Macros.foo("hello") then the x that's mentioned there is just text, not a reference to the variable. It would need to be {{ x }} inside the regular macro methods.

But I think that's fine.

What's missing

Now, because these macro methods don't allow {% %}, and because iteration methods such as each simply don't exist in macro land, and because while doesn't work inside macro land, things are still a bit restrictive.

My idea forward would be to maybe:

• allow while inside macros (same for until),
• and also implement the many each and each_with_index methods,
• and even eventually remove {% for ... %} and replace it with .each (but this last point is not necessary).

And to polish things up, I would define all built-in macro methods in Crystal code but tagged with @[Primitive], the same way it's done with primitive methods.

• Another thing that probably doesn't work right now is defining a macro inside Crystal::Macros::ASTNode and finding it through Crystal::Macros::StringLiteral, because that hierarchy is fake and is not defined anywhere (but this is a simple fix).

I won't keep working on this because 1.0 might be near and I don't want to introduce a new feature that will radically change the existing language, but if there's interest and approval I will (if I have time).

[Blacksmoke16]

Wow! That was quick! This is 💯.

I think having this will allow macro code to be much cleaner/easier to maintain than it currently is.

I'd also be totally okay with 1 and 3, that is if 2 is adding the most complexity.

As you mentioned, the main thing this is missing is being able to iterate the macro method arguments. Is there a reason why .each .each_with_index isn't implemented on ArrayLiteral or TupleLiteral considering its essentially the same as .map and .map_with_index that currently exist?

Either way, .map and .map_with_index can be used as a workaround.

module Crystal::Macros
  macro foo(arr)
    hash = {} of Nil => Nil
    arr.map_with_index { |v, idx| hash[idx] = v + 1 }
    hash
  end
end

macro bar
  {{ foo([4, 5, 6]) }}
end

bar # => {0 => 5, 1 => 6, 2 => 7}

EDIT: One question I thought of. How would these fit into the generated API docs? I imagine 3 would just inherently work since it's just a macro, but what about the others that are defined in a different namespace. I.e. say I define some custom stuff in a shard.

[asterite]

Is there a reason why .each .each_with_index isn't implemented on ArrayLiteral or TupleLiteral

The reason is that you can use {% for ... %} so there was no need for that. But implementing those methods is really easy.

How would these fit into the generated API docs? I imagine 3 would just inherently work since it's just a macro, but what about the others that are defined in a different namespace. I.e. say I define some custom stuff in a shard.

All the macros are defined in Crystal code and you can document them and they will appear in docs. Or do you have an example where this doesn't work? You can try compiling the compiler of this branch, documenting these macro methods and they should show up in the docs.

That's also why I'd like to use @[Primitive] with the existing hardcoded macro methods, because the doc generator will pick them up automatically (no need to keep a separate fake file for this anymore), and even the lookup will be simplified (though we'd have to define the actual implementation of these primitives somewhere so it's just a slight simplification).

[Blacksmoke16]

@asterite I mainly mean the custom macro code.

module Crystal::Macros
  macro foo(x)
    x + "bar"
  end
end

module Athena
  macro add(one, two)
  end
end

macro bar(x)
  {{ foo(x) }}
end

Given this example, I can see bar defined in the Top Level Namespace, I see add defined in the Athena namespace, but foo is missing, probably since it's in the Crystal::Macros namespace?

Also probably should have a separate section like Macro Method Detail since they are not like normal macros.

[asterite]

Ah, yes, it's just a bug. Probably #8553 . But it's unrelated to macros or how they behave.

[asterite]

It does work if you define it like:

module Crystal
  module Macros
    macro foo(x)
      x + "bar"
    end
  end
end

module Athena
  macro add(one, two)
  end
end

macro bar(x)
  {{ foo(x) }}
end

So just a minor bug.

[Blacksmoke16]

This also enables recursive code in macros 💯

module Crystal::Macros
  macro fib(n)
    n <= 1 ? 1 : fib(n - 1) + fib(n -2)
  end
end

macro bar
  {{ fib(10) }}
end

pp bar # => 89

[RX14]

Isn't the macro def syntax gone now? If these macros operate differently, I think reusing that syntax is best.

But, to back up a minute, I'd like a discussion on whether want this first, I imagine @bcardiff will have some thoughts.

[Blacksmoke16]

@asterite Thoughts on implementing next keyword as well? Currently it's not allowed so have to do a good amount of nested if/else statements.

[asterite]

I guess it can be done (but I won't until this discussion continues).

[ysbaddaden]

@asterite I definitely want this. I want inflection methods (e.g. singularize, pluralize) on StringLiteral 😍

Yet, the reuse of macro is maybe a little confusing. Reusing the removed macro def as @RX14 proposed could make it more explicit that this isn't a regular macro but a macro method definition.

[Blacksmoke16]

It would be 💯 if this could make it into 0.34.0. It's pretty 🚀, so far.

@asterite One thing I did notice is the error message undefined macro method: 'xxx' is misleading when the macro exists, but was just passed the wrong amount of arguments.

macro foo(x, y)
  return x + "bar" + y
end

macro bar(value)
  {{ foo value }}
end

bar "foo"

In test.cr:6:6

 6 | {{ foo value }}
        ^--
Error: undefined macro method: 'foo'

[asterite]

Yes, I'm aware of that and a few other limitations (like for example you have to explicitly use self if inside a macro method and you want to call a macro in that instance).

I just won't keep working on this until it's decided we want something like this.

[Blacksmoke16]

I just won't keep working on this until it's decided we want something like this.

Fair enough. I know myself and @ysbaddaden want it 😉. Any thoughts on this @bcardiff, @straight-shoota, @RX14?

[asterite]

I also agree that we could maybe have a different syntax for this, maybe macro def. But then it's a new concept.

I'd like to think of it like this: if a macro has no interpolations inside it (no {{ ... }} nor {% ... %}) then it's a macro method that can be called from other macros.

[straight-shoota]

But then it's a new concept.

The way I see it, it is already a different concept than regular macros. Even if the technical distinction is pretty minimal.

[Blacksmoke16]

I tend to agree with @straight-shoota. It would be helpful if these macro methods had a separate section in the API docs. This way, actual code generating macros could be differentiated from helper/utility macro methods.

[Blacksmoke16]

Any movement on this? Seems like there is a consensus on wanting to use macro def syntax, and no objections on this moving forward so far.

[dscottboggs]

I definitely want this feature. Some of my current projects are waiting on a feature like this.

[AndiLavera]

Second, could definitely use this. Have a branch open kind of waiting for this.

[RomainFranceschini]

Kinda hope this feature will make it as well 🤞

[Blacksmoke16]

Any thoughts of moving forward on this @asterite? There hasn't been any objections to not wanting it. It also seems like this PR would unblock some work myself and others have in progress.

[asterite]

I guess, since this isn't a breaking change, I'll continue with it for a while (if I have time!).

[Blacksmoke16]

Bump 🙂. Please don't forget about this <3.

[asterite]

Let's wait for 0.34.0 to be released.

After that I'll ask the core team again if this is wanted, and we can maybe think how we want it to be.

[Blacksmoke16]

Now that 0.34.0 is live I'd like to get the discussion on this moving again.

My Proposal

First lets vote on if this is a wanted feature, 👍 for yes, and 👎 for no.

\cc @RX14, @ysbaddaden, @straight-shoota, @bcardiff, @asterite, @waj

Assuming it's a wanted feature, lets focus this PR on getting the core features implemented, then can iterate on additional QoL features once people have a chance to get their hands on it for a while.

This PR

1. Reuse the old macro def syntax to make it clearer that this is a new concept
2. Support next keyword when using iterator methods within these macro defs

After this PR

1. Update doc generation to separate normal macro and macro def.
2. Possibly support type restrictions in macro defs?
3. Update the book to include section on macro defs

I'd be happy to help on some of the possible iterations and documentation updates. This feature would allow doing some pretty cool new things with macros.

It would also allow Crystal itself, or other shards, to share common utility macros, which ultimately makes macro code more DRY, easier to read/document, and more maintainable.

[RX14]

As much as I think macros are largely a tool of last resort which is easy to shoot yourself in the foot with, at least this makes the gun cool and pretty.

[Blacksmoke16]

@asterite Given how the vote is going so far, would you be willing to update this PR with the points from my proposal?

It sure seems to me everyone is on board; myself and others would love to have this feature to unblock some current work/ideas.

[asterite]

@Blacksmoke16 Sorry, I don't have time right now to work on this. But if I have time I'll continue.

[Blacksmoke16]

But if I have time I'll continue.

@asterite I would greatly appreciate it ❤️. I can only imagine how busy you must be.

I'd be willing to add a bounty to this or something if it would help. Myself and others have some in progress work blocked by this.

Otherwise, I'll be patient 🙂. Unfortunately this is a bit out of my area of expertise 😉.

[Blacksmoke16]

@asterite I took a stab at this.

https://github.com/crystal-lang/crystal/compare/experimental/welcome-to-macro-land...Blacksmoke16:experimental/welcome-to-macro-land?expand=1

I'm open to better implementation ideas if you have any. Otherwise, I can add some more specs and open a PR.

Essentially I added a is_macro_def query getter to the Macro type and updated the parser to set this if it was created with macro def. We can then use this getter to exclude non macro defs when looking up custom macro methods.

[asterite]

@Blacksmoke16 Thank you! But I'm still not convinced about using macro def. And the bulk of the remaining logic is not that. I'll try to work on that now that I don't have anything else to work on... but no promises.

[Blacksmoke16]

@asterite That would be great! <3.

But I'm still not convinced about using macro def

I would be okay either way. However, I do tend to agree that these macros are not the same thing as the current macros. Being able to differentiate the two would be beneficial, such as displaying these within their own section in the docs, make it easier to know the intended purpose of each when looking at code, and possibly adding new features to one or the other independently.

That was my reasoning for the approach on my attempt. Treating them as macros to reuse the existing code, but just flag it as one or the other to know how to handle it.

[asterite]

I guess macro def would be fine, I'm just not sure it's clear what's that. However, if we do have macro def then the body should be parsed like a regular method, because {{...}} is not allowed in such macros.

But I'll work on this, don't worry. I'll just do that as a final step.

[Blacksmoke16]

I'm just not sure it's clear what's that

I'm planning on adding a section to the book about this feature, so having that should be able to explain the differences/use cases well enough.

However, if we do have macro def then the body should be parsed like a regular method, because {{...}} is not allowed in such macros.

I think that's fine. These are meant to accept/return ASTNodes versus "paste" code into a program as you mentioned in the OP so should be 👍.

But I'll work on this, don't worry. I'll just do that as a final step.

❤️

[asterite]

Sorry... in the end, I don't feel motivated to this if it's not going to be decided whether this is wanted for 1.0 or not. It's a huge change. And because of that, it probably doesn't make sense to rush and move forward with this.

It can wait post 1.0.

[straight-shoota]

@asterite As far as I can see, this is non-breaking, right? So it can easily be added after 1.0.
That also means it could be added before it because it won't affect anything relevant for 1.0. If classifed as a preview feature, we can easily introduce breaking changes after 1.0.
But I suppose just letting this sit for a while shouldn't be to much of an issue. It's not super urgent, even if @Blacksmoke16 can't wait to get this into his hands to make awesome macro stuff =)

[asterite]

Okay... so I started working no this but I'm not sure what's missing. I mentioned a few things in the original comment but they were all optional.

Maybe we can merge this as-is? I wouldn't like to introduce new syntax. This is backwards compatible also, nothing breaks. You just get to do new stuff with the same things. If some things don't work as expected we can fix them later. Or eventually completely remove the future.

[asterite]

Ah... I guess it makes sense to use macro def because then it's like a method that operates at the macro level. Plus it will get parsed as a method and it can be formatted, etc. I'll at least do that.

[Blacksmoke16]

It's not super urgent, even if @Blacksmoke16 can't wait to get this into his hands to make awesome macro stuff =)

I started on a refactor of my DI shard soon as this PR was made 😉. athena-framework/dependency-injection#8 allows doing some super cool stuff thats for sure.

Ah... I guess it makes sense to use macro def because then it's like a method that operates at the macro level.

Yea exactly, just differentiates it a bit to make it more clear what it actually is used for. Also id imagine it would be possible to separate them in the docs etc.

I'm not sure what's missing.

The only other thing would be to possibly support next within the macro versions of .each, .map etc. I have no idea how hard that would be to do tho. If its simple it would deff be a win for readability.

Currently I find myself doing like (contrived example):

macro def resolve_dependencies(args)
  args.map_with_index do |arg, idx|
    if some_condition
     "a"
    else
      if ...
      elsif ...
      ...
      end
    end
  end
end

When you could just do:

args.map_with_index do |arg, idx|
  next "a" if some_condition

  next "b" if ...
  ...
end

Deff not a requirement, just a nice to have.

[asterite]

Yeah, sorry, this is a bit more complex. If we go with macro def... what happens if you have:

class Foo
  macro def foo(x)
  end

  macro foo(x)
  end
end

We'll need a new space for storing this macro methods and looking them up so they don't collide. It's a lot more work than I expected.

If someone wants to fully implement this, please go ahead. I'm quitting.

[asterite]

The only other thing would be to possibly support next within the macro versions of .each, .map etc.

Supporting that and yield, also making self work is a huge amount of work.

[asterite]

Closing because I'm won't continue with this, but alternate PRs are welcome.

[Blacksmoke16]

Thoughts on merging this as in then at least? If in the future we change the syntax to macro def I don't think it would be a big issue. Mainly since it's more of an advanced feature so probably not a lot of usage, and a simple change.