ethereum / solidity

Solidity, the Smart Contract Programming Language
https://soliditylang.org
GNU General Public License v3.0
22.67k stars 5.62k forks source link

yul literal value as struct of u256 + optional formatting hint #15112

Closed clonker closed 5 days ago

clonker commented 1 month ago

Changed the definition of the solidity::yul::Literal to carry its value not as YulString but as LiteralValue struct, consisting of a u256 and an optional string representation hint. Upon converting from its data back to a string representation, it is first checked if the hint is not empty and in that case, whether value == parseValue(hint).

clonker commented 6 days ago

the tests are now failing due to the change in expression hasher. what do you think @cameel, should I undo it? or rather update test expectations? Intuitively I would say undo, feels more natural to have a true instead of 1. But either is fine for me

cameel commented 6 days ago

Well, it's definitely better to have true there, but why exactly does the BlockHasher affect that? Does the less specific hash result in some extra optimization kicking in that wasn't before? The change did seem correct so maybe the actual problem is outside of BlockHasher. If we locate that then maybe we could keep the change and still keep it at true?

In any case, it would still probably be best to have the same thing in both functions, since I don't see why we'd ever want them to behave differently. They should both hash kind or both hash unlimited(). Perhaps you could just pull the code it into a common local helper for hashing a Literal since they've been getting out of sync all the time in this PR? :)