Yul: add comments on function dispatcher.

[drortirosh]

The yul code is sometimes hard to read.
When optimized, the main dispatch looks like:

                    let _2 := 0
                    switch shr(224, calldataload(_2))
                    case 0x1a212539 {
                        if callvalue() { revert(_2, _2) }
                  ...

It is very nice that the public functions are inlined. but it makes the code very hard to read.
when the code is unoptimized, the code is slightly more readable:

                    switch shr(224, calldataload(0))
                    case 0x1a212539 { external_fun_xxx() }
                    case 0x1e13bcd2 { external_fun_yyy() }

Suggestion:

add a comment near the "case xxxx", with the external method name (or its full signature)

[github-actions]

This issue has been marked as stale due to inactivity for the last 90 days.
It will be automatically closed in 7 days.

[drortirosh]

didn't get any comment on this.
I think it should be quite easy to implement, as currently it is quite annoying to read through the assembly code.

[cameel]

This sounds reasonable and very helpful but there are significant obstacles unfortunately.

Just inserting a comment with the signature before every case in IRGenerator::dispatchRoutine() would be trivial.

The problem is that the optimizer currently does not preserve comments. That's because they're not a part of the AST and the AST is the only thing being carried over between steps. So a proper solution would require something like #13865 but for Yul AST and that's big for a prerequisite. I don't really see us working on something like that any time soon unfortunately, especially if the only motivation is only a small readability improvement. Not saying it's not a problem, just one where the solution seems quite expensive compared to potential gains.

Some post-processing solution that can insert such comments after the fact based on contract's ABI sounds more viable but that also seems more appropriate as a part of some external tool. Maybe an IDE.

For these reasons I'm going to close it as "wontfix" for the time being.

[drortirosh]

actually, there is a simpler solution: the comment can be re-constructed from the methodsig.

[cameel]

That's pretty much what I meant by post-processing based on ABI.

But now that I think of it, the selector could also be a part of the debug info (the comments that you can control with --debug-info). If we set the source location of the selector literal to point at the signature in the Solidity source, you'd not even need any new piece of debug info, you'd get the snippet printed already with the current system and it would not really be incorrect - the selector is generated from that.

That actually sounds nice and easy so I'm going to reopen. But I still can't really assign much priority too this.

[github-actions]

This issue has been marked as stale due to inactivity for the last 90 days.
It will be automatically closed in 7 days.

[github-actions]

This issue has been marked as stale due to inactivity for the last 90 days.
It will be automatically closed in 7 days.

[drortirosh]

actually, a reference to the solidity source is not needed, merely adding the methodsig above the "case", to make the "yul" readable...

Yes, it is possible to be done "offline", but I think it makes sense for the compiler itself to generate these comments.

e.g., a script to process "forge inspect" output:

#!/usr/bin/perl
#"optimized yul" code generates just one big function, with all exported functions code together.
# this script adds a comment with the method signature just before the "case {methodsig}" of each method

$contractName = shift;

#die "usage: $0 {contractName}\n" unless $contractName;

$names = `forge inspect $contractName methodIdentifiers`;
die "\n" if $?;

while ( $names =~ s/"(.*?)": "(.*?)"// ) {
	$methodSigs{$2}=$1;
}


open OUT, "forge inspect $contractName irOptimized|";

while (<OUT>) {

    if ( /(.*)case (?:0x)?(\w+)/ && $methodSigs{$2} ) {
        print "$1// function $methodSigs{$2} :\n";
    }
    print;
}

e.g.: for an arbitrary contract, this is the added comment:

                    // function balanceOf(address) :
                    case 0x70a08231 {
                        if callvalue() { revert(0, 0) }
                        if slt(add(calldatasize(), 0xfffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffc), 32) { revert(0, 0) }
                        mstore(0, and(abi_decode_address(), 0xffffffffffffffffffffffffffffffffffffffff))
                        mstore(32, 0)
                        let _53 := sload(keccak256(0, 64))
                        let memPos_6 := mload(64)
                        mstore(memPos_6, _53)
                        return(memPos_6, 32)
                    }