Skip to content

Conversation

@aarlt
Copy link
Collaborator

@aarlt aarlt commented Apr 23, 2021

No description provided.


void SemanticTest::initializeBuiltins()
{
m_builtins["smoke_test"] = [](FunctionCall const&) -> std::optional<bytes>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we typically use camel case for builtins?

Also, make sure there are no name clashes (unlikely but better safe than sorry :)).

Suggested change
m_builtins["smoke_test"] = [](FunctionCall const&) -> std::optional<bytes>
solAssert(m_builtins.count("smokeTest") == 0, "");
m_builtins["smokeTest"] = [](FunctionCall const&) -> std::optional<bytes>

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the first builtin to be added, but the assert is good to have, yes.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, good point. I thought that the existing "directives" were builtins but I see that it's a new thing.

@aarlt aarlt force-pushed the fix_test_builtins branch from 7b6184a to 4ab3cb6 Compare April 26, 2021 12:49
@aarlt aarlt force-pushed the fix_test_builtins branch from 4ab3cb6 to 8239eaa Compare April 26, 2021 12:51
@chriseth chriseth merged commit d4ea845 into develop Apr 26, 2021
@chriseth chriseth deleted the fix_test_builtins branch April 26, 2021 13:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants