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.

Sign up for GitHub

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

Jump to bottom

Add support for interfaceId. #8642

Merged
merged 1 commit into from
Apr 23, 2020
Merged

Add support for interfaceId. #8642

merged 1 commit into from
Apr 23, 2020

Conversation

aarlt
Copy link
Member

@aarlt aarlt commented Apr 8, 2020 •
edited by axic
Loading

Closes #7856.

@chriseth
Copy link
Contributor

chriseth commented Apr 9, 2020

Rebasing this to see the coding style reporter in action.

@stackenbotten
Copy link

There was an error when running chk_coding_style for commit 3fc2ec17238f669e6a17e5aae616df682f53585e:

Coding style error:
 libsolidity/codegen/ExpressionCompiler.cpp:1588: for (auto& function : contract.definedFunctions()) {
 libsolidity/codegen/ExpressionCompiler.cpp:1589: const u256 hash(util::keccak256(function->externalSignature()));

Please check that your changes are working as intended.

@aarlt
Copy link
Member Author

aarlt commented Apr 9, 2020

Rebasing this to see the coding style reporter in action.

awesome! 😃

@aarlt aarlt force-pushed the interfaceid branch 2 times, most recently from 1f017ed to 42eabf9 Compare April 9, 2020 21:57
@aarlt aarlt requested a review from chriseth April 9, 2020 22:08
@aarlt aarlt changed the title [WIP] Add support for interfaceId. Add support for interfaceId. Apr 9, 2020
axic
axic reviewed Apr 9, 2020
View reviewed changes
libsolidity/ast/Types.cpp Outdated Show resolved Hide resolved
axic
axic reviewed Apr 9, 2020
View reviewed changes
libsolidity/ast/Types.cpp Outdated Show resolved Hide resolved
@axic
Copy link
Member

axic commented Apr 10, 2020

@aarlt please also change documentation.

@aarlt aarlt dismissed a stale review via 075e84b April 11, 2020 15:53
@aarlt aarlt force-pushed the interfaceid branch 2 times, most recently from 075e84b to 65858e4 Compare April 14, 2020 16:51
@aarlt aarlt force-pushed the interfaceid branch 4 times, most recently from ef743cc to 30e3b55 Compare April 14, 2020 17:02
chriseth
chriseth requested changes Apr 14, 2020
View reviewed changes
libsolidity/codegen/ir/IRGeneratorForStatements.cpp Outdated Show resolved Hide resolved
libsolidity/codegen/ExpressionCompiler.cpp Outdated Show resolved Hide resolved
libsolidity/ast/AST.cpp Outdated Show resolved Hide resolved
chriseth
chriseth reviewed Apr 15, 2020
View reviewed changes
libsolidity/ast/AST.cpp Outdated Show resolved Hide resolved
chriseth
chriseth reviewed Apr 15, 2020
View reviewed changes
libsolidity/ast/AST.h Outdated Show resolved Hide resolved
chriseth
chriseth reviewed Apr 15, 2020
View reviewed changes
libsolidity/ast/AST.h Outdated Show resolved Hide resolved
chriseth
chriseth reviewed Apr 15, 2020
View reviewed changes
test/libsolidity/semanticTests/interfaceID/interfaces.sol

function hello() public pure returns (bytes4 data){
HelloWorld i;
return i.hello.selector;
Copy link
Contributor

Choose a reason for hiding this comment

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

HelloWorld.hello.selector does not work?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah yes, it seem to work. At least if Yul is not used. If Yul is used, we got the following error:

Exception during extracted test: /Users/alex/git/solidity/libsolidity/codegen/ir/IRGeneratorForStatements.cpp(988): Throw in function virtual void solidity::frontend::IRGeneratorForStatements::endVisit(const solidity::frontend::MemberAccess &)
Dynamic exception type: boost::wrapexcept<solidity::langutil::InternalCompilerError>
std::exception::what: Member access to unknown type.
[solidity::util::tag_comment*] = Member access to unknown type.

Copy link
Contributor

Choose a reason for hiding this comment

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

#8684

@aarlt aarlt force-pushed the interfaceid branch 2 times, most recently from 6fb43ce to eac142a Compare April 16, 2020 02:45
chriseth
chriseth reviewed Apr 16, 2020
View reviewed changes
Changelog.md Outdated Show resolved Hide resolved
@axic
Copy link
Member

axic commented Apr 16, 2020

Can you add a test case for both selector and interfaceId not to include events?

Also can you add the example contract form the bottom of the EIP-165 spec (which has the helper for supportsInterface)?

@chriseth chriseth merged commit a371910 into develop Apr 23, 2020
@chriseth chriseth deleted the interfaceid branch April 23, 2020 10:24
axic
axic reviewed Apr 23, 2020
View reviewed changes
Changelog.md
@@ -4,11 +4,11 @@ Important Bugfixes:
* Fix tuple assignments with components occupying multiple stack slots and different stack size on left- and right-hand-side.

Language Features:
* Add support for EIP 165 interface identifiers with `type(I).interfaceId`.
Copy link
Member

Choose a reason for hiding this comment

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

Should have used EIP-165 😉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@axic axic axic left review comments

@ekpyron ekpyron ekpyron left review comments

@chriseth chriseth chriseth approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support ERC-165 identifiers
5 participants
@aarlt @chriseth @stackenbotten @axic @ekpyron