Skip to content

Restore old behavior in FrameInstance#getCallNode() - #90

Merged
chrisseaton merged 4 commits into
oracle:masterfrom
chumer:restore_old_stack_trace_behavior
Mar 3, 2016
Merged

Restore old behavior in FrameInstance#getCallNode()#90
chrisseaton merged 4 commits into
oracle:masterfrom
chumer:restore_old_stack_trace_behavior

Conversation

@chumer

@chumer chumer commented Mar 3, 2016

Copy link
Copy Markdown
Member

@chrisseaton please verify that the behavior is now fixed in ruby.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does "next" mean in this context? It means "caller"? Or the node representing the "call site" that invoked the method with this frame instance?

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.

"The call node that invoked a target to get outside this frame" maybe?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better?

    /**
     * @return the node that represents the caller of the next upper {@link CallTarget target} in
     *         the stack trace. Null in case there is no upper target or if the target was not
     *         invoked using a {@link TruffleRuntime#createDirectCallNode(CallTarget) direct} or
     *         {@link TruffleRuntime#createDirectCallNode(CallTarget) indirect} call node.
     *
     * @since 0.8 or earlier
     **/
    Node getCallNode();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not think I understand "outside". Maybe we can at least get some ASCII art visualization of the call stack to help with a common understanding how the frame iteration takes place.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR is there to fix the problems for the languages quickly. But I will prepare nice ASCII art for in the next PR :-).

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.

Do we call it a call stack of frames or a stack of targets ?
It is a FrameInstanceVisitor so I would think frame.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eregon Please comment here: #91

@woess

woess commented Mar 3, 2016

Copy link
Copy Markdown
Member

The fix works for me.

@chumer
chumer force-pushed the restore_old_stack_trace_behavior branch from 783d047 to 43c60b8 Compare March 3, 2016 16:52
@chrisseaton

Copy link
Copy Markdown
Contributor

Works for me. Merging when Travis completes.

@woess woess added the accept label Mar 3, 2016
chrisseaton added a commit that referenced this pull request Mar 3, 2016
Restore old behavior in FrameInstance#getCallNode()
@chrisseaton
chrisseaton merged commit 79fd67a into oracle:master Mar 3, 2016
* @return a node representing the callsite of the next new target on the stack. Null in case
* there is no upper target or if the target was not invoked using a
* {@link TruffleRuntime#createDirectCallNode(CallTarget) direct} or
* {@link TruffleRuntime#createDirectCallNode(CallTarget) indirect} call node.

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.

Should be {@link TruffleRuntime#createIndirectCallNode()}

dougxc pushed a commit that referenced this pull request Jun 21, 2016
…truffle:instrumentation_source_listeners to master

* commit '5c2f517858620e8fdb7ae7682c47666280292063':
  Fix comment about WeakHashMap.
  Fix failing safe replace test.
  Make the wrapper insertion more atomic; cannot check parent node immeditaly after replace. might be racy.
  Run five times for one second instead of one time for five seconds in the multithreaded test.
  Fix a node is safely replaceable if the child was not found anymore in the parent.
  Fix iterator needs to capture the array.
  Add explicit event classes for source events for better evolvability.
  Fix unused import.
  Only use atomic accesses to instrumenterMap.
  Only mark bindings as disposed when they are invalidated.
  Implement iterator for AbstractAsyncList and use Collection as an interface for it to make it easier to reuse in the future.
  Fix typos in javadoc.
  Fix typo in AbstractAsyncList
  Fix unnecessary eager initialization of the debugger.
  Implement source and source section load event listeners; Further refinements to thread-safety.
  Cleanup event binding.
  Fix checkstyle.
  Make instrumentation thread-safe.
  Add multithreading test for instrumentation.
jerboaa pushed a commit to jerboaa/graal that referenced this pull request Jun 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants