Conversation
a4f1d9d to
84313bb
Compare
|
@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 |
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. |
b7f7244 to
3548b2d
Compare
84313bb to
198e2e2
Compare
3548b2d to
1f20ccd
Compare
198e2e2 to
9ef1aff
Compare
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 |
1f20ccd to
a7eaac7
Compare
3c1b4fb to
7972770
Compare
a7eaac7 to
7972c51
Compare
7972770 to
6dd31bb
Compare
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. |
6dd31bb to
c38580b
Compare
Depends on #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
targetsfield from places where this information is irrelevant and adds some tests for the expected output.