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

fix function isdef #173

Closed omlins closed 6 months ago

omlins commented 2 years ago

fixes #172

omlins commented 2 years ago

@cstjean, can you review and merge this?

cstjean commented 2 years ago

It's on my todo list, but it may take a few more weeks, with covid... Sorry

cstjean commented 2 years ago

should that be a function definition?

We should probably check the docs. I'd call it function declaration, but if the docs say otherwise...

matthias314 commented 11 months ago

The proposed function

islongdef(ex)  = @capture(ex, function (fcall_ | fcall_) body_ end)

seems to work (although I don't fully understand why). Replacing (fcall_ | fcall_) by fcall_ would lead to a syntax error both in Julia 1.9 and 1.10.0-beta1.

However, I think there is a problem with isshortdef: I've tried definitions of the form

ex1 = :( function f(x,y) x+y end )
ex2 = :( function (x,y) x+y end )
ex3 = :( f(x,y) = x+y )
ex4 = :( (x,y) -> x+y )

plus variants with argument type annotations, return type annotations and where clauses. They are all accepted by splitdef. Moreover, longdef preserves ex1 and ex2 and converts ex3 and ex4 to ex1 and ex2, respectively. shortdef preserves ex3 and ex4 and converts ex1 and ex2 to ex3 and ex4, respectively. islongdef is true for ex1 and ex2, but isshortdef is true only for ex3. I think it would make sense for isshortdef to return true also for ex4.

cstjean commented 10 months ago

Right - those would all be good test cases for whoever takes up the baton on this PR.

matthias314 commented 10 months ago

I could give it a try. Do you insist on using @capture? For example, I think one could simply say

islongdef(ex) = isexpr(ex, :function)

Of course, this doesn't check whether ex is well-formed, but the version with @capture doesn't do this completely, either.

cstjean commented 10 months ago

As long as the tests have good coverage, I don't think we need to worry about malformed expressions. isexpr is fine.