solidity icon indicating copy to clipboard operation
solidity copied to clipboard

[DO NOT MERGE] Remove source locations on Yul optimizer function deduplication for testing.

Open ekpyron opened this issue 10 months ago • 1 comments

@veniger As we talked about, this is a quick and dirty branch that removes source locations whenever functions are deduplicated by the Yul optimizer. I'm a bit sceptical, whether this is really what we want (and it may still actually be the libevmasm optimizer that's doing the deduplication in the relevant cases), but I'd be curious to hear, if this actually changes the cases you're struggling with - if not, it may be interesting to post the sources for such a case, s.t. we can look into it in more detail.

Although, weirdly, I just see in some tests that - while it actually just removes source locations - in fact seems to sometimes add some - I guess since we'll fall back to previous ones, I'll still need to look into that to avoid that.

ekpyron avatar Feb 26 '25 18:02 ekpyron

I think i created a minimal example that shows the problems here. If you revert with Err1 some instructions get mapped to line 26 and some get mapped to line 49. I can demonstrate this on the next call if needed.

In this case I'm not even sure that the removal of deduplicated source locations would be enough, we would need to store multiple source locations, since no instruction here can be used to distinguish between the reverts (except falling back to the AST and figuring it out that way, which wouldn't be viable for larger contracts)

settings: compiler version 0.8.26, via-ir enabled, --optimize-steps 30

// SPDX-License-Identifier: GPL-3.0
pragma solidity >=0.8.16 <0.9.0;

contract Example0
{
    error Err0(uint256 a, address addr);
    error Err1(uint256 a, uint256 b);
    error Err2(uint256 a, uint256 b);

    uint256 public x;

    function set(uint256 nx) public
    {
        x = nx;
    }

    function test0(uint256 a) public
    {
        if(a >= 0 && a < 10)
        {
            revert Err0(a + x, msg.sender);
        }
        
        if(a >= 10 && a < 20)
        {
            revert Err1(x, a);
        }
        
        if(a >= 20)
        {
            revert Err2(x, a);
        }
        
        x = a;

    }

    function test1(uint256 a, uint256 b) public
    {
        uint256 c = a + b;

        if(c >= 0 && c < 10)
        {
            revert Err0(a + x, msg.sender);
        }
        
        if(c >= 10 && c < 20)
        {
            revert Err1(x, a);
        }
        
        if(c >= 20)
        {
            revert Err2(x, a);
        }
        
        x = c;

    }


}

veniger avatar Mar 04 '25 15:03 veniger

This pull request is stale because it has been open for 14 days with no activity. It will be closed in 7 days unless the stale label is removed.

github-actions[bot] avatar Mar 19 '25 12:03 github-actions[bot]

This pull request was closed due to a lack of activity for 7 days after it was stale.

github-actions[bot] avatar Mar 26 '25 12:03 github-actions[bot]