eof: new contract creation

[rodiazet]

Depends on #15456. Merged
Depends on #15521 Dropped
Depends on: #15529 Merged
Depends on #15535 Merged
Depends on #15536 Merged

[github-actions]

Thank you for your contribution to the Solidity compiler! A team member will follow up shortly.

If you haven't read our contributing guidelines and our review checklist before, please do it now, this makes the reviewing process and accepting your contribution smoother.

If you have any questions or need our help, feel free to post them in the PR or talk to us directly on the #solidity-dev channel on Matrix.

[cameel]

I only skimmed through the PR for now. Here's some initial feedback.

My main question would be whether it wouldn't make more sense to deal with loadimmutable/setimmutable in a separate PR first. It's related dataloadn and looks like creation depends on immutables to some extent.

[cameel]

Approving, since only a few cosmetic things are left now.

[cameel]

This actually looks a bit ambiguous, as if EOFCREATE was getting only one argument.

How about we change the notation to for immediates in asm output to {} in AssemblyItem::toAssemblyText)? Then we'd get this:

Suggested change

	// eofcreate(0)
	// eofcreate{0}

I'd do that for RETURNCONTRACT and AUXDATALOADN as well. And I'd also return true for RETURNCONTRACT from AssemblyItem::canBeFunctional(). Is there actually a good reason why it's not like that already?

[rodiazet]

I will change to brackets in AssemblyItem::toAssemblyText. Fot DATALOADN I would do this in a separated PR #15545.

Regarding ssemblyItem::canBeFunctional()
It depends how you define functional context. I assumed that if there is no return value it's not functional.

[cameel]

It depends how you define functional context. I assumed that if there is no return value it's not functional.

canBeFunctional() determines whether the parameters of the item should be printed in parentheses after it (in functional style) or as separate instructions before it.

At least that's how I understand it from how Functionalizer uses it: https://github.com/ethereum/solidity/blob/879d8e69f884419a9bc4a0cfe17a6d73ae14d836/libevmasm/Assembly.cpp#L324-L345

Here m_pending is a list of previous expressions, which are assumed to be the arguments.

I think that we want true for almost everything. The only things for which we return false seem to be PUSH and DUP instructions and Tag, AssignImmutable, VerbatimBytecode items. Not sure about the reasons for all of them but e.g. DUP is defined as taking the whole top of the stack as arguments (so e.g. DUP16 takes 17 arguments).

The closest thing we have to RETURNCONTRACT is probably RETURN and for that we return true.

[cameel]

BTW, I wonder if it wouldn't be better to show sub names like we do for dataOffset/dataSize instead of the numeric container IDs.

So for example in the assembly we would print eofcreate{sub_0} rather than eofcreate{0}.

Though I guess with EOF the ID approach is actually viable because we can't have nesting so maybe the current approach is still good. It's something to consider though.

[rodiazet]

First fixed. I will check the later.

[cameel]

Looks good, can be merged as soon as #15536 goes in.

In the meantime... a few more nitpicks :P

[cameel]

#15536 has been merged. Please rebase.