-
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
base: develop
Are you sure you want to change the base?
Conversation
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
| fatalParserError(7858_error, "Expected pragma, import directive or contract/interface/library/struct/enum/constant/function/error definition."); |
It should also mention UDVTs, but I'd recommend just replacing "struct/enum" with a catch-all "user-defined type" instead.
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.
For more complete coverage, we should add other types of tests here:
natspecJSONABIJSONASTJSON(to make sure it appears in listed inusedEvents)
And I suspect we're missing this coverage for imported errors as well.
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.
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.
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 contract is being imported here. event_emit_from_module_via_member_access.sol would be a more accurate name.
| ==== Source: a ==== | ||
| event Transfer(address indexed _from, address indexed _to, uint256 _value); | ||
| ==== Source: b ==== | ||
| import * as Test2 from "a"; |
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.
A bit clearer naming so that it's more obvious at a glance what is a file, what is a module, etc.
| ==== Source: a ==== | |
| event Transfer(address indexed _from, address indexed _to, uint256 _value); | |
| ==== Source: b ==== | |
| import * as Test2 from "a"; | |
| ==== Source: A.sol ==== | |
| event Transfer(address indexed _from, address indexed _to, uint256 _value); | |
| ==== Source: B.sol ==== | |
| import * as M from "A.sol"; |
|
|
||
| contract A { | ||
| function returnAddress() external { | ||
| emit Test2.Transfer(address(11), address(12), 13); |
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.
Do we have coverage for the M.C.Transfer and M1.M2.Transfer cases (where C is a contract)?
And same for errors?
Note that this technically does not have to be a separate file (and it's a file, not a contract). A file can import itself as a module. |
|
Also, this needs a changelog entry. |
This PR fixes a bug in implementation of accessing an event which is defined in a separated, imported contract.
Closes: #16314