Cannot use word `_offset` in local variable name in inline assembly because of old 0.7.x syntax

Description

I have the following Solidity code with the inline assembly part that does not compile. See example below. It always return me the following error:

DeclarationError: Identifier not found. Use ".slot" and ".offset" to access storage variables.
  --> contracts/Playground.sol:23:24:
   |
23 |                 revert(resultdata_offset, resultdata_size)
   |                        ^^^^^^^^^^^^^^^^^

I am wondering if this is normal. As far as I am aware, this is related to the old syntax from v0.7.x, where the suffixes _slot and _offset were used to access the slot number of a state variable (or its offset position in a storage slot). This was replaced by .slot and .offset.

https://docs.soliditylang.org/en/v0.8.21/070-breaking-changes.html#inline-assembly

However, in this instance, I am just writing the local variable in assembly in snake_case with the word _offset and the compiler seems to confuse this with the old syntax. I am not sure if this is a bug or an inaccuracy in the error reported. If the two words _slot and _offset are not allowed for local variables in inline assembly, it would be useful I guess to mention it somewhere in the docs maybe?

Environment

Compiler version: 0.8.21
Target EVM version (as per compiler settings): default
Framework/IDE (e.g. Truffle or Remix): Remix

Steps to Reproduce

// SPDX-License-Identifier: GPL-3.0
pragma solidity 0.8.21;

contract Example {

    address someAddress = 0xCAfEcAfeCAfECaFeCaFecaFecaFECafECafeCaFe;

    bytes4 private constant _SOME_SELECTOR = bytes4(keccak256("hey()"));

    function test() public payable {
        (bool success, bytes memory result) = someAddress.call(
            abi.encodePacked(_SOME_SELECTOR, msg.sender, msg.value)
        );

        assembly {
            // `mload(result)` -> offset in memory where `result.length` is located
            // `add(result, 32)` -> offset in memory where `result` data starts
            let resultdata_size := mload(result)
            let resultdata_offset := add(result, 32)

            // if call failed, revert
            if eq(success, 0) {
                revert(resultdata_offset, resultdata_size)
            }

            // otherwise return the data returned by the external call
            return(resultdata_offset, resultdata_size)
        }
    }

}

Thanks for the report!

I investigated this and it seems to be unintended. Both the comment that suggested the check in the PR that introduced this breaking change and the way the check is implemented indicate that this was meant to be a helpful suggestion to be shown only when the identifier is undeclared. And we only have test cases covering the cases where it is undeclared (storage_reference_old.sol) or refers to a storage variable (storage_reference_old_shadow.sol). The case where it is shadowed by a local assembly variable is not covered.

The direct cause is that ReferencesResolver seems unable to resolve local assembly variables to their declarations in the same block (not sure if that in itself is a bug or not). However, for those that do not have the suffix, it just continues without doing anything. It's only those with the suffix that trigger the error on the assumption that the declaration is missing.

By the way, this problem does not exist for Solidity variables, so until we fix this, the workaround is to declare it outside of the assembly block if you really want it to have that name.

ethereum / solidity