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

More checks of argument aliasing #965

Merged
merged 3 commits into from
Mar 31, 2021

Conversation

jad-hamza
Copy link
Contributor

For issue #947

@samarion I tried your suggestion of using Try(getTargets(e)).toOption.getOrElse(exprOps.variablesOf(arg).map(v => Target(v, None, Path.empty)))) to identify aliases. Did you then want to check whether a target of one argument is a prefix of a target of another argument?

For issue #947, this doesn't seem to work because the targets of c1 are Some(Target(c1, None, <empty>)) and the targets of c2 are Some(Target(c1, None, <empty>)). Should getTargets on c2 also return c1?

We might have the same issue with getTargets returns None, because variablesOf will not capture the transitive dependencies.

@samarion
Copy link
Member

I tried your suggestion of using Try(getTargets(e)).toOption.getOrElse(exprOps.variablesOf(arg).map(v => Target(v, None, Path.empty)))) to identify aliases.

You might want to filter out variables that don't have a mutable type in the orElse case.

Did you then want to check whether a target of one argument is a prefix of a target of another argument?

Yes, that sounds right.

For issue #947, this doesn't seem to work because the targets of c1 are Some(Target(c1, None, )) and the targets of c2 are Some(Target(c1, None, )).

You probably need to apply rewritings with exprOps.replaceFromSymbols(env.rewritings, arg) before calling getTargets and variablesOf. That should hopefully fix the issue of transitive dependencies. If it doesn't, then we probably have a more general problem with aliasing in let-bindings.

@jad-hamza
Copy link
Contributor Author

Thanks for the quick review! The rewritings make this example work correctly (make it fail as it should).

@jad-hamza
Copy link
Contributor Author

I disabled the aliasing check for accessors, otherwise it was making simple updates such as x = x + 1 for a var x in a trait fail.

@jad-hamza jad-hamza merged commit 5afe125 into epfl-lara:master Mar 31, 2021
@jad-hamza jad-hamza deleted the argument-aliasing branch March 31, 2021 12:31
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