-
Notifications
You must be signed in to change notification settings - Fork 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
emit pseudo-keyword for events. #3538
Conversation
6bc74a0
to
2829264
Compare
Looks good! Are there any plans to make "emit" a proper keyword for 0.5.x? |
@federicobond yes, of course. |
docs/contracts.rst
Outdated
// for `Deposit` to be called. | ||
Deposit(msg.sender, _id, msg.value); | ||
// Events are emitted using `emit` followed by | ||
// a call of the event. Any such call |
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.
How about "followed by the name of the event" ?
libsolidity/analysis/TypeChecker.cpp
Outdated
_emit.eventCall().annotation().kind != FunctionCallKind::FunctionCall || | ||
dynamic_cast<FunctionType const&>(*type(_emit.eventCall().expression())).kind() != FunctionType::Kind::Event | ||
) | ||
m_errorReporter.typeError(_emit.eventCall().expression().location(), "Expression has to be an event."); |
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.
You are using "event call" elsewhere, perhaps worth using the same for consistency.
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.
Or how about going with "event invocation" in all places?
std::tie(arguments, names) = parseFunctionCallArguments(); | ||
eventCallNodeFactory.markEndPosition(); | ||
nodeFactory.markEndPosition(); | ||
expectToken(Token::RParen); |
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.
Expect checks the current position and is not reading the next token, right? If so markEndPosition
should be correct otherwise not.
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 had a look at the --ask
output and it is fine this way.
Apart from those comments it looks good. |
I believe the most appropriate name for this new word would be |
The issue #2877 would be a better place to discuss. |
Fixes #2877