Skip to content

Conversation

@aarlt
Copy link
Collaborator

@aarlt aarlt commented Jul 22, 2024

  • if debug-info was set to ethdebug
    • it will only work if also ir, irOptimized and/or ethdebug was selected as output.
      standard_debug_info_in_yul_ethdebug_output_ir_optimized, standard_debug_info_in_yul_ethdebug_output_no_ir
    • it will always work with strict-assembly e.g. solc --strict-assembly <yul> --debug-info ethdebug
    • debug-info ethdebug is excluded from the help on cli
    • debug-info ethdebug is excluded from all on cli and wildcard selection * in standard-json
  • if ethdebug was selected as output
    • if no debug-info was selected, it implicitly set debug-info to ethdebug. solc <contract> --ethdebug
    • if via-ir was not specified, it will error with a message stating that ethdebug can only be selected as output, if via-ir was defined. solc <contract> --ethdebug only works with --via-ir
    • if debug-info was selected and did not contain ethdebug, an error will be generated stating that ethdebug need to be set in debug-info solc <contract> --ethdebug --debug-info location
    • strict-assembly will always work e.g. solc --strict-assembly <yul> --ethdebug
    • output selection ethdebug is not shown in cli help
    • ethdebug output selection is excluded from wildcard selection * in standard-json

UPDATE
After some discussion with @gnidan and @ekpyron it turned out that we need something slightly different:

  • we need to be able to distinguish ethdebug debug data from deploy-time and run-time.
  • standard-json:  ethdebug output will now be enabled with evm.bytecode.ethdebug(deploytime part) and evm.deployedBytecode.ethdebug (runtime part)
    • special case: output selection of evm.bytecodeand evm.deployedBytecodebehave like a wildcard, so the ethdebug stuff is excluded here.
  • cli: like binary selection --bin and --bin-runtime ethdebug selection will now work similar with --ethdebug and --ethdebug-runtime

UPDATE 01/2025

  • enabling optimization with ethdebug will now create an error - it is not supported yet

@aarlt aarlt force-pushed the enable_debug_info_ethdebug branch 3 times, most recently from ccbe426 to 71bf655 Compare July 23, 2024 14:08
@argotorg argotorg deleted a comment from stackenbotten Jul 23, 2024
@aarlt aarlt self-assigned this Jul 23, 2024
@cameel cameel changed the title [ethdebug] Enable ethdebug debug info selection. Enable ethdebug debug info selection Jul 24, 2024
@aarlt aarlt marked this pull request as draft July 24, 2024 11:14
@aarlt aarlt force-pushed the enable_debug_info_ethdebug branch 5 times, most recently from ec729b3 to ee2bc12 Compare July 24, 2024 23:05
@aarlt aarlt force-pushed the enable_debug_info_ethdebug branch from ee2bc12 to 9772d20 Compare August 1, 2024 18:51
@aarlt aarlt changed the title Enable ethdebug debug info selection Enable ethdebug debug info and output selection. Aug 1, 2024
@aarlt aarlt force-pushed the enable_debug_info_ethdebug branch 2 times, most recently from 09fd459 to 53e12ef Compare August 5, 2024 11:12
@argotorg argotorg deleted a comment from stackenbotten Aug 6, 2024
@aarlt aarlt force-pushed the enable_debug_info_ethdebug branch 4 times, most recently from 4b5c548 to a262eef Compare August 7, 2024 16:00
@aarlt aarlt marked this pull request as ready for review August 7, 2024 16:00
@aarlt aarlt force-pushed the enable_debug_info_ethdebug branch from a262eef to c4cb445 Compare August 12, 2024 10:30
@nikola-matic nikola-matic self-requested a review August 12, 2024 13:30
@aarlt aarlt reopened this Dec 2, 2024
@aarlt aarlt force-pushed the enable_debug_info_ethdebug branch from b8851f6 to 1edf1f4 Compare December 4, 2024 10:37
@github-actions github-actions bot added the stale The issue/PR was marked as stale because it has been open for too long. label Dec 18, 2024
@aarlt aarlt force-pushed the enable_debug_info_ethdebug branch from 1edf1f4 to bf1527c Compare December 18, 2024 13:28
@github-actions github-actions bot removed the stale The issue/PR was marked as stale because it has been open for too long. label Dec 19, 2024
@github-actions github-actions bot added the stale The issue/PR was marked as stale because it has been open for too long. label Jan 3, 2025
@ekpyron ekpyron added 🟡 PR review label and removed 🟡 PR review label stale The issue/PR was marked as stale because it has been open for too long. labels Jan 6, 2025
@ekpyron ekpyron requested a review from clonker January 6, 2025 14:42
@argotorg argotorg deleted a comment from github-actions bot Jan 7, 2025
@argotorg argotorg deleted a comment from github-actions bot Jan 7, 2025
Copy link
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.

Nothing deal-breaking I think. A couple of suggestions and questions. I would be wary when/how much debug data is collected, in particular that we don't collect it by default and just don't output it in the end - that could pose a big performance drain.

static DebugInfoSelection const Only(bool DebugInfoSelection::* _member) noexcept;
static DebugInfoSelection const Default() noexcept { return All(); }
static DebugInfoSelection const Default() noexcept { return ExceptExperimental(); }
static DebugInfoSelection const Except(std::vector<bool DebugInfoSelection::*> const& _members) noexcept;
Copy link
Member

Choose a reason for hiding this comment

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

I think it is fine but I was a bit confused about the naming Except and ExceptExperimental. I would have imagined to call something like All{}.exceptExperimental(). Or, to make it more clear for ... people like me, you could name it AllExceptExperimental. Or something. Minor point in any case, feel free to ignore

[](const Json& result)
{
return result["contracts"]["fileA"]["contractA"]["evm"]["deployedBytecode"].contains("ethdebug") &&
result["contracts"]["fileB"]["contractB"]["evm"]["bytecode"].contains("ethdebug") ;
Copy link
Member

Choose a reason for hiding this comment

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

ping

@aarlt aarlt force-pushed the enable_debug_info_ethdebug branch from bf1527c to df4a941 Compare January 8, 2025 15:06
@aarlt aarlt requested a review from clonker January 8, 2025 15:18
Copy link
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.

just a few whitespaces :D
lgtm otherwise

@aarlt aarlt force-pushed the enable_debug_info_ethdebug branch from e4ab7ac to 7e14812 Compare January 9, 2025 13:56
clonker
clonker previously approved these changes Jan 9, 2025
@aarlt aarlt force-pushed the enable_debug_info_ethdebug branch 3 times, most recently from 82df2ba to 0b702ac Compare January 17, 2025 17:14
@aarlt aarlt force-pushed the enable_debug_info_ethdebug branch from 0b702ac to 23d9607 Compare January 27, 2025 14:47
@aarlt aarlt force-pushed the enable_debug_info_ethdebug branch from 23d9607 to 7ea985d Compare January 27, 2025 15:35
@aarlt aarlt merged commit e1e33b2 into develop Jan 27, 2025
75 checks passed
@aarlt aarlt deleted the enable_debug_info_ethdebug branch January 27, 2025 17:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ethdebug 🟡 PR review label

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants