Skip to content

Fix ast-id-only IR debug info output - #16734

Open
LeviofCrypto wants to merge 1 commit into
argotorg:developfrom
LeviofCrypto:fix-ast-id-only-ir-debug-info
Open

Fix ast-id-only IR debug info output#16734
LeviofCrypto wants to merge 1 commit into
argotorg:developfrom
LeviofCrypto:fix-ast-id-only-ir-debug-info

Conversation

@LeviofCrypto

Copy link
Copy Markdown

Description

settings.debug.debugInfo: ["ast-id"] still emitted @src comments in generated IR because the shared source-location helper treated any non-empty debug info selection as a location request. Skip location comment creation unless the location component is selected, while preserving @ast-id output.

Adds a Standard JSON cmdline test for the ast-id-only IR/EVM assembly output case.

Checklist

AI Disclosure

  • No AI tools were used
  • AI tools were used (details below)

OpenAI Codex assisted with reviewing the change.

@matheusaaguiar

Copy link
Copy Markdown
Contributor

Hi @LeviofCrypto , thanks for the PR.
It looks good, but there is still one spot missing.
AsmPrinter::formatDebugData, calls formatSourceLocation and still leaks @src under ast-id only.
So a Yul input with @src given to the compiler will not remove those under ast-id only setting.

/// @use-src 0:"input.sol"
object "C_6_deployed" {
    code {
        /// @src 0:60:101  "contract C {..."
        mstore(64, 128)
    }
}

$ solc --strict-assembly --debug-info ast-id input.yul

pragma solidity >=0.0;

contract C {
function f() public {}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

f() is optimized out in the irOptimized output. It would be interesting to have a more complex function that can show in the optmized output.

solAssert(_location.sourceName, "");
_context.markSourceUsed(*_location.sourceName);

if (!_context.debugInfoSelection().location)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This PR also needs a Changelog entry.

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.

2 participants