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

Adjust to upstream edges change #296

Merged
merged 1 commit into from
Nov 12, 2024
Merged

Adjust to upstream edges change #296

merged 1 commit into from
Nov 12, 2024

Conversation

Keno
Copy link
Collaborator

@Keno Keno commented Nov 5, 2024

No description provided.

@Keno Keno requested a review from aviatesk November 5, 2024 00:53
@Keno
Copy link
Collaborator Author

Keno commented Nov 5, 2024

cc @vtjnash

Comment on lines +80 to +83
function CC.add_edges_impl(edges::Vector{Any}, info::FRuleCallInfo)
CC.add_edges!(edges, info.info)
CC.add_edges!(edges, info.frule_call.info)
end
Copy link
Member

Choose a reason for hiding this comment

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

I don’t think this implementation allows the method table backedge equivalent to CC.add_mt_backedge!(sv, frule_mt, frule_atype) to be created, but I also think this should be sufficient for the purpose of invalidating when a new frule is added—which was the reason for adding add_mt_backedge!. In the current implementation, a backedge is added to the fallback frule(...), which I think should trigger equivalent invalidation.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Right, the point was to trigger on additions of new signatures that would change the lookup result. My understanding is that the callinfo encodes exactly the lookup we want, so this should do what we need.

@Keno Keno merged commit 06cb7dd into main Nov 12, 2024
4 of 9 checks passed
@oscardssmith oscardssmith deleted the kf/edgesadjust branch November 12, 2024 23:04
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