Why are implicit string to bytes conversions allowed in external calls but not internally?

Feedback
1

Take the following code:

"Vm" and "Foo" 
// SPDX-License-Identifier: UNLICENSED
pragma solidity >=0.8.17;

contract Vm {
    event ExpectRevert1();

    function expectRevert() external {
        emit ExpectRevert1();
    }

    event ExpectRevert2(bytes4 message);

    function expectRevert(bytes4 message) external {
        emit ExpectRevert2(message);
    }

    event ExpectRevert3(bytes message);

    function expectRevert(bytes calldata revertData) external {
        emit ExpectRevert3(revertData);
    }
}

contract Foo {
    Vm internal vm = new Vm();

    function foo() external {
        vm.expectRevert("Hello World");
    }
}

Calling foo will emit an ExpectRevert3 event, as I expected. However, if I rewrite the code like this:

Just "Foo" 
contract Foo {
    event ExpectRevert1();

    function expectRevert() public {
        emit ExpectRevert1();
    }

    event ExpectRevert2(bytes4 message);

    function expectRevert(bytes4 message) public {
        emit ExpectRevert2(message);
    }

    event ExpectRevert3(bytes message);

    function expectRevert(bytes calldata revertData) public {
        emit ExpectRevert3(revertData);
    }
    
    function foo() external {
        expectRevert("Hello World");
    }
}

The code does not compile anymore. I am getting this error:

TypeError: No matching declaration found after argument-dependent lookup.
  --> contracts/Foo.sol:44:9:
   |
44 |         expectRevert("Hello World");
   |         ^^^^^^^^^^^^
Note: Candidate:
  --> contracts/Foo.sol:27:5:
   |
27 |     function expectRevert() public {
   |     ^ (Relevant source part starts here and spans across multiple lines).
Note: Candidate:
  --> contracts/Foo.sol:33:5:
   |
33 |     function expectRevert(bytes4 message) public {
   |     ^ (Relevant source part starts here and spans across multiple lines).
Note: Candidate:
  --> contracts/Foo.sol:39:5:
   |
39 |     function expectRevert(bytes calldata revertData) public {
   |     ^ (Relevant source part starts here and spans across multiple lines).

Why does the former code compile, but the latter not? Why is Solidity able to implicitly convert (coerce?) the string literal when the call is external, but when the exact same function is called internally, it does not work?

2 Likes
2

It looked weird to me at first too but after taking a closer look turns out it’s just that in the first example there’s exactly one matching overload and in the second one there is none. It’s easy to see if you try to call each of these functions in isolation:

bytes4 

contract Foo {
    event ExpectRevert2(bytes4 message);

    function expectRevert(bytes4 message) public {
        emit ExpectRevert2(message);
    }

    function foo() external {
        expectRevert("Hello World");
    }
}

Error: Invalid type for argument in function call. Invalid implicit conversion from literal_string "Hello World" to bytes4 requested. Literal is larger than the type.
 --> test.sol:9:22:
  |
9 |         expectRevert("Hello World");
  |                      ^^^^^^^^^^^^^

So this one does not match simply because "Hello World" won’t fit in bytes4. You’d make it convertible if you shortened it to "Hell" :slight_smile:

bytes calldata 

contract Foo {
    event ExpectRevert3(bytes message);

    function expectRevert(bytes calldata revertData) public {
        emit ExpectRevert3(revertData);
    }

    function foo() external {
        expectRevert("Hello World");
    }
}

Error: Invalid type for argument in function call. Invalid implicit conversion from literal_string "Hello World" to bytes calldata requested.
 --> test.sol:9:22:
  |
9 |         expectRevert("Hello World");
  |                      ^^^^^^^^^^^^^

This one doesn’t work because a string literal is not stored in calldata. In internal calls the conversion is obviously not possible. For external calls, the location does not really matter due to ABI-encoding so the compiler is fine with it.

1 Like
3

This is still something that we could improve though so thanks for bringing it up. I always thought that the overloading errors could be more detailed but this is a great concrete example of what’s exactly missing there. The compiler should be able to tell you why each overload does not work. I added a feature request for that: More detail in error messages for ambiguous overloaded calls · Issue #13812 · ethereum/solidity · GitHub.

1 Like
4

Oh, I see! Thanks very much for explaining, it all makes sense now.

I agree.