Skip to content

refactor[venom]: add dom walk property#4546

Merged
charles-cooper merged 4 commits intovyperlang:masterfrom
harkal:fix/venom/dom_walk
Mar 30, 2025
Merged

refactor[venom]: add dom walk property#4546
charles-cooper merged 4 commits intovyperlang:masterfrom
harkal:fix/venom/dom_walk

Conversation

@harkal
Copy link
Copy Markdown
Collaborator

@harkal harkal commented Mar 28, 2025 •
edited by charles-cooper
Loading

What I did

Refactored DominatorTreeAnalysis to be more clear. Added a dominator post-order traversal property.

How I did it

How to verify it

Commit message

Refactor `DominatorTreeAnalysis` to be more clear regarding cfg walk
usage. Add a dominator post-order traversal property.

The `MakeSSA` pass is refactored to use dom tree walk instead of CFG
post walk. Traversing based on the dominator tree is not technically
mandatory but it aligns with the algorithm's reliance on dominance and
simplifies the renaming phase.

Description for the changelog

Cute Animal Picture

image-14

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 28, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 96.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 92.37%. Comparing base (c645972) to head (6c2b89d).
Report is 96 commits behind head on master.

Files with missing lines Patch % Lines
vyper/venom/analysis/dominators.py 95.45% 1 Missing ⚠️
Additional details and impacted files 
@@           Coverage Diff           @@
##           master    #4546   +/-   ##
=======================================
  Coverage   92.36%   92.37%           
=======================================
  Files         123      123           
  Lines       17531    17538    +7     
  Branches     2959     2961    +2     
=======================================
+ Hits        16193    16200    +7     
  Misses        934      934           
  Partials      404      404

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@harkal harkal marked this pull request as ready for review March 28, 2025 11:25
@harkal harkal requested a review from charles-cooper March 28, 2025 11:25
charles-cooper
charles-cooper reviewed Mar 28, 2025
View reviewed changes
@charles-cooper charles-cooper changed the title fix[venom]: dom walk refactor[venom]: add dom walk property Mar 28, 2025
charles-cooper
charles-cooper approved these changes Mar 28, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@charles-cooper charles-cooper left a comment

Choose a reason for hiding this comment

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

lgtm, please address https://github.com/vyperlang/vyper/pull/4546/files#r2018575899

@charles-cooper charles-cooper merged commit 206a119 into vyperlang:master Mar 30, 2025
162 checks passed
@harkal harkal deleted the fix/venom/dom_walk branch March 30, 2025 12:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@charles-cooper charles-cooper charles-cooper approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@harkal @charles-cooper