ABI Encode Unit Type

[Maltby]

What was wrong?

closes #442

How was it fixed?

To-Do

• OPTIONAL: Update Spec if applicable

• Add entry to the release notes (may forgo for trivial changes)

• Clean up commit history

[sbillig]

@Maltby I went down this road once, and abandoned it because our compiler pipeline wasn't powerful enough to solve this in a satisfactory way. In yul, functions that don't return anything really don't return anything, so they can't be used in a position that expects some kind of value. There's no concept of a zero-sized value (everything is a u256).

Fe code like this:

fn f(a: u256, b: ()) -> u256:  return a
fn g():  pass
f(10, g())

is currently compiled to something like this in yul:

function f($a, $b) -> return_val {
    return_val := $a
    leave
}
function g() -> return_val {
    return_val := 0x0
    leave
}
pop(f(10, g()))

which only works because g returns a value. If we get rid of g's return value, we can't use g() as an argument to f. We could modify the current yulgen so that it calls g() first, then calls f(10, 0) or something, but I think that the ideal solution here would be for all zero-sized types (including unit) to not exist at all by the time we reach the code generation (yulgen) phase. In a pre-yul optimization, we could eliminate all unit-type fn args, all unit-type struct fields, and zero-sized structs altogether.

The "middle" of the compiler is currently being revamped (#661, and a follow-up pr that'll generate yul from mir). My tentative suggestion is that unit types could be eliminated in an optimization pass on mir, and the mir-to-yul codegen could assume that unit types don't exist. Thoughts, @Y-Nak?

[Y-Nak]

Yeah, I agree with the opinion. We should/could remove unit types from both function call-sites and def-sites.
Also, I assume the removal would be done in the legalization pass because its removal would be specific for EVM and should be independent of MIR.
So the compilation passes in my mind currently would look like the below.

Of course, Target agnostic transformations are optional.

[Maltby]

@sbillig @Y-Nak thanks for the feedback and info! I'll look into removing the unit type within MIR.

[Y-Nak]

@Maltby I think it'd be better to fix this issue in codegen crate, which will be introduced in #676.
I'm still working on it, and I could finish it within a week if there are not so many problems. So could you wait for its completion, please?

[Maltby]

@Y-Nak Ok, I'll wait for #676 to be merged. No rush at all! Thanks again for the guidance.

[Y-Nak]

Hi @Maltby, I'm so sorry for the late response.
I've struggled so much through #676. Also, I fixed this issue in #676.
I should have let you know about this, I'm really sorry for my rudeness.

[Maltby]

@Y-Nak No worries! #676 looks like a challenging PR!