-
Notifications
You must be signed in to change notification settings - Fork 5.9k
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
LLL: macro with zero args #2375
Conversation
I think this is a good short term fix, though we should aim at removing the "literal definitions" and just keep macros. |
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.
Use different names in the test cases please, because this could conflict with the current builtin instruction as well as when the macro is added to the standard list.
Perhaps go with headlesschicken
:)
3633dfb
to
f958bf1
Compare
I chose something more neutral. |
test/liblll/Parser.cpp
Outdated
@@ -174,6 +174,12 @@ BOOST_AUTO_TEST_CASE(list) | |||
BOOST_CHECK(!successParse("()")); | |||
} | |||
|
|||
BOOST_AUTO_TEST_CASE(macro_with_zero_args) | |||
{ | |||
char const* text = "(def 'panic () (asm INVALID))"; |
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.
Can you change it here too for clarity? (You are right, change is not needed 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.
No, that was me being headless chicken.
f958bf1
to
a039134
Compare
test/liblll/Parser.cpp
Outdated
@@ -174,6 +174,12 @@ BOOST_AUTO_TEST_CASE(list) | |||
BOOST_CHECK(!successParse("()")); |
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.
Tests fail on this one.
test/liblll/EndToEndTest.cpp
Outdated
char const* sourceCode = R"( | ||
(returnlll | ||
(seq | ||
(def 'zeroarg () (asm INVALID)) |
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 think we should have a better test here. Such as:
(def 'zeroarg () { (mstore 0 0x1234) (return 0 32) })
@axic ready to merge? |
On
develop
, LLL macros with zero arguments were not definable because()
was not a valid list.However, #2350 found a usage for that:
(panic)
.This PR modifies the parser so that LLL macros with zero arguments can be defined.