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

add ExprUsesArgs analysis #293

Closed
wants to merge 6 commits into from
Closed

Conversation

yihozhang
Copy link
Collaborator

This analysis returns a set of variables used by an expression. Users of this analysis need to explicitly place demands on the expression they want to run this analysis on.

@yihozhang yihozhang requested a review from rtjoa January 27, 2024 02:34
@oflatt
Copy link
Member

oflatt commented Jan 27, 2024

Why does this PR require using a set? Not sure what it is used for yet.
Could you do a relation (relation ExprUsesVar (Expr Expr))?

@yihozhang
Copy link
Collaborator Author

There are several reasons for this:

  • I need to collect a list FV of all free variables of an expression, and add FV to the input context of let-inlining. Collecting all values of a column in a container is less straightforward than just having a set as output.
  • There was a typo in the original analysis. The merge function for ExprUsesArgs should be set-intersect, otherwise a constant may use variables if it is merged with x-x, etc. relation cannot do this.

@oflatt
Copy link
Member

oflatt commented Jan 29, 2024

So this is more like a ExprAlwaysUsesArgs analysis? As in, it uses these arguments in all equivalent versions?

@oflatt
Copy link
Member

oflatt commented Jan 31, 2024

I think we decided to close this, since we are changing it to extract a particular term that uses a particular set of arguments.

@oflatt oflatt closed this Jan 31, 2024
@yihozhang yihozhang deleted the yihozhang-tu-args-used-analysis branch May 1, 2024 22:26
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