-
-
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
Add inject_primitives: false
to macro_spec
#11269
Add inject_primitives: false
to macro_spec
#11269
Conversation
|
Instead of: assert_type(<<-CR, inject_primitives: false) { int32 }
macro foo
1
end
foo
CR This is also possible: assert_type(<<-CR,
macro foo
1
end
foo
CR
inject_primitives: false) { int32 } But the main benefit of using heredocs is that if those tests fail, the exceptions thrown from them will have more comprehensible error locations than before because there are no leading spaces or indentation in those code literals. |
@Sija I disagree. Previously, you would have all components of the method call distributed across lines before and after the sample code. With heredocs, the method call is clean and concise. The code follows afterwards. |
@straight-shoota heredocs are fine in general, it's just this eye-ping-pong that bothers me ;) |
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.
Let's not fret about style here.
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.
🚀 Thanks @straight-shoota 🙏
This patch adds
inject_primitves: false
to the semantic specs inspec/compiler/semantic/macro_spec.cr
. I picked this specs file at random. Lots of compiler specs don't need primitives injected. Ideally, most of them should always be fully self-contained and not rely on an external prelude.In the same motion, I transformed percent string literals to heredocs in order to improve readability and clearly associate the arguments following the code sample to the call to
assert_type
andassert_error
.Two or three examples in this file still require primitives because they use methods like
+
and==
. I think they can and should be refactored to be self-contained but I tried to avoid semantical changes to the example codes for now.The performance impact of spec execution is quite significant:
Without this patch, the 107 examples execute in 11.54 seconds
With this patch, they execute in 1.05 seconds.
Other compiler specs can certainly be improved in a similar way.