Skip to content

Turn non-analysis expRelation into queryProvider - #2076

Open
michael-schwarz wants to merge 3 commits into
masterfrom
exprelation_simplify
Open

Turn non-analysis expRelation into queryProvider#2076
michael-schwarz wants to merge 3 commits into
masterfrom
exprelation_simplify

Conversation

@michael-schwarz

Copy link
Copy Markdown
Member

It has been an analysis for historical reasons, but carrying around a () everywhere to just answer some queries from time-to-time seems excessive.
This introduces a new interface for things that can sometimes answer queries but are not analyses. This is currently expRelation only,

Copilot AI review requested due to automatic review settings July 17, 2026 06:17
@michael-schwarz michael-schwarz added the cleanup Refactoring, clean-up label Jul 17, 2026

Copilot AI left a comment

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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot AI review requested due to automatic review settings July 17, 2026 06:21

Copilot AI left a comment

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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@sim642
sim642 self-requested a review July 17, 2026 11:58
@sim642

sim642 commented Jul 20, 2026

Copy link
Copy Markdown
Member

It has been an analysis for historical reasons, but carrying around a () everywhere to just answer some queries from time-to-time seems excessive.

Are there actual cases where it's excessive performance-wise?

This introduces a new interface for things that can sometimes answer queries but are not analyses. This is currently expRelation only,

I'm not sure if a new abstraction interface is that useful if there's just a single instance of it. We have many things based on UnitAnalysis (e.g. access, race, assert, mhp, etc.) that also unnecessarily require dragging a unit around but all of them also do a few small things that require a bit more generality.
If the unit is just annoying in the outputs, then maybe we can introduce more generally something like should_print is for accesses.

Or maybe this could be more tightly coupled into base's evalbinop_mustbeequal, which includes a bit of symbolic rewriting as well. It would make all of these accessible via EvalInt anyway. The only difference would be if we wanted to use exprelation, but not base, which is a quite unlikely combination, especially because exprelation mainly exists for partitioned arrays in base.

@michael-schwarz

Copy link
Copy Markdown
Member Author

Or maybe this could be more tightly coupled into base's evalbinop_mustbeequal, which includes a bit of symbolic rewriting as well.

I may try to see what this looks like once my AI tokens reset on Aug 01.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cleanup Refactoring, clean-up

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants