Subassembly syntax for plain EVM assembly tests #16021
Conversation
| BOOST_THROW_EXCEPTION(std::runtime_error(formatError("The subassembly ID must be a hex number prefixed with '0x'."))); | ||
|
|
||
| subassemblyID.remove_prefix("0x"s.size()); | ||
| codeJSON.push_back({{"name", "PUSH " + pushType}, {"value", subassemblyID}}); |
There was a problem hiding this comment.
I was very suspicious of the subassemblyID and thought it might lead to lifetime issues. Turns out, nlohmann_json copies the thing. So this is fine!
There was a problem hiding this comment.
Yeah, any other behavior here would be unsafe. Storing references to something you don't own and don't know the lifetime of would be a pretty bad design decision.
There was a problem hiding this comment.
Though I have to say that this PR took much more time than I expected, mainly due to me having to fight with dangling references, so it's a valid concern in general.
We don't normally make heavy use of string_view, but here I tried to use it more to avoid copies. Turns out there are many new ways to silently shoot yourself that I just didn't have to think about before. For example, even if your object owns both the parent string and the string_views pointing at it, it's not necessarily enough on its own to prevent dangling references. When you copy the object, your string_views still point at the original. If you move it, you'll have a bad time :)
I also learned that taking references to nested parts of a nlohmann json objects leads to weird results. They seem to be very easily invalidated when modifying even unrelated parts of the object. I initially used a stack of references to subassemblies, but had to switch to a recursive implementation and refactor the parsing a bit due to that.
There was a problem hiding this comment.
Yeah that sounds painful.
The way I personally like the most to deal with nlohmann json is to set up a corresponding hierarchy of POD structs with to_json methods in the same namespace (or from_json). Then you can just build up the object with stl tools and implicitly convert it to json in the end - ADL and the json lib does the rest.
cameel
force-pushed
the
subassemblies-in-evm-assembly-tests
branch
from
1c1e58d to
12f8750
Compare
Follow-up to #16012.
Adds support for plain assembly tests that contain more than one assembly. This resolves the limitation I mentioned in #16012 (comment) and therefore will allow proper testing of the constant optimizer in #15935.