Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ethdebug: correct handling of abstract contracts. #15920

Merged
merged 1 commit into from
Mar 11, 2025

Conversation

aarlt
Copy link
Member

@aarlt aarlt commented Mar 11, 2025

No description provided.

if (_linkerObject)
{
Copy link
Member

Choose a reason for hiding this comment

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

You should do this to get saner diff :)

Suggested change
if (_linkerObject)
{
if (!_linkerObject)
Json::array();

Copy link
Member

Choose a reason for hiding this comment

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

But also why is it even called when the object is null? Wouldn't make more sense to just skip it?

@aarlt aarlt force-pushed the ethdebug_instructions_and_sources_fix branch from 8e6729d to 3813bce Compare March 11, 2025 01:09
@aarlt aarlt force-pushed the ethdebug_instructions_and_sources_fix branch from 3813bce to 2abfe0b Compare March 11, 2025 01:12
@aarlt aarlt changed the title ethdebug: correct handling of interfaces. ethdebug: correct handling of abstract contracts. Mar 11, 2025
@ekpyron ekpyron enabled auto-merge March 11, 2025 01:18
@ekpyron
Copy link
Member

ekpyron commented Mar 11, 2025

For the record: I'm approving this PR and setting it to merge, even though this is a mild violation of the ethdebug spec - we shouldn't produce "program" ethdebug output for anything that doesn't produce any actual bytecode - so ideally we should already filter out abstract contracts and interfaces earlier than here - which would likely mean both in CommandLineInterface and in StandardCompiler (in the latter case we should make it an error, if ethdebug program output is specifically requested for an interface/abstract contract, while just skipping them for a wildcard request).

But the PR fixes the actual usage limitation here (which would have been a bit unfortunate to have kept in for the release), and there's no harm in producing this output, so we can keep it as is and adjust this properly after the release.

@ekpyron ekpyron merged commit 07b202c into develop Mar 11, 2025
74 checks passed
@ekpyron ekpyron deleted the ethdebug_instructions_and_sources_fix branch March 11, 2025 01:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants