Skip to content

Conversation

DaniPopes
Copy link
Member

@DaniPopes DaniPopes commented Sep 6, 2025

Closes #10964.

By using Solar to analyze contracts to find items we can avoid requesting AST output from solc, saving a considerable amount of serde time.

This also fixes a few bugs, while trying to remain 100% compatible with the previous solc AST implementation. See individual commits and their descriptions for more details.


Default project:
image

ithacaxyz/account:
image

@DaniPopes DaniPopes self-assigned this Sep 8, 2025
@DaniPopes DaniPopes moved this to In Progress in Foundry Sep 8, 2025
Comment on lines +221 to +222
},
stmt.span,
Copy link
Member Author

@DaniPopes DaniPopes Sep 12, 2025

Choose a reason for hiding this comment

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

this is not entirely equivalent, but the previous logic resulted in weird spans for else-if chains (left is before, right is after):
image

this was added in #3094

@DaniPopes DaniPopes marked this pull request as ready for review September 12, 2025 19:20
Copy link
Collaborator

@grandizzy grandizzy left a comment

Choose a reason for hiding this comment

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

lgtm!

@DaniPopes DaniPopes moved this from In Progress to Ready For Review in Foundry Sep 15, 2025
@onbjerg onbjerg merged commit 632f36b into master Sep 15, 2025
25 checks passed
@onbjerg onbjerg deleted the dani/solar-coverage-1 branch September 15, 2025 12:56
@github-project-automation github-project-automation bot moved this from Ready For Review to Done in Foundry Sep 15, 2025
@grandizzy grandizzy moved this from Done to Completed in Foundry Sep 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Completed
Development

Successfully merging this pull request may close these issues.

replace Solc AST usage in forge coverage
3 participants