search

JuliaServices / AutoHashEquals.jl

A Julia macro to add == and hash() to composite types.
Other
57 stars 12 forks source link

Simplify skipping syntax nodes... #40

Open gafter opened 1 year ago

gafter commented 1 year ago

in https://github.com/gafter/AutoHashEqualsCached.jl/pull/1#discussion_r1150925015 @nystrom wrote:

I'd make this more robust and just skip LineNumberNode, expecting a struct (or wrapped struct) at the beginning of the block. Or you could apply the macro to all structs in the block.

NHDaly commented 1 year ago

I think i agree with @nystrom. Or at least, you could consider asserting that there's only a single struct declaration in there. It seems it would a bit strange to encounter something like:

@auto_hash_equals begin
    struct Foo end
    struct Bar end
end

As a reader, i would probably incorrectly assume the macro is applying to all the structs, not just the first. No reason to keep this potential misunderstanding, i think.

gafter commented 1 year ago

The problem is that the block might be the result of applying a macro to the struct, and there might be a LineNumberNode in there when it is the result of applying the other macro. This is a scenario I'm trying to enable. So I think I do need to skip LineNumberNodes. However, your suggestion to assert a single struct or apply to every struct in the block if that situation arises - makes sense.

gafter commented 1 year ago

@NHDaly Asserting that there is only one struct would break this package's ability to interoperate with another hypothetical macro that produces a utility type during its expansion (inside begin ... end, which you get implicitly from quote). Is that what we intend?

NHDaly commented 11 months ago

hrmm yeah those are good points! Sorry for my long delay. 🤔 Yeah i don't have a great suggestion here then...

It's worth noting that i think macros run outside-in, so you should be able to see what the user wrote before any macros are expanded inside the usage?