hyperledger-solang / solang

Solidity Compiler for Solana and Polkadot
https://solang.readthedocs.io/
Apache License 2.0
1.27k stars 215 forks source link

`_;` gets parsed as an `Expression::Variable` #1600

Closed alexfertel closed 11 months ago

alexfertel commented 11 months ago

Problem

tl;dr _; gets parsed as an Expression::Variable, which means that when the parse tree is formatted using Display, it gets emitted as _, without the trailing semicolon. Since https://github.com/ethereum/solidity/releases/tag/v0.4.0, _ must be followed by a ;.

Repro

Given this contract:

// SPDX-License-Identifier: UNLICENSED
pragma solidity 0.8.0;

contract Foo {
    modifier bar() {
        _;
    }

    function baz() external bar {}
}

The formatted representation of the parse tree is:

pragma solidity 0.8.0;
contract Foo {modifier bar() {_} function baz() external bar {}}

Solution

  1. Parse it as a Statement::VariableDefinition, like so:
                let variable = Statement::VariableDefinition(
                    variable_loc,
                    VariableDeclaration {
                        loc: variable_loc,
                        ty: Expression::Variable(Identifier {
                            loc: variable_loc,
                            name: "_".to_owned(),
                        }),
                        storage: None,
                        name: None,
                    },
                    None,
                );

since the Display implementation of this node does append a ;.

  1. Add a new Statement::Placeholder node, which represents a _; statement.
  2. Special case _ in the display implementation.
seanyoung commented 11 months ago

The parser is correct here: it cannot know that _ is a placeholder statement. In regular functions, you can have a variable called _. This is determined during semantic analysis. The problem here is that the code formatter does follow an expression statement with a ;, fixed in #1601

alexfertel commented 11 months ago

In regular functions, you can have a variable called _.

I tried declaring one in Remix, but couldn't:

Screenshot 2023-11-26 at 11 01 16 Screenshot 2023-11-26 at 11 00 49

You can see on the second screenshot that _ is a reserved name, so I don't think you can have variables named like that.

This is determined during semantic analysis.

Could you expand on this? Do you mean that the semantic analysis phase is what determines if _ is a placeholder statement or not?

The problem here is that the code formatter does follow an expression statement with a ;, fixed in https://github.com/hyperledger/solang/pull/1601

Nice! I think that works 👍🏻.

seanyoung commented 11 months ago

I tried declaring one in Remix, but couldn't:

If you select solc 0.5 or 0.6 it works fine. Every solc release, a bunch of minor things are changed which breaks backwards compatibility.

seanyoung commented 11 months ago

Could you expand on this? Do you mean that the semantic analysis phase is what determines if _ is a placeholder statement or not?

Yes, the parser generates a parse tree based of the grammar. The semantic analysis stage resolves all the variables and functions calls (and much more).