ethereum
Adjust Yul CFG export output
Depends on https://github.com/ethereum/solidity/pull/15505
This PR fixes a minor inconsistency in the CLI output of the Yul CFG exporter, which was missing a header when exporting Solidity sources. It also removes the targets field from places where this information is irrelevant and adds some tests for the expected output.
@ekpyron I did a change adding the Yul CFG Json output to the needsHumanTargetedStdout because I believe this was also expected from Alejandro's comment. However, I noticed that we don't use that for the IR AST Json and IR Optimized AST Json export, not sure if we should keep them consistent as well.
I updated the comment above since --ast-compact-json does contain the header.
@ekpyron I did a change adding the yul cfg json output to the needsHumanTargetedStdout because I believe this was also expected from Alejandro's comment. However, I noticed that we don't use that for the ast json export, not sure if we should keep them consistent as well.
We can merge this as is in any case - we just shouldn't forget to check the other case as well :-). I'd need to look into it myself - is there a difference between it being file-based or contract-based or not? The CFG is contract based, an AST isn't, so maybe that's why? But yeah, we should check whether what currently happens makes sense. (But merge this regardless)
@ekpyron I did a change adding the yul cfg json output to the needsHumanTargetedStdout because I believe this was also expected from Alejandro's comment. However, I noticed that we don't use that for the ast json export, not sure if we should keep them consistent as well.
We can merge this as is in any case - we just shouldn't forget to check the other case as well :-). I'd need to look into it myself - is there a difference between it being file-based or contract-based or not? The CFG is contract based, an AST isn't, so maybe that's why? But yeah, we should check whether what currently happens makes sense. (But merge this regardless)
I just checked here and it is actually the IR ast exporter (and optimized version) that does not include such file header.
The CFG is contract based, an AST isn't, so maybe that's why?
I'm pretty sure it's not that. The AST export was just done by a contributor, who wasn't aware of such details and we didn't want to spend too much effort reviewing it either not to scare them off. Which resulted in lots of forgotten bits like this or bugs like #14452, or the 25% slowdown due to the outputs being naively always generated rather than using LazyInit which most of the other outputs use.
The CFG is contract based, an AST isn't, so maybe that's why?
I'm pretty sure it's not that. The AST export was just done by a contributor, who wasn't aware of such details and we didn't want to spend too much effort reviewing it either not to scare them off. Which resulted in lots of forgotten bits like this or bugs like #14452, or the 25% slowdown due to the outputs being naively always generated rather than using
LazyInitwhich most of the other outputs use.
Yeah, when I commented, I did so quickly without seeing that it's about the Yul ast, for that it's definitely good/safe to add this as well.