-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Semantic: Allow macro overload on named args #5808
Semantic: Allow macro overload on named args #5808
Conversation
0c61cb9
to
9f894c1
Compare
it "something between 2 macros (I need a name!!!)" do | ||
assert_type(%( | ||
macro foo(*, arg1) | ||
{{ arg1 }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps use 1
and 1_i64
for these two so the spec can tell if the compiler uses the other overload.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I used 1
and "foo"
, as I find it easier to directly see the difference in type of these two literals!
!!double_splat == !!other.double_splat | ||
# If they have different number of arguments, splat index or presence of | ||
# double splat, no override. | ||
unless args.size == other.args.size && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd apply de morgan's here
if args.size != other.args.size ||
...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
needs a format, somehow you managed to add extra spaces. |
Cool, thank you 😃 |
I think this is my first PR in the compiler, deep into the semantic pass 🎉
Fixes #5769
I added a few comments here and there to help me understand what some parts does, I can remove them if needed. I also fixed a bad argument name (
a_def
=>a_macro
) in 2 macro-related methods.I'll add more specs later if you think what I did is correct.
Thank you!