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

API change: replace loop and forObject callbacks with cfRoot/cfBlock #51

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

alexjordan
Copy link
Collaborator

This proposed change avoids trying to map the underlying to instrumentation signals into higher-level constructs.

It addresses the following issues with the previous API in #48:

  • Duplicate callbacks issued to loop and forObject API
  • Lack of filtering of conditional events in loop callback
  • cfBlockEnter/Exit are now anchored to their parent root via parentIID

@alexjordan alexjordan mentioned this pull request Sep 30, 2019
Extended forinof test uses the callbacks with a custom for-of iterator.
@Haiyang-Sun
Copy link
Owner

@alexjordan So here the test code reveals that when there is a customized iterator defined, the last expression might not be the target object for the for in/of event (but the last expression executed in the iterator function).

How about we track the last expression value executed at each stack frame? In this case, we should still be able to know the object being iterated without complex logic checks?

@alexjordan
Copy link
Collaborator Author

@Haiyang-Sun in general it would be nice to have an easy way (ideally directly exposed by Graal.js instrumentation) to communicate which object is being iterated over. But in the end it depends on the analysis. When an object has a custom iterator and an analysis is interested in data flow, the object is not enough, the analysis has to analyze the iterator code as well.

@Haiyang-Sun
Copy link
Owner

Haiyang-Sun commented Oct 2, 2019

@alexjordan I agree. I like the example analysis here to illustrate how it can be used for such a complex use case. I just propose to add a simple and easy-to-understand analysis example to show the for in/of event with the object. I will add a PR later to this branch.

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.

None yet

3 participants