Skip to content

Make ethdebug inputs/outputs consistent#16565

Merged
nikola-matic merged 3 commits intodevelopfrom
make-ethdebug-consistent-with-existing-conventions
Apr 17, 2026
Merged

Make ethdebug inputs/outputs consistent#16565
nikola-matic merged 3 commits intodevelopfrom
make-ethdebug-consistent-with-existing-conventions

Conversation

@nikola-matic
Copy link
Copy Markdown
Contributor

@nikola-matic nikola-matic commented Mar 31, 2026

Closes #16460

  • docs
  • changelog?

@nikola-matic nikola-matic force-pushed the make-ethdebug-consistent-with-existing-conventions branch from 2e64751 to a1c530b Compare April 1, 2026 12:45
m_options.compiler.outputs.ethdebug ||
m_options.compiler.outputs.ethdebugRuntime ||
m_options.compiler.outputs.ethdebugProgram ||
m_options.compiler.outputs.ethdebugProgramRuntime ||
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nikola-matic What was the thing you wanted to do with --stop-after? The way I see it, you only need to touch CompilerStack::PipelineConfig. It defines which parts of the pipeline should be enabled. Perhaps we just need separate entry for analysis in it.

TBH these two things are kinda redundant and we should probably just integrate stopAfter into PipelineConfig. We could do that just fine without changing compiler's interface. Both tell CompilerStack which stages to execute, they were just created at different times for different purposes.

But not sure if you want to get into that now. For the purpose of this PR I think it will be enough to just add relevant fields to PipelineConfig and if they happen to overlap with stopAfter, just use asserts and validations to ensure they do not contradict each other.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, I saw it after the call. I'll ping you for review once the PR is in better shape.

@nikola-matic nikola-matic force-pushed the make-ethdebug-consistent-with-existing-conventions branch 6 times, most recently from 6eb6ffd to 1986be6 Compare April 6, 2026 18:21
@nikola-matic nikola-matic marked this pull request as ready for review April 8, 2026 07:00
@nikola-matic nikola-matic requested a review from cameel April 8, 2026 07:00
Copy link
Copy Markdown
Member

@clonker clonker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice cleanup! Mainly two things though:

  • the requiresFullCompilation / skip link() thing seems orthogonal and should probably be extracted into its own pr unless i'm missing the point here
  • global ethdebug output assumes analysis succeeded. this should be tested and guarded in the implementation

the rest is nits and a design question about the standard json interface (the * selection for global settings seems a bit awkward to me) as well as if we want to add a --ethdebug-full or just --ethdebug output as short-hand for the common case.

// evm.gasEstimates - Function gas estimates
// yulCFGJson - Control Flow Graph (CFG) of the Single Static Assignment (SSA) form of the contract (experimental)
//
// Global level (needs "*" as file name and "*" as contract name):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure why it needs "*" for file and contract name, can this be solved differently? wouldn't it be better as flat top-level key if it's actually global?

+-----------------------+--------------------------+------------------+-----------------------------------------------------------------------------------------------------------------------------------------+
| Non-mainnet EVMs | ``evm`` | yes | ``--evm-version <version name>`` |
+-----------------------+--------------------------+------------------+-----------------------------------------------------------------------------------------------------------------------------------------+
| Ethdebug | ``ethdebug`` | no | ``--ethdebug-resources``, ``--ethdebug-compilation``, ``--ethdebug-program``, ``--ethdebug-program-runtime``, ``--debug-info ethdebug`` |
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's the default now, actually? is that somehow reflected in the docs?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Default in what way?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I confused myself here. What I meant were the default settings for emitting debug info. Which, obviously, is to not emit anything unless debug info output is requested. So nothing to change or even discuss in that regard.

Tangentially (and where my confusion came from): I've always found it a bit funny that the default behavior of IR output is to attach source location info.

Comment thread libevmasm/Ethdebug.h Outdated
Comment thread libevmasm/Ethdebug.h Outdated
Comment on lines +840 to +841
if (requiresFullCompilation)
this->link();
Copy link
Copy Markdown
Member

@clonker clonker Apr 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how is this (as well as the perf optimization above) related to ethdebug? isn't this orthogonal?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I guess you could say so, although it was a requirement in the issue. E.g. requiresting ethdebug.compilation, which just gives the compiler details shouldn't really trigger full compilation.

Comment thread solc/CommandLineInterface.cpp Outdated
Comment thread solc/CommandLineParser.cpp Outdated
Comment on lines +1584 to +1585
if (m_options.output.viaSSACFG)
solUnimplemented("ethdebug is not yet supported with --" + g_strViaSSACFG + ".");
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

out of curiosity, what happens if you do request it? at least some information should propagate, i think

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll check and report back :)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test.sol:

contract C
{
        uint256 x = 1;

        function foo() public view returns(uint256) {
                return x;
        }
}

Running solc --experimental --ethdebug-program --via-ssa-cfg test.sol outputs the following:

======= test.sol:C =======
Debug Data (ethdebug/format/program):
{"contract":{"definition":{"source":{"id":0}},"name":"C"},"environment":"create","instructions":[{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":0,"operation":{"arguments":["0x80"],"mnemonic":"PUSH1"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":2,"operation":{"arguments":["0x40"],"mnemonic":"PUSH1"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":4,"operation":{"mnemonic":"MSTORE"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":5,"operation":{"mnemonic":"CALLVALUE"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":6,"operation":{"arguments":["0x22"],"mnemonic":"PUSH1"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":8,"operation":{"mnemonic":"JUMPI"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":9,"operation":{"arguments":["0x0e"],"mnemonic":"PUSH1"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":11,"operation":{"arguments":["0x8b"],"mnemonic":"PUSH1"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":13,"operation":{"mnemonic":"JUMP"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":14,"operation":{"mnemonic":"JUMPDEST"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":15,"operation":{"arguments":["0x14"],"mnemonic":"PUSH1"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":17,"operation":{"arguments":["0x26"],"mnemonic":"PUSH1"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":19,"operation":{"mnemonic":"JUMP"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":20,"operation":{"mnemonic":"JUMPDEST"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":21,"operation":{"arguments":["0x0142"],"mnemonic":"PUSH2"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":24,"operation":{"arguments":["0x0097"],"mnemonic":"PUSH2"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":27,"operation":{"mnemonic":"DUP3"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":28,"operation":{"mnemonic":"CODECOPY"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":29,"operation":{"arguments":["0x0142"],"mnemonic":"PUSH2"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":32,"operation":{"mnemonic":"SWAP1"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":33,"operation":{"mnemonic":"RETURN"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":34,"operation":{"mnemonic":"JUMPDEST"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":35,"operation":{"arguments":["0x2c"],"mnemonic":"PUSH1"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":37,"operation":{"mnemonic":"JUMP"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":38,"operation":{"mnemonic":"JUMPDEST"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":39,"operation":{"arguments":["0x40"],"mnemonic":"PUSH1"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":41,"operation":{"mnemonic":"MLOAD"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":42,"operation":{"mnemonic":"SWAP1"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":43,"operation":{"mnemonic":"JUMP"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":44,"operation":{"mnemonic":"JUMPDEST"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":45,"operation":{
"mnemonic":"PUSH0"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":46,"operation":{"mnemonic":"DUP1"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":47,"operation":{"mnemonic":"REVERT"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":48,"operation":{"mnemonic":"JUMPDEST"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":49,"operation":{"mnemonic":"PUSH0"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":50,"operation":{"mnemonic":"SHL"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":51,"operation":{"mnemonic":"SWAP1"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":52,"operation":{"mnemonic":"JUMP"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":53,"operation":{"mnemonic":"JUMPDEST"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":54,"operation":{"mnemonic":"PUSH0"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":55,"operation":{"mnemonic":"NOT"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":56,"operation":{"mnemonic":"SWAP2"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":57,"operation":{"arguments":["0x3f"],"mnemonic":"PUSH1"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":59,"operation":{"mnemonic":"SWAP1"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":60,"operation":{"arguments":["0x30"],"mnemonic":"PUSH1"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":62,"operation":{"mnemonic":"JUMP"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":63,"operation":{"mnemonic":"JUMPDEST"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":64,"operation":{"mnemonic":"DUP3"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":65,"operation":{"mnemonic":"NOT"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":66,"operation":{"mnemonic":"SWAP1"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":67,"operation":{"mnemonic":"SWAP2"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":68,"operation":{"mnemonic":"AND"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":69,"operation":{"mnemonic":"SWAP2"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":70,"operation":{"mnemonic":"AND"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":71,"operation":{"mnemonic":"OR"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":72,"operation":{"mnemonic":"SWAP1"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":73,"operation":{"mnemonic":"JUMP"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":74,"operation":{"mnemonic":"JUMPDEST"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":75,"operation":{"mnemonic":"SWAP1"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":76,"operation":{"mnemonic":"JUMP"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":77,"operation":{"mnemonic":"JUMPDEST"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":78,"operation":{"mnemonic":"SWAP1"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":79,"operation":{"mnemonic":"JUMP"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":80,"operation":{"mnemonic":"JUMPDEST"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":81,"operation":{"mnemonic":"SWAP1"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":82,"operation":{"mnemonic":"JUMP"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":83,"operation":{"mnemonic":"JUMPDEST"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":84,"operation":{"arguments":["0x5a"],"mnemonic":"PUSH1"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":86,"operation":{"mnemonic":"SWAP1"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":87,"operation":{"arguments":["0x4a"],"mnemonic":"PUSH1"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":89,"operation":{"mnemonic":"JUMP"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":90,"operation":{"mnemonic":"JUMPDEST"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":91,"operation":{"arguments":["0x61"],"mnemonic":"PUSH1"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":93,"operation":{"mnemonic":"SWAP1"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":94,"operation":{"arguments":["0x50"],"mnemonic":"PUSH1"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":96,"operation":{"mnemonic":"JUMP"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":97,"operation":{"mnemonic":"JUMPDEST"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":98,"operation":{"arguments":["0x68"],"mnemonic":"PUSH1"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":100,"operation":{"mnemonic":"SWAP1"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":101,"operation":{"arguments":["0x4d"],"mnemonic":"PUSH1"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":103,"operation":{"mnemonic":"JUMP"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":104,"operation":{"mnemonic":"JUMPDEST"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":105,"operation":{"mnemonic":"SWAP1"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":106,"operation":{"mnemonic":"JUMP"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":107,"operation":{"mnemonic":"JUMPDEST"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":108,"operation":{"mnemonic":"SWAP1"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":109,"operation":{"mnemonic":"JUMP"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":110,"operation":{"mnemonic":"JUMPDEST"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":111,"operation":{"mnemonic":"SWAP1"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":112,"operation":{"arguments":["0x76"],"mnemonic":"PUSH1"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":114,"operation":{"mnemonic":"SWAP1"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":115,"operation":{"arguments":["0x53"],"mnemonic":"PUSH1"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":117,"operation":{"mnemonic":"JUMP"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":118,"operation":{"mnemonic":"JUMPDEST"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":119,"operation":{"arguments":["0x7d"],"mnemonic":"PUSH1"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":121,"operation":{"mnemonic":"SWAP1"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":122,"operation":{"arguments":["0x6b"],"mnemonic":"PUSH1"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":124,"operation":{"mnemonic":"JUMP"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":125,"operation":{"mnemonic":"JUMPDEST"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":126,"operation":{"mnemonic":"DUP2"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":127,"operation":{"mnemonic":"SLOAD"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":128,"operation":{"arguments":["0x87"],"mnemonic":"PUSH1"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":130,"operation":{"mnemonic":"SWAP2"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":131,"operation":{"mnemonic":"SWAP1"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":132,"operation":{"arguments":["0x35"],"mnemonic":"PUSH1"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":134,"operation":{"mnemonic":"JUMP"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":135,"operation":{"mnemonic":"JUMPDEST"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":136,"operation":{"mnemonic":"SWAP1"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":137,"operation":{"mnemonic":"SSTORE"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":138,"operation":{"mnemonic":"JUMP"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":139,"operation":{"mnemonic":"JUMPDEST"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":140,"operation":{"arguments":["0x94"],"mnemonic":"PUSH1"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":142,"operation":{"arguments":["0x01"],"mnemonic":"PUSH1"}},{"context":{"code":{"range":{"length":94,"offset":26},"source":{"id":0}}},"offset":144,"operation":{"mnemonic":"PUSH0"}},{"context":{"code":{"range":{"length":1,"offset":53},"source":{"id":0}}},"offset":145,"operation":{"arguments":["0x6e"],"mnemonic":"PUSH1"}},{"context":{"code":{"range":{"length":1,"offset":53},"source":{"id":0}}},"offset":147,"operation":{"mnemonic":"JUMP"}},{"context":{"code":{"range":{"length":1,"offset":53},"source":{"id":0}}},"offset":148,"operation":{"mnemonic":"JUMPDEST"}},{"context":{"code":{"range":{"length":1,"offset":53},"source":{"id":0}}},"offset":149,"operation":{"mnemonic":"JUMP"}}]}

Didn't wanna pretty print so as not to take up too much vertical comment space :)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah sweet, so it does emit ethdebug! Perhaps not the full granularity but yeah, something. :D

@@ -1 +1 @@
--experimental --ethdebug --via-ir --pretty-json --json-indent 4
--experimental --ethdebug-compilation --ethdebug-program --ethdebug-program-runtime --via-ir --pretty-json --json-indent 4
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i get that this is to make things uniform but from a UX point of view.. ugh.. :) what do you think about --ethdebug-full or something? i guess that's the target use-case?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Current decision is to not have a top level (full) ethdebug output - it's all experimental, and we'll see down the road if it makes sense.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah i also don't want to waste your time with this. just seemed a bit annoying to have to provide four cli flags for the full ethdebug output is all

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In practice, most everyone (and especially tooling) rely on standard JSON, where this granular approach makes significantly more sense - the purpose of this PR is to introduce equivalence between the two, which I do agree is quite inconvenient given that we're mapping each stdjson option into a flag :)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right. Well let's see. Maybe we can stomach the small divergence of introducing an additional flag in the cli that groups together standard json settings. If things become too annoying down the line. :)

}
}

BOOST_AUTO_TEST_CASE(cli_ethdebug_ethdebug_output)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ethdebug_cli_flag_combinations?

anyways for the whole thing i think such a structure would be nice:

BOOST_AUTO_TEST_SUITE(CommandLineInterfaceTest)
BOOST_AUTO_TEST_SUITE(Ethdebug)

BOOST_AUTO_TEST_CASE(rejects_optimize_with_program) { ... }
BOOST_AUTO_TEST_CASE(rejects_program_without_via_ir) { ... }
BOOST_AUTO_TEST_CASE(accepts_resources_standalone) { ... }
BOOST_AUTO_TEST_CASE(accepts_compilation_standalone) { ... }

BOOST_AUTO_TEST_SUITE_END()
BOOST_AUTO_TEST_SUITE_END()

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All of the test cases start with cli_* (except for standard json onses), so cli_ethdebug_ethdebug_output is more consistent.

As far as adding a separate suite for ethdebug and separating each combination into a separate test case - I do agree that would be significantly better in almost every way, and failures would also be significantly easier to find, instead of trying to figure out which combination in a vector of combinations caused a failure.

I don't necessarily think it's worth a refactor at the moment, since the long term plan is to migrate all cmdLineTests to isoltest, which would then make all of these boost tests redundant (i.e. we would move all of them to the new isoltest as well).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, that makes sense. I don't think I'd mind an enclosing Ethdebug suite around the ethdebug tests in the current boost test setup and it's like a 2 lines of code. But also completely optional and a bit cosmetic. Restructuring the tests is a lot more work. Seems like there is no free lunch after all :)

@nikola-matic
Copy link
Copy Markdown
Contributor Author

Nice cleanup! Mainly two things though:

* the `requiresFullCompilation` / skip `link()` thing seems orthogonal and should probably be extracted into its own pr unless i'm missing the point here

* global ethdebug output assumes analysis succeeded. this should be tested and guarded in the implementation

the rest is nits and a design question about the standard json interface (the * selection for global settings seems a bit awkward to me) as well as if we want to add a --ethdebug-full or just --ethdebug output as short-hand for the common case.

All of the requirements are in the issue, including the full/partial compilation requirements as well as the * handling (read through the comments as well). That was Kamil's spec, so I didn't want to deviate.

@nikola-matic nikola-matic force-pushed the make-ethdebug-consistent-with-existing-conventions branch from 1986be6 to 74f8ebf Compare April 16, 2026 12:05
Copy link
Copy Markdown
Member

@clonker clonker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's good code-wise now, the remaining points are more general design questions and/or refactorings and improvements that can be done as follow-ups. I think it'd be good to do the templated areArtifactsRequested + static constexpr arrays when possible but I'll leave that up to you.

Other than that I think it would be good to have test coverage for stopAfter: parsing with ethdebug.compilation and/or ethdebug.resources requested.

Leaves the final point: changelog? :)

Comment thread libsolidity/interface/StandardCompiler.cpp Outdated
@nikola-matic nikola-matic force-pushed the make-ethdebug-consistent-with-existing-conventions branch 2 times, most recently from db47406 to b006193 Compare April 17, 2026 17:20
Copy link
Copy Markdown
Member

@clonker clonker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me now!

@nikola-matic nikola-matic force-pushed the make-ethdebug-consistent-with-existing-conventions branch from b006193 to 32a5a84 Compare April 17, 2026 17:34
@nikola-matic nikola-matic merged commit 9be6619 into develop Apr 17, 2026
83 checks passed
@nikola-matic nikola-matic deleted the make-ethdebug-consistent-with-existing-conventions branch April 17, 2026 19:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make ethdebug consistent with our existing conventions for outputs/settings

3 participants