Skip to content
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

Merged
merged 6 commits into from
Feb 22, 2018
Merged

emit pseudo-keyword for events. #3538

merged 6 commits into from
Feb 22, 2018

Conversation

chriseth
Copy link
Contributor

@chriseth chriseth commented Feb 16, 2018

Fixes #2877

@federicobond
Copy link
Contributor

Looks good! Are there any plans to make "emit" a proper keyword for 0.5.x?

@chriseth
Copy link
Contributor Author

@federicobond yes, of course.

// 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
Copy link
Member

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" ?

_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.");
Copy link
Member

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.

Copy link
Member

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);
Copy link
Member

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.

Copy link
Contributor Author

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.

@axic
Copy link
Member

axic commented Feb 22, 2018

Apart from those comments it looks good.

@chriseth chriseth merged commit 2b62c20 into develop Feb 22, 2018
@axic axic deleted the emitEvents branch February 22, 2018 21:45
SerCeMan added a commit to intellij-solidity/intellij-solidity that referenced this pull request Mar 13, 2018
@ProphetDaniel
Copy link

I believe the most appropriate name for this new word would be trigger rather than emit.

@axic
Copy link
Member

axic commented Mar 30, 2018

The issue #2877 would be a better place to discuss.

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