Repository navigation
ethdebug: Require an explicit debug info selection and ast-id - #16961
Conversation
|
cc @cameel |
19a7db3 to
3f08fa2
Compare
bd52feb to
cc712c9
Compare
nikola-matic
left a comment
nikola-matic
left a comment
There was a problem hiding this comment.
I was off for a while so just curious - are we making ast-id an explicit requirement to be more in line with other output requests, or was there a different reason?
| if iszero(condition) { panic_error_0x01() } | ||
| } | ||
| /// @src 1:72:121 | ||
| /// @ast-id 41 @src 1:72:121 |
There was a problem hiding this comment.
I know this wasn't introduced here, but I'd say we should consolidate the printers to have these outputs be consistent (either dump everything into one line, or put each item on a new one).
There was a problem hiding this comment.
Sounds good, though that's something I'd do in a separate PR.
Yes. Context: #16780 (comment). |
| // Without the ethdebug debug info component the pipeline does not carry the | ||
| // info the program output is derived from; the output is empty rather than | ||
| // an error, like the outputs of a contract that produces no bytecode. | ||
| if (!m_debugInfoSelection.ethdebug) | ||
| return {}; |
There was a problem hiding this comment.
Well, this is still not exactly what I meant in #16961 (comment). I think I wasn't clear enough about how that empty output is meant to be produced.
I said "empty output" because I assumed that's what we will get from Yul stack if we don't block it with an error. We should pass on to the user whatever it gives us rather than short-circuit here. Just like yulIR() or object() return whatever we have stored in the Contract object as a result of the compilation.
There was a problem hiding this comment.
I mean, will the output even be completely empty? Now that I think of it, it will just be missing the variable info that goes into context, right? There's still other useful info elsewhere. E.g. instructions, the compilation reference, type of assembly, etc. These things are likely available even without passing any extra debug info through Yul and we should return them if they are. Basically, the content of the ethdebug program output from single- and two-stage compilation should be the same.
We should actually have a simple test verifying that.
There was a problem hiding this comment.
Hmm, I guess I misunderstood that one then. I dropped the early return, now it just returns whatever the assembly gives, instructions and all, only the context is missing.
cc712c9 to
173a029
Compare
Yes! |
7020fa3 to
cd69b55
Compare
|
Looks like some of the tests break the build. |
e445dae to
306db70
Compare
It is fixed now. |
|
@nikola-matic @cameel is this ready? |
306db70 to
48d5fb3
Compare
48d5fb3 to
c6a660d
Compare
cameel
left a comment
cameel
left a comment
There was a problem hiding this comment.
Here's the final batch.
The tests here were quite tedious to review for such a small change. Commandline tests are too verbose and Boost tests are getting repetitive as well. And they both have a lot of overlap. We would be better off with more things being testable using custom TestCases.
We should think about it in the longer term, but in the context of this PR I just suggested a few adjustments and removals. When you apply those, we can move on.
c6a660d to
51fd389
Compare
…debug outputs Selecting an ethdebug output no longer implicitly enables the ethdebug and ast-id components of the debug info selection, just as --ir-optimized does not imply --optimize. Without ethdebug in --debug-info or settings.debug.debugInfo the program outputs are still produced from what the pipeline has, i.e. without the annotations that component adds, the same way the outputs of an interface are produced without bytecode. Semantic debug info transfer depends on @ast-id comments, so ethdebug now also depends on the ast-id component: selecting ethdebug without ast-id is an error. The ethdebug.resources and ethdebug.compilation outputs are derived from analysis results alone and need no selection. The optimizer gate applies whenever ethdebug is selected. The SSA CFG gate applies to the program outputs as well, since that code generator attaches no source locations, which those outputs consist of.
51fd389 to
e6934d8
Compare
solc's develop branch rejects `debugInfo: ["ethdebug"]` since argotorg/solidity#16961 (merged 2026-09-19): "To use 'ethdebug' with settings.debug.debugInfo you must select also 'ast-id'." Every conformance run against a solc built after that date fails at compilation. The adapter now selects both.
3 participants
Selecting an
ethdebugoutput no longer implicitly enables theethdebugandast-idcomponents of the debug info selection, just as--ir-optimizeddoes not imply--optimize: requesting a program ethdebug output without ethdebug in--debug-infoorsettings.debug.debugInfois now an error, in both the CLI and Standard JSON.Semantic debug info transfer depends on
@ast-idcomments, soethdebugnow also depends on theast-idcomponent: selectingethdebugwithoutast-idis an error. The ethdebug.resources and ethdebug.compilation outputs are derived from analysis results alone and still need no selection.The optimizer and SSA CFG gates now apply whenever
ethdebugis selected, not only when a program output is requested.