Fix: Also replace tags in the list of tags referenced from outside.#6047
Fix: Also replace tags in the list of tags referenced from outside.#6047
Conversation
chriseth
force-pushed
the
fixTagReplacement
branch
2 times, most recently
from
abb7116 to
1e58244
Compare
Codecov Report
@@ Coverage Diff @@
## develop #6047 +/- ##
===========================================
+ Coverage 88.34% 88.34% +<.01%
===========================================
Files 361 361
Lines 34858 34862 +4
Branches 4128 4130 +2
===========================================
+ Hits 30796 30800 +4
Misses 2685 2685
Partials 1377 1377
|
chriseth
force-pushed
the
fixTagReplacement
branch
from
1e58244 to
f511d5c
Compare
| for (auto const& replacement: dedup.replacedTags()) | ||
| { | ||
| tagReplacements[replacement.first] = replacement.second; | ||
| assertThrow( |
libevmasm/Assembly.cpp
Outdated
| tagReplacements.insert(dedup.replacedTags().begin(), dedup.replacedTags().end()); | ||
| for (auto const& replacement: dedup.replacedTags()) | ||
| { | ||
| tagReplacements[replacement.first] = replacement.second; |
There was a problem hiding this comment.
Should this assert that replacement.first doesn't exist yet?
There was a problem hiding this comment.
We can try and see what happens...
Changelog.md
Outdated
| * ABIEncoderV2: Fix internal error related to ecrecover. | ||
| * ABIEncoderV2: Fix internal error related to mappings as library parameters. | ||
| * Inline Assembly: Proper error message for missing variables. | ||
| * Optimizer: Fix bug related to unused tag removal across assemblies. |
There was a problem hiding this comment.
If this never had any adverse effect we should mention that.
There was a problem hiding this comment.
Reworded into mentioning "internal error", I think this should be sufficient. It always resulted in a "tag not found" - tag numbers are not re-used, only replaced.
There was a problem hiding this comment.
I think we should still mention it didn't generate invalid code to avoid scaring people.
chriseth
force-pushed
the
fixTagReplacement
branch
2 times, most recently
from
2c51fb2 to
f92c129
Compare
chriseth
force-pushed
the
fixTagReplacement
branch
from
f92c129 to
7f6f958
Compare
| map<u256, u256> Assembly::optimiseInternal( | ||
| OptimiserSettings const& _settings, | ||
| std::set<size_t> const& _tagsReferencedFromOutside | ||
| std::set<size_t> _tagsReferencedFromOutside |
There was a problem hiding this comment.
No, it is not needed in the caller. The function returns the replacements themselves and then the caller takes them into account.
3 participants
Hopefully the last fix needed by #5102