-
Notifications
You must be signed in to change notification settings - Fork 6.3k
Fix emitting events via module member access #16336
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For more complete coverage, we should add other types of tests here:
And I suspect we're missing this coverage for imported errors as well.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. BTW, I think ABI/AST tests are not multi-file but you can work around it by just having the file import itself as a module.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No contract is being imported here. |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,13 @@ | ||||||||||||||||||
| ==== Source: a ==== | ||||||||||||||||||
| event Transfer(address indexed _from, address indexed _to, uint256 _value); | ||||||||||||||||||
| ==== Source: b ==== | ||||||||||||||||||
| import * as Test2 from "a"; | ||||||||||||||||||
|
Comment on lines
+1
to
+4
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A bit clearer naming so that it's more obvious at a glance what is a file, what is a module, etc.
Suggested change
|
||||||||||||||||||
|
|
||||||||||||||||||
| contract A { | ||||||||||||||||||
| function returnAddress() external { | ||||||||||||||||||
| emit Test2.Transfer(address(11), address(12), 13); | ||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we have coverage for the And same for errors?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added. |
||||||||||||||||||
| } | ||||||||||||||||||
| } | ||||||||||||||||||
| // ---- | ||||||||||||||||||
| // returnAddress() -> | ||||||||||||||||||
| // ~ emit Transfer(address,address,uint256): #0x0b, #0x0c, 0x0d | ||||||||||||||||||
Uh oh!
There was an error while loading. Please reload this page.
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.
There is a related error message in the parser that is missing events as well:
solidity/libsolidity/parsing/Parser.cpp
Line 173 in 2f5d103
It should also mention UDVTs, but I'd recommend just replacing "struct/enum" with a catch-all "user-defined type" instead.