Skip to content

[Sol->Yul] Provide selector for some internal functions. #11015

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

Merged
merged 1 commit into from
Mar 2, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion libsolidity/codegen/ir/IRGeneratorForStatements.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1689,9 +1689,15 @@ void IRGeneratorForStatements::endVisit(MemberAccess const& _memberAccess)
functionType.kind() == FunctionType::Kind::DelegateCall
)
define(IRVariable{_memberAccess}, IRVariable(_memberAccess.expression()).part("functionSelector"));
else if (functionType.kind() == FunctionType::Kind::Declaration)
else if (
functionType.kind() == FunctionType::Kind::Declaration ||
// In some situations, internal function types also provide the "selector" member.
// See Types.cpp for details.
functionType.kind() == FunctionType::Kind::Internal
)
{
solAssert(functionType.hasDeclaration(), "");
solAssert(functionType.declaration().isPartOfExternalInterface(), "");
define(IRVariable{_memberAccess}) << formatNumber(functionType.externalIdentifier() << 224) << "\n";
}
else
Expand Down
11 changes: 11 additions & 0 deletions test/libsolidity/semanticTests/constants/function_unreferenced.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
contract B {
function g() public {}
}
contract C is B {
bytes4 constant s2 = B.g.selector;
Copy link
Member

Choose a reason for hiding this comment

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

Isn't this exactly the case that already worked before?

function f() external pure returns (bytes4) { return s2; }
}
// ====
// compileViaYul: also
// ----
// f() -> 0xe2179b8e00000000000000000000000000000000000000000000000000000000
14 changes: 14 additions & 0 deletions test/libsolidity/semanticTests/functionTypes/selector_1.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
contract B {
function ext() external {}
function pub() public {}
}

contract C is B {
function test() public returns (bytes4, bytes4, bytes4, bytes4) {
return (B.ext.selector, B.pub.selector, this.ext.selector, pub.selector);
}
}
// ====
// compileViaYul: true
// ----
// test() -> 0xcf9f23b500000000000000000000000000000000000000000000000000000000, 0x7defb41000000000000000000000000000000000000000000000000000000000, 0xcf9f23b500000000000000000000000000000000000000000000000000000000, 0x7defb41000000000000000000000000000000000000000000000000000000000
14 changes: 14 additions & 0 deletions test/libsolidity/semanticTests/functionTypes/selector_2.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
contract B {
function ext() external {}
function pub() public {}
}

contract D {
function test() public returns (bytes4, bytes4) {
return (B.ext.selector, B.pub.selector);
}
}
// ====
// compileViaYul: true
// ----
// test() -> 0xcf9f23b500000000000000000000000000000000000000000000000000000000, 0x7defb41000000000000000000000000000000000000000000000000000000000
26 changes: 26 additions & 0 deletions test/libsolidity/syntaxTests/functionTypes/selectors.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
contract B {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This file does not trigger an error when run through solc --ir (and I think this is also tested for all syntax tests)

Copy link
Contributor

Choose a reason for hiding this comment

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

Do we run IR generator on all syntax tests? That's good to know.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Acutally we do not. We compile to bytecode, but not via yul. I just tried it and it is kind of stuck in the optimizer :(

Copy link
Member

Choose a reason for hiding this comment

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

Do we run IR generator on all syntax tests? That's good to know.

Not yet.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just ran the IR on all syntax tests and found one unimplemented feature: #11024

Copy link
Member

Choose a reason for hiding this comment

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

We did that a few weeks ago already (it used to be more like 9 cases or so), so that issue may be a duplicate...

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Weird... I do remember that we were wondering if it actually works and that I noted that it's a bit weird even in old codegen... but maybe we still didn't do anything about it or even file an issue :-).

function ext() external {}
function pub() public {}
}

contract C is B {
function test() public pure {
B.ext.selector;
B.pub.selector;
this.ext.selector;
pub.selector;
}
}

contract D {
function test() public pure {
B.ext.selector;
B.pub.selector;
}
}
// ----
// Warning 6133: (136-150): Statement has no effect.
// Warning 6133: (160-174): Statement has no effect.
// Warning 6133: (184-201): Statement has no effect.
// Warning 6133: (289-303): Statement has no effect.
// Warning 6133: (313-327): Statement has no effect.