Bug 40912 - [java debug] Debugger should show value of "last returned method"
Summary: [java debug] Debugger should show value of "last returned method"
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Debug (show other bugs)
Version: 2.1.1   Edit
Hardware: All All
: P3 enhancement with 100 votes (vote)
Target Milestone: 4.7 M2   Edit
Assignee: Till Brychcy CLA
QA Contact:
URL:
Whiteboard:
Keywords: helpwanted
: 75519 210662 389127 (view as bug list)
Depends on:
Blocks: 499500 499655
  Show dependency tree
 
Reported: 2003-07-29 15:23 EDT by David Corbin CLA
Modified: 2018-07-06 07:36 EDT (History)
48 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description David Corbin CLA 2003-07-29 15:23:03 EDT
When I step out of a method, I should be able to see in *some* view, the value
that was returned by that method. Or, if I step over a method, it should show it.

In Microsoft Visual C++, this is usually shown in the "variable's view", though
it is not really a variable.
Comment 1 Troy Folger CLA 2003-07-30 13:36:48 EDT
While the eclipse "Watch Expression" provides something like this 
behavior, it isn't automatic.

This is how I initially requested this feature (the ability to
see method and expression evaluations while debugging):

In C++ debuggers I have used in the past (I believe
Borland C++ for OS/2 or maybe Visual Age or VC++), this feature was
a little more 'automatic'.  You didn't have to select anything, you
could just see intermediate results in the same view as the stack.
If you stepped over a line with multiple evaluations, you saw the
results of the last evaluation; if you stepped into that line, you
saw the results of each evaluation as you 'step returned' or similar.
(I think the UI was similar to Eclipse's Variables view with details
at the bottom.)

If this detail is insufficient, please append and I (we) will try again.
Comment 2 Markus Keller CLA 2003-08-05 09:01:03 EDT
And if a method call completes abruptly, the thrown exception should be shown
instead of a return value.
Comment 3 David Corbin CLA 2003-08-05 09:09:56 EDT
But it has to be identified as a *thrown* exception, and not a returned object.
Comment 4 Darin Wright CLA 2003-08-07 14:40:01 EDT
Looking at the JDI spec/support, I do not believe that the Java debugger can do 
this currently. There is no way to retrieve the result of the last execution 
(i.e. no access to an execution stack, or equivalent). As well, the step 
requests and events do not reveal this information.

Adding Luc as CC to see if we agree.
Comment 5 David Corbin CLA 2003-08-07 14:56:52 EDT
If this is the case, can the eclipse team submit a feature request to Sun?  I
expect to carry more weight if you guys do it...
Comment 6 Luc Bourlier CLA 2003-08-07 16:30:12 EDT
I confirm, there is nothing in the debug protocol to get the result of the last
operation.
Comment 8 Darin Wright CLA 2004-09-21 21:06:40 EDT
The bug reports show that the feature is in JVMTI, but the accessor has not 
yet been added to the MethodExitEvent in JDI. The comment shows it was de-
committed from Tiger due to time constraints. Marking as Later since we may be 
able to solve this one in the future.
Comment 9 Darin Wright CLA 2004-09-21 21:07:52 EDT
Deferred, waiting on JDI support.
Comment 10 Ilja Preuss CLA 2004-09-22 06:08:17 EDT
Bug 73029 is a similar, though possibly somewhat more general request.
Comment 11 Darin Wright CLA 2007-06-13 15:36:58 EDT
*** Bug 75519 has been marked as a duplicate of this bug. ***
Comment 12 Darin Wright CLA 2007-06-19 15:47:01 EDT
Java 6 has this feature (MethodExitEvent.returnValue()). Unfortunately this feature will be inefficient to take advantage of - it requires setting up method tracing on method exits to be able to obtain the return value. Method tracing is slow and is different than step requests... see EventRequestManager.createMethodExitRequest(...). It might be faster with thread filters, class filters, etc., but then you have to compute that information. As well, one needs to match up the method exit event with the end of a step (which can be hard in cases of recursion, etc., without tracing all method exits... which again, is slow).

The feature would be simpler to implement on "step return", rather than "step over".
Comment 13 Darin Wright CLA 2007-11-22 09:16:21 EST
*** Bug 210662 has been marked as a duplicate of this bug. ***
Comment 14 Eclipse Webmaster CLA 2009-08-30 02:38:23 EDT
As of now 'LATER' and 'REMIND' resolutions are no longer supported.
Please reopen this bug if it is still valid for you.
Comment 15 Eddie Galvez CLA 2009-09-14 11:17:20 EDT
I'd like for this request to reopen. What a huge request to lose to a bugzilla state cleanup!

Being able to see a method return value is a *huge* feature to have.
Comment 16 Darin Wright CLA 2009-09-14 11:22:41 EDT
Fair enough, re-opened.
Comment 17 Viliam Durina CLA 2009-11-04 04:45:31 EST
I remember having this feature in JDeveloper more than two years ago, even in JDK 1.5. I don't know how they did it, but I really miss it in Eclipse. Added a vote.
Comment 18 Henrik Lindberg CLA 2009-12-11 20:48:02 EST
(In reply to comment #15)
> Being able to see a method return value is a *huge* feature to have.
Agree - this would be very valuable! Hope there is a solution. I could live with only "step return" supporting this if that is easier to do.
Comment 19 Darin Wright CLA 2009-12-14 12:23:11 EST
Marking as help wanted. Currently there are no resources working on this.
Comment 20 arjan tijms CLA 2009-12-26 06:02:28 EST
Just the other day I was stepping through the code of Jboss Cache to solve an obscure bug. Due to a mechanism called 'interceptors' being used in abundance, call chains can be fairly deep; easily up to 20 or 30 calls with every method directly returning the result from the called method and in the end assigning the value to... nothing.

This makes for incredible slow, tedious and frustrating debugging. Here too, being a transactional cache that sets the status of internal nodes transactionally, I cannot always reliably use the watch expressions. It's a workaround sometimes, but a very, very, crude workaround.

I like to beg for this feature being implemented. If I remember correctly, Visual Studio had this feature back in 1998. I can imagine that C++/C# people coming from that platform to Eclipse, would surely expect something basic like this to be there.
Comment 21 Keith CLA 2010-05-28 15:03:42 EDT
When I run into this issue I have been modifying my code to inspect a local variable before returning it. It's a pain...

Glad to see others are interested, please get this done!
Comment 22 Bert van Brakel CLA 2011-06-21 11:30:03 EDT
+1 

Seems a no brainer as a requirement for a debugger. Be fine if it's only for a step return IMHO. Real pain currently when stepping through third party code.
Comment 23 arjan tijms CLA 2011-10-01 11:03:39 EDT
Any activity here? This bug is open for more than 8 years already...
Comment 24 Greg Wiley CLA 2012-07-13 17:50:24 EDT
I hit this exact need today while debugging what my method was returning to the Spring Framework.  Can we look forward to an implementation?
Comment 25 Michael Rennie CLA 2012-09-10 10:25:07 EDT
*** Bug 389127 has been marked as a duplicate of this bug. ***
Comment 26 Eugene Kaganovich CLA 2013-09-03 19:19:02 EDT
+1. I don't see a way to vote, but please implement this.
Comment 27 Peter Williams CLA 2014-04-22 09:06:16 EDT
+1 Ditto.  Also add a vote system please! ;-)
Comment 28 Stephan Herrmann CLA 2014-04-22 09:22:07 EDT
(In reply to Eugene Kaganovich from comment #26)
> +1. I don't see a way to vote, but please implement this.

I wonder how the 76 votes for this bug got entered ... :)

Check this line:

    Importance: P3 enhancement with 76 votes (vote) 

Is "vote" a link for you?

But the next field is even more important:

    Keywords: helpwanted
Comment 29 Markus Keller CLA 2014-04-22 10:55:47 EDT
(In reply to Stephan Herrmann from comment #28)
> (In reply to Eugene Kaganovich from comment #26)
> > +1. I don't see a way to vote, but please implement this.
> 
> I wonder how the 76 votes for this bug got entered ... :)

Voting sometimes gets lost during updates (bug 367771). This time, it was added back with bug 416527. Yes, I should have reported that here, since Eugene's comment made me open that bug.
Comment 30 Andrey Rogozhnikov CLA 2015-03-05 00:51:10 EST
It's odd to be unable to see return values because Qt Creator shows them on top of the same GDB (okay, it's jdt thread, but seems the right place to vote for the feature). Honestly, one cannot constantly change return statements throughout the code in order to see every return value of interest. I have no experience with programming for Eclipse itself and thus ask developers for this feature to be implemented. If there is some way to croudfund such issues, let everyone know.
Comment 31 Andrey Rogozhnikov CLA 2015-03-05 04:06:08 EST
(In reply to Andrey Rogozhnikov from comment #30)
> It's odd to be unable to see return values because Qt Creator shows them on
> top of the same GDB (okay, it's jdt thread, but seems the right place to
> vote for the feature). Honestly, one cannot constantly change return
> statements throughout the code in order to see every return value of
> interest. I have no experience with programming for Eclipse itself and thus
> ask developers for this feature to be implemented. If there is some way to
> croudfund such issues, let everyone know.

Sorry, CDT 8.5.0 already shows return values. Removed my vote.

(Eclipse IDE for C/C++ Developers
Version: Luna Service Release 1 (4.4.1)
Build id: 20140925-1800)
Comment 32 David Chassin CLA 2016-05-05 14:48:26 EDT
This really needs to be fixed.  It is an entirely reasonable request for "modern" IDE-based debugger, protocols and implementation challenges notwithstanding. The fact that this pretty basic capability has been languishing for almost 15 years is really quit astonishing because not having this is very frustrating. Even a simple compound "if" can be very tedious and slow to debug when you cannot quickly determine what the return values of the functions used to evaluate the expression.
Comment 33 feng lin CLA 2016-05-18 08:26:45 EDT
I think this can be done by use the aspectj or some code generate libs to insert the monitor code to the return . Or in the return byte code replace a method around it.
Comment 35 David Vree CLA 2016-06-11 18:50:42 EDT
SonarLint comes with a default rule for Java (squid:S1488) that says:

"Local Variables should not be declared and then immediately returned or thrown"

However, without the capability in Eclipse, the easiest way to view a return result is to assign a local variable even though it is consider code smell.
Comment 36 Eclipse Genie CLA 2016-07-03 06:11:17 EDT
New Gerrit change created: https://git.eclipse.org/r/76473
Comment 37 Till Brychcy CLA 2016-07-03 06:14:14 EDT
(In reply to Eclipse Genie from comment #36)
> New Gerrit change created: https://git.eclipse.org/r/76473

work in progress (but works already). 
values are displayed like in cdt as variable "someMethodName() returned"

to be done (at least:)
- add some toggle to switch this of
- handle a situation if a step return is interrupted, but a value from a recursive invocation has already been recorded.
Comment 38 Stephan Herrmann CLA 2016-07-03 09:28:29 EDT
(In reply to Till Brychcy from comment #37)
> (In reply to Eclipse Genie from comment #36)
> > New Gerrit change created: https://git.eclipse.org/r/76473
> 
> work in progress (but works already). 
> values are displayed like in cdt as variable "someMethodName() returned"

That's super cool news, thanks!

Given the high number of votes, we might publish an inofficial patch feature, so people can try this on any regular Neon install. WDYT? Is it ready to be tried? Do you have the means to publish a patch feature, do you want me to help? etc.
Comment 39 Sarika Sinha CLA 2016-07-03 23:32:25 EDT
(In reply to Eclipse Genie from comment #36)
> New Gerrit change created: https://git.eclipse.org/r/76473

Thanks Till for the contribution.
Comment 40 Michael Meß CLA 2016-07-04 03:57:11 EDT
As this works already, I would include this into the official build, so that many people have the chance to use it, not only the ones who do the work and install a patch.
The remaining issues that affect only some special cases may be addressed later.
Comment 41 Stephan Herrmann CLA 2016-07-04 11:32:16 EDT
(In reply to Michael Meß from comment #40)
> As this works already, I would include this into the official build, 

I personally would be interested in test driving the feature in an installation that is otherwise based on an official *release*. If everybody else is fine with using an unstable build for testing, no patch feature is needed. Anyway, it's up to Till and Sarika to decide when it is ready for pushing to master.
Comment 42 Till Brychcy CLA 2016-07-05 01:11:21 EDT
(In reply to Till Brychcy from comment #37)
> to be done (at least:)
> [...]
> - handle a situation if a step return is interrupted, but a value from a
> recursive invocation has already been recorded.

Done in patch set 2
Comment 43 Till Brychcy CLA 2016-07-05 01:55:55 EDT
(In reply to Stephan Herrmann from comment #38)
> That's super cool news, thanks!

Thanks for the motivating words :-)

> 
> Given the high number of votes, we might publish an inofficial patch
> feature, so people can try this on any regular Neon install. WDYT? Is it
> ready to be tried? Do you have the means to publish a patch feature, do you
> want me to help? etc.

Yes, It is ready to be tried - note that (as in CDT), just the "Step Return" case is implemented (see comment #12 for some background).

Doing a patch feature is an interesting suggestion. If we do that, it would be great if you could help, because I currently have no idea how and where to make and publish it.

But let me first add a setting to turn this off.
Comment 44 Till Brychcy CLA 2016-07-05 16:49:01 EDT
(In reply to Till Brychcy from comment #37)
> to be done (at least:)
> - add some toggle to switch this of

Done in patch set 3. Patch set 3 also contains two fixes (w.r.t. static methods and invocations of interface methods), more and improved tests and some other minor code improvements.
Comment 45 Till Brychcy CLA 2016-07-06 13:47:26 EDT
(In reply to Till Brychcy from comment #44)
> Done in patch set 3. Patch set 3 also contains two fixes (w.r.t. static
> methods and invocations of interface methods), more and improved tests and
> some other minor code improvements.

I forgot to write, the setting is in Preferences > Java > Debug.

@Sarika, you can look at the patch now!
Comment 46 Sarika Sinha CLA 2016-07-07 01:53:51 EDT
(In reply to Till Brychcy from comment #45)
> (In reply to Till Brychcy from comment #44)
> > Done in patch set 3. Patch set 3 also contains two fixes (w.r.t. static
> > methods and invocations of interface methods), more and improved tests and
> > some other minor code improvements.
> 
> I forgot to write, the setting is in Preferences > Java > Debug.
> 
> @Sarika, you can look at the patch now!

Thanks, will be looking at it early next week.
Comment 47 Till Brychcy CLA 2016-07-21 17:25:35 EDT
(In reply to Markus Keller from comment #2)
> And if a method call completes abruptly, the thrown exception should be shown
> instead of a return value.

Done in patch set 6.

I've used patch set 6 for some time now and it works great (except that returned null values are not shown)

But I found I really want to see the returned/thrown values for "step over", too.

After some thinking and experimenting, I have found a strategy that performs good enough: When a step-over is started, a MethodEntryRequest is created in addition to the requests created for step-return (needed as a step-over may return from the current method). if a MethodEntryEvent is observed, an automatic step-return is done (with the normal mechanism from patch set 6 active). If the returned location is still on the same line, another step-over is started.

This is done (prototype quality) in patch set 8.

TODO:
- show returned null values
- remove experimental preference for exceptions, improve label of existing preference
- add tests for step-over
- cleanup and possibly simplify code
Comment 48 Till Brychcy CLA 2016-07-23 12:32:01 EDT
> TODO:
> - show returned null values
> - remove experimental preference for exceptions, improve label of existing
> preference
> - add tests for step-over
> - cleanup and possibly simplify code

All TODOs are completed in patch set 11.
Comment 49 Sarika Sinha CLA 2016-07-23 12:55:49 EDT
(In reply to Till Brychcy from comment #48)
> > TODO:
> > - show returned null values
> > - remove experimental preference for exceptions, improve label of existing
> > preference
> > - add tests for step-over
> > - cleanup and possibly simplify code
> 
> All TODOs are completed in patch set 11.

Great!! Will be looking at the new patch set for support of Step Over.
Comment 50 Till Brychcy CLA 2016-07-28 14:38:39 EDT
(In reply to Sarika Sinha from comment #49)
> Great!! Will be looking at the new patch set for support of Step Over.

As Markus today wrote, 4.7M1 is drawing near - do you think we can get this into it?
Comment 51 Sarika Sinha CLA 2016-07-29 02:24:21 EDT
(In reply to Till Brychcy from comment #50)
> As Markus today wrote, 4.7M1 is drawing near - do you think we can get this
> into it?

It will be difficult for M1 with Step Over feature. We will target first week of M2 with Step Return and Step Over together.
Comment 52 Stephan Herrmann CLA 2016-08-03 14:00:51 EDT
(In reply to Sarika Sinha from comment #51)
> (In reply to Till Brychcy from comment #50)
> > As Markus today wrote, 4.7M1 is drawing near - do you think we can get this
> > into it?
> 
> It will be difficult for M1 with Step Over feature. We will target first
> week of M2 with Step Return and Step Over together.

Target Milestone currently says 4.6.1 (which would be lovely), but I don't think this is realistic, is it?
Comment 53 Sarika Sinha CLA 2016-08-05 01:38:51 EDT
(In reply to Stephan Herrmann from comment #52)
> Target Milestone currently says 4.6.1 (which would be lovely), but I don't
> think this is realistic, is it?

It is. Plan is to release it on Monday to Master. If everything goes fine we can seek pmc approval for 4.6.1
Comment 54 Sarika Sinha CLA 2016-08-08 00:31:20 EDT
Released to master.
Increased the bundle version
http://git.eclipse.org/c/jdt/eclipse.jdt.debug.git/commit/?id=23ac170f3b1ad104b9d4d3f3a27baf1209eb385a
Created Bug 499339 to track adding of filtering of methods for return value calculation.
Comment 57 Dani Megert CLA 2016-08-11 02:29:13 EDT
(In reply to Sarika Sinha from comment #53)
> (In reply to Stephan Herrmann from comment #52)
> > Target Milestone currently says 4.6.1 (which would be lovely), but I don't
> > think this is realistic, is it?
> 
> It is. Plan is to release it on Monday to Master. If everything goes fine we
> can seek pmc approval for 4.6.1

It is still a bit too early for that. See my comments ins https://dev.eclipse.org/mhonarc/lists/eclipse-pmc/msg02676.html.
Comment 58 Till Brychcy CLA 2016-08-11 15:57:55 EDT
(In reply to Dani Megert from comment #57)
> It is still a bit too early for that. See my comments ins
> https://dev.eclipse.org/mhonarc/lists/eclipse-pmc/msg02676.html.

As hint for those who think they can't wait: you can try an integration build like

http://download.eclipse.org/eclipse/downloads/drops4/I20160809-1300/

or later. If additional packages like marketplace client, m2e etc. are not yet available in the preconfigured installation locations, you may want to add 

http://download.eclipse.org/releases/neon

as installation location for the neon versions.

Of course: be warned, integration and nightly builds in general contain fresh code that may break things.
Comment 59 Stefan Xenos CLA 2016-08-11 21:01:10 EDT
Wow, I just noticed this feature in the latest I-build. This is awesome. Good job, guys.
Comment 60 Sarika Sinha CLA 2016-09-12 03:08:37 EDT
Verified using
Eclipse SDK

Version: Oxygen (4.7)
Build id: N20160911-0220
Comment 61 Eclipse Genie CLA 2016-09-14 16:19:35 EDT
New Gerrit change created: https://git.eclipse.org/r/81132
Comment 62 Till Brychcy CLA 2016-09-14 16:22:59 EDT
(In reply to Eclipse Genie from comment #61)
> New Gerrit change created: https://git.eclipse.org/r/81132

"New & Noteworthy" entry for 4.7M2
Comment 63 Stephan Herrmann CLA 2016-09-14 18:35:21 EDT
(In reply to Till Brychcy from comment #62)
> (In reply to Eclipse Genie from comment #61)
> > New Gerrit change created: https://git.eclipse.org/r/81132
> 
> "New & Noteworthy" entry for 4.7M2

I made minor editorial modifications, tried to re-submit to gerrit (was told I'm not allowed to), and finally - to meet the deadline 'today' EOD - pushed the change directly as http://git.eclipse.org/c/www.eclipse.org/eclipse/news.git/commit/?id=994feb70bd8f92d715eaee869559ec5ba36c34f6

HTML was checked OK.
Comment 64 Till Brychcy CLA 2016-09-15 16:46:00 EDT
(In reply to Stephan Herrmann from comment #63)
> I made minor editorial modifications, tried to re-submit to gerrit (was told
> I'm not allowed to), and finally - to meet the deadline 'today' EOD - pushed
> the change directly as
> http://git.eclipse.org/c/www.eclipse.org/eclipse/news.git/commit/
> ?id=994feb70bd8f92d715eaee869559ec5ba36c34f6
> 
> HTML was checked OK.

Thanks
Comment 65 Eclipse Genie CLA 2017-03-11 04:19:43 EST
New Gerrit change created: https://git.eclipse.org/r/92839
Comment 67 Andrey Loskutov CLA 2018-07-06 07:09:49 EDT
We see (bug 533702 comment 7) that "show return value ON" introduced and enabled by default via this bug can show dramatic overhead (x800) compared with usual execution time and x25 compared with interpreted execution, so I've proposed to disable this feature by default, as very dangerous in bug 536751.

Please comment on bug 536751.
Comment 68 Till Brychcy CLA 2018-07-06 07:36:04 EDT
(In reply to Andrey Loskutov from comment #67)
> We see (bug 533702 comment 7) that "show return value ON" introduced and
> enabled by default via this bug can show dramatic overhead (x800) compared
> with usual execution time and x25 compared with interpreted execution, so
> I've proposed to disable this feature by default, as very dangerous in bug
> 536751.
> 
> Please comment on bug 536751.

Just for future readers, as this is out of context: in that specific use case, the relative slowdown by the feature implemented in this bug is just 7x, the rest (x100) is just from stepping