Remove EOF#16584
Conversation
|
This pull request is stale because it has been open for 14 days with no activity. |
github-actions
Bot
added
the
stale
rodiazet
removed
the
stale
rodiazet
force-pushed
the
remove-eof
branch
2 times, most recently
from
0067b76 to
9685176
Compare
rodiazet
requested review from
cameel,
clonker,
matheusaaguiar,
nikola-matic and
r0qs
rodiazet
added
medium effort
nikola-matic
left a comment
nikola-matic
left a comment
There was a problem hiding this comment.
Niceeee. By the way - we have multiple references to EOF in the changelog, so in this case it would appropriate to add one saying it was removed.
| if (deduplicator.deduplicate()) | ||
| { | ||
| for (auto const& replacement: deduplicator.replacedTags()) | ||
| { | ||
| assertThrow( | ||
| replacement.first <= std::numeric_limits<size_t>::max() && replacement.second <= std::numeric_limits<size_t>::max(), | ||
| OptimizerException, | ||
| "Invalid tag replacement." | ||
| ); | ||
| assertThrow( | ||
| !tagReplacements.count(replacement.first), | ||
| OptimizerException, | ||
| "Replacement already known." | ||
| ); | ||
| tagReplacements[replacement.first] = replacement.second; | ||
| if (_tagsReferencedFromOutside.erase(static_cast<size_t>(replacement.first))) | ||
| _tagsReferencedFromOutside.insert(static_cast<size_t>(replacement.second)); | ||
| } | ||
| count++; | ||
| } | ||
| } |
| /// Appends EOF contract return instruction which returns a subcontainer ID (_containerID) with auxiliary data filled in. | ||
| virtual void appendReturnContract(ContainerID _containerID) = 0; | ||
| /// Appends data to the very end of the bytecode. Repeated calls concatenate. | ||
| /// EOF auxiliary data in data section and the auxiliary data are different things. |
There was a problem hiding this comment.
| /// EOF auxiliary data in data section and the auxiliary data are different things. |
rodiazet
force-pushed
the
remove-eof
branch
4 times, most recently
from
edf4bca to
31a616e
Compare
matheusaaguiar
left a comment
matheusaaguiar
left a comment
There was a problem hiding this comment.
I found some other places where we have EOF related code, but looks good.
There was a problem hiding this comment.
We can also remove this.
There was a problem hiding this comment.
Good point. Haven’t noticed this one.
| } | ||
| } | ||
|
|
||
| solAssert(message.kind != EVMC_EOFCREATE); |
There was a problem hiding this comment.
Can we just remove the enum value from evmc.h ?
Also, here in line 1383 we have this enum value in a switch case, so I think we should either assert or remove it there.
There was a problem hiding this comment.
I will assert because evmc.h is a header copied from evmone implementation.
There was a problem hiding this comment.
Can we just remove the enum value from
evmc.h?
I'd keep the evmc modifications to a minimum. This is external code. The more independent changes we have to track the harder it is to update. And it will be gone when we update anyway.
There was a problem hiding this comment.
I haven’t change evmc.h.
There was a problem hiding this comment.
Well, just assert then since EOF is dead?
There was a problem hiding this comment.
I’m lost :) So what exactly?
There was a problem hiding this comment.
I guess no harm in asserting, but not necessary too.
There was a problem hiding this comment.
| std::bitset<3> value; |
rodiazet
force-pushed
the
remove-eof
branch
2 times, most recently
from
91cdfae to
485f165
Compare
5 participants
This PR removes EOF entirely from the compiler. Last commit also removes redundant vector storing code sections in the Assembly class. It’s splitted into a couple of separated commits. Each commits covers changes in each solidity library and tests. It’s better to review each commit separately.
Closes: #16593