search

FluxML / MacroTools.jl

MacroTools provides a library of tools for working with Julia code and expressions.
https://fluxml.ai/MacroTools.jl/stable/
Other
308 stars 77 forks source link

Update pattern-matching.md #202

Closed jkroso closed 1 month ago

jkroso commented 1 month ago

Explain why QuoteNode never matches

jkroso commented 1 month ago 
@match :(:a) begin
  s_QuoteNode => 1
  s_quote => 2
end

You might expect the first pattern to match but actually its the second one that does so the result of that expression is 2.

cstjean commented 1 month ago

I see! Perhaps that example would be good to add to the PR.

Do you think it'd be reasonable to fix it? We can merge this PR in the meantime if you don't have time to consider it, but special-casing QuoteNode in the macro code doesn't sound unreasonable to me.

jkroso commented 1 month ago

If the Julia parser only produces QuoteNodes for quoted symbols then it would make sense to change the way MacroTools handles QuoteNodes. And as far as I know that's the case but I’m not sure on that. In a sense then though MacroTools currently does special case QuoteNode so making it work the way I expected would mean removing that special case. Which is just 1 line

cstjean commented 1 month ago

And that line comes from https://github.com/FluxML/MacroTools.jl/commit/1e01db99b35526d880846d351aff962367a378c3, 9 years ago, with not much to go on.

The problem with MacroTools is that A) it is lacking test cases B) breaking it can break a good chunk of the Julia ecosystem...

Ah, I'd lean towards merging this PR at the moment unless we are really confident of the implications of removing that line. It's more or less a breaking change. Can you please add the example you posted above?

jkroso commented 1 month ago

Yeah I think the best we can do is document the odity for now. I reworded it to include the example. How does it read now?

cstjean commented 1 month ago

Perfect, thanks.