Skip to content

Reverse order in which we visit derived classes.#2264

Draft
bbannier wants to merge 3 commits intomainfrom
topic/bbannier/node-visition-order
Draft

Reverse order in which we visit derived classes.#2264
bbannier wants to merge 3 commits intomainfrom
topic/bbannier/node-visition-order

Conversation

@bbannier
Copy link
Member

No description provided.

When invoking Node visitors we previously would visit from base class
to most derived. For visitors accumulating information this was
fine since for them order didn't matter. Matchers which should be
invoked exactly once on the other hand were hard to implement since we
would always invoke the most general matcher without even knowing
whether a more specific one would also be invoked.

This patch reverses the order we invoke handlers, now from most derived
to base one. With additional state tracking this allows implementing
skipping of more gerneral default handlers.
This code was already valid before, but coverity discovered different
usage before and inferred that the dereferenced pointer could be null
here as well. Add an `assert` to make clear that we expect it to be set.
@bbannier bbannier self-assigned this Feb 18, 2026
@codspeed-hq
Copy link

codspeed-hq bot commented Feb 18, 2026

Merging this PR will not alter performance

✅ 28 untouched benchmarks


Comparing topic/bbannier/node-visition-order (5e4ad24) with main (3142d19)

Open in CodSpeed

@bbannier bbannier marked this pull request as ready for review February 18, 2026 19:30
@bbannier bbannier requested a review from rsmmr February 18, 2026 19:30
Copy link
Member

@rsmmr rsmmr left a comment

Choose a reason for hiding this comment

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

Have you looked at the baseline changes more closely? In particular these stand out to me:

- [debug/resolver] -> [D21] declaration::Type Bar::Bar1 | public type Bar1 = string; (bar.hlt:6:1-6:26)
+ [debug/resolver] -> [D21] declaration::Type <no-canon-id> | public type Bar1 = string; (bar.hlt:6:1-6:26)

@bbannier bbannier marked this pull request as draft February 20, 2026 13:25
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.

2 participants