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

AntiAliasing: restore var binding in mapApplication #1509

Merged
merged 1 commit into from
Apr 23, 2024

Conversation

mario-bucev
Copy link
Collaborator

PR #1507 made some changes to mapApplication, and one I made by negligence (in the diff line) turned out to unveil an issue in the Let case handling of AntiAliasing (line in question).

Here, we compute the targets of the transformed binding. If this binding happens to contain a function call, the returned targets may be invalid because getTargets works on functions prior to the AntiAliasing function purification, not after (as it's done here).

The reason this issue did not arise before is the var introduced by mapApplication prevented target computation, which ultimately resulted in rejection.

This PR simply reverts the changed lines but the underlying issue is still here (though it cannot be triggered). To properly fix this, we would need to distinguish pre/post transformation getTargets computation, which would require significant changes to EffectsAnalyzer.

@vkuncak
Copy link
Collaborator

vkuncak commented Apr 22, 2024

Could you move the rest of the PR comment to the /* ... */ in the function you comment, including the remark about needing to rewrite?

@mario-bucev
Copy link
Collaborator Author

Good idea! I've added a comment about these issues as requested

@mario-bucev mario-bucev merged commit 54399d0 into epfl-lara:main Apr 23, 2024
2 checks passed
@mario-bucev mario-bucev deleted the fix-mapapp branch April 23, 2024 08:35
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