Community
Participate
Working Groups
Taking this out of Bug 516319. arg$1, arg$2 etc should be replaced by proper variable names. arg$1 is by default the class having lambda.
@Jesper. Do you have any inputs on this? I was analyzing it but I see that the name arg$1 etc come from the data stream retrieved from the jdwp command . org.eclipse.jdi.internal.FieldImpl.readWithNameSignatureModifiers(ReferenceTypeImpl, ReferenceTypeImpl, boolean, DataInputStream) At that level we don't have the information about the thread stack.
I don't see any other way than "guessing" from the lambda expression from the source position of the program counter: * Take the AST of the lambda expression * Look at the the lambda expression escape analysis: - If the lambda is not static (shouldCaptureInstance == true), then the first (arg$1) is the enclosing 'this' reference. - The remaining arg$n variables should match (in order) the values in syntheticOuterLocalVariables. But ... since we'd be guessing, there are some potential problems that need remedy: #0: It seems to me that the AST doesn't carry the results of the variable escape analysis, so that may require an extension of the AST conversion process #1: Finding out that we are on a lambda expression is JRE implementation specific. I'm now aware of any variations in the implementation at present, but it could happen. Graceful degradation is key. #2: The guessing assumes that the code is made by a compiler which works like ECJ. As I recall from the spec, the compiler is free to order the variables as it pleases, box them, or whatever. Also, if the the code is changed during debugging, that could make the analysis wrong. Again: Graceful degradation is key - if the types don't match, fallback to the current implementation. #3: There could be several lambda expressions near the source position (because we only know the pc by source line, not column, right?). Example: int n = 42; Function<Integer, Integer> nop = Function.<Integer>identity().andThen(x -> x+n).andThen(x -> x-n); We can't know which one is the active if there are more than one lambda on that line. So we'd have to check all lambdas on that line and consider only those whose functional interface type from the AST match with the implemented interfaces of the lambda object. Or perhaps we can match it, since we know from the method which lambda implementation method we're in, and we can match that back to the position of the lambda expression, again. Lambda implementation methods are typically implemented in source order under their implementation reference type. At least in ECJ... Anyway, if the search (filtered by check #2) is ambiguous or doesn't match, fallback to the current behaviour. #4: There are a few cases where we build "implicit lambdas" where we can't wrangle the context of reference expression into a direct MethodHandle at bootstrap, and so an implicit lambda is generated with an extra invocation to the method reference. This is an example: BiFunction<Integer, Integer, List<Integer>> toList = Arrays::asList; // varargs-bridge needed Such a lambda object could show up in a stack frame, but I'm pretty sure that it doesn't carry a source line reference back to the reference expression. Again: If there's no source information, fallback to the current behaviour. So, there's a bit of work to do in making it work, but it could be quite useful.
Thanks Jesper for providing an in depth analysis. Will you have some time in the near future to provide an initial patch? I can take it forward from there.
(In reply to Sarika Sinha from comment #3) > Thanks Jesper for providing an in depth analysis. > > Will you have some time in the near future to provide an initial patch? > I can take it forward from there. Unfortunately, no. I really don't know enough about how everything fits together in the debugger, and I'm also busy with ECJ Java 12 work (and my day job).
Will have to re plan for this, doesn't look like it will be possible in near future.
Due to Java 12 work, could not focus on this.
I will be try to work on couple of things - 1. Try to give names for the simplest case first 2. As we will be guessing there might be cases that it doesn't satisfy and we fallback to arg$1 etc 3. Once this is achieved work on Point 3 and 4 from #c2
(In reply to Jesper Moller from comment #2) > I don't see any other way than "guessing" from the lambda expression from > the source position of the program counter: > * Take the AST of the lambda expression > * Look at the the lambda expression escape analysis: > - If the lambda is not static (shouldCaptureInstance == true), then the > first (arg$1) is the enclosing 'this' reference. > - The remaining arg$n variables should match (in order) the values in > syntheticOuterLocalVariables. > > @Jesper, Can you elaborate a little on what you meant while saying this ? While we are trying to find the variables, what we have is the lambda method, Thread and the stack frames. org.eclipse.jdt.internal.debug.core.model.JDIStackFrame.getVariables0()
(In reply to Sarika Sinha from comment #8) > @Jesper, > Can you elaborate a little on what you meant while saying this ? > While we are trying to find the variables, what we have is the lambda > method, Thread and the stack frames. > > org.eclipse.jdt.internal.debug.core.model.JDIStackFrame.getVariables0() What I mean is that we'd have to take the AST information for the compilation unit into account, and extract the information from it. By using the line number and lambda method information, we *should* be able to find the enclosing expression and use the semantic information gathered *at* *debug* *time* to figure out which variables are which.
(In reply to Jesper Moller from comment #9) > (In reply to Sarika Sinha from comment #8) > > @Jesper, > > Can you elaborate a little on what you meant while saying this ? > > While we are trying to find the variables, what we have is the lambda > > method, Thread and the stack frames. > > > > org.eclipse.jdt.internal.debug.core.model.JDIStackFrame.getVariables0() > > What I mean is that we'd have to take the AST information for the > compilation unit into account, and extract the information from it. By using > the line number and lambda method information, we *should* be able to find > the enclosing expression and use the semantic information gathered *at* > *debug* *time* to figure out which variables are which. Thanks, working on it.
New Gerrit change created: https://git.eclipse.org/r/141304
New Gerrit change created: https://git.eclipse.org/r/141316
(In reply to Eclipse Genie from comment #12) > New Gerrit change created: https://git.eclipse.org/r/141316 @Sarika: Please find a WIP solution for the mapping for synthetic outer local variables of lambda. Idea is to pass the information of synthetic outer local variables to the LambdaMethod binding at the resolveMethod stage. The ASTConverter18 has a test case which provides an example of the usage. You can take this patch further (needs more testing as well) in a separate do mast bug, since this introduces API changes. Feel free to modify this if you figure out a better way. Note: synthetic outer locals are prefixed with TypeConstants.SYNTHETIC_OUTER_LOCAL_PREFIX which is val$ and hence the prefix.
(In reply to Manoj Palat from comment #13) > (In reply to Eclipse Genie from comment #12) > > New Gerrit change created: https://git.eclipse.org/r/141316 > > @Sarika: Please find a WIP solution for the mapping for synthetic outer > local variables of lambda. Idea is to pass the information of synthetic > outer local variables to the LambdaMethod binding at the resolveMethod > stage. The ASTConverter18 has a test case which provides an example of the > usage. You can take this patch further (needs more testing as well) in a > separate do mast bug, since this introduces API changes. Feel free to modify > this if you figure out a better way. > Note: synthetic outer locals are prefixed with > TypeConstants.SYNTHETIC_OUTER_LOCAL_PREFIX which is val$ and hence the > prefix. Thanks Manoj ! I have updated JDT debug patch to use these new API's from JDT Core. It works and we can see the variable names. I have created bug in JDT core 547148.
(In reply to Sarika Sinha from comment #14) > Thanks Manoj ! I have updated JDT debug patch to use these new API's from > JDT Core. It works and we can see the variable names. > > I have created bug in JDT core 547148. Cool.
Questions/Comments about the patch: 1. What's the motivation for the default method in IMethodBinding? Do we believe that there would be other implementors of IMethodBinding? 2. Can we create a constant and reuse it instead of creating an empty array every time, like we do for NO_TYPE_BINDINGS? If we do that, we should do for the default method as well as the implementation.
(In reply to Jay Arthanareeswaran from comment #16) > Questions/Comments about the patch: > > 1. What's the motivation for the default method in IMethodBinding? Do we > believe that there would be other implementors of IMethodBinding? I see @noImplement so default method need not be required. @Manoj can confirm. > > 2. Can we create a constant and reuse it instead of creating an empty array > every time, like we do for NO_TYPE_BINDINGS? If we do that, we should do for > the default method as well as the implementation. Updated the gerrit with this change.
(In reply to Jay Arthanareeswaran from comment #16) > Questions/Comments about the patch: > > 1. What's the motivation for the default method in IMethodBinding? Do we > believe that there would be other implementors of IMethodBinding? @Christian: I know XText sometimes does illegal things with JDT - can you please check if XText would be affected by the new code (implements IMethodBinding which is supposed not to be implemented by clients)?
afaik we dont implement IMethodBinding if that is the question
(In reply to Jay Arthanareeswaran from comment #16) > Questions/Comments about the patch: > > 1. What's the motivation for the default method in IMethodBinding? Do we > believe that there would be other implementors of IMethodBinding? I took the liberty to update the patch and made this method abstract.
(In reply to Andrey Loskutov from comment #20) > (In reply to Jay Arthanareeswaran from comment #16) > I took the liberty to update the patch and made this method abstract. +1. Thanks Andrey.
(In reply to Manoj Palat from comment #21) > (In reply to Andrey Loskutov from comment #20) > > (In reply to Jay Arthanareeswaran from comment #16) > > > I took the liberty to update the patch and made this method abstract. > > +1. Thanks Andrey. Manoj, are you OK with the patch itself? Can we merge?
(In reply to Andrey Loskutov from comment #22) > Manoj, are you OK with the patch itself? Can we merge? Yes Andrey, it can be merged. Thanks
(In reply to Manoj Palat from comment #23) > Yes Andrey, it can be merged. Thanks JDT is merged, we should wait for the next build to run Gerrit for JDT debug change.
Gerrit change https://git.eclipse.org/r/141304 was merged to [master]. Commit: http://git.eclipse.org/c/jdt/eclipse.jdt.debug.git/commit/?id=b318840e3fcadffad066a4c6e48822620f39bf3f
I think we are done here.
We see NPE in the new code in our product tests. I will push a patch ASAP. The code works on a XBase based DSL code that generated to a class file and does not have source attached. In the test we stop debugger somewhere at this code and request evaluation. What we see is this: java.lang.NullPointerException at org.eclipse.jdt.internal.debug.core.model.JDIStackFrame.setLambdaVariableNames(JDIStackFrame.java:412) at org.eclipse.jdt.internal.debug.core.model.JDIStackFrame.getVariables0(JDIStackFrame.java:387) at org.eclipse.jdt.internal.debug.core.model.JDIStackFrame.getVariables(JDIStackFrame.java:309) at org.eclipse.jdt.internal.debug.core.model.LambdaUtils.getLambdaFrameVariables(LambdaUtils.java:88) at org.eclipse.jdt.internal.debug.eval.ast.engine.ASTEvaluationEngine.getCompiledExpression(ASTEvaluationEngine.java:306) at org.eclipse.jdt.internal.debug.eval.ast.engine.ASTEvaluationEngine.evaluate(ASTEvaluationEngine.java:148)
New Gerrit change created: https://git.eclipse.org/r/142415
Gerrit change https://git.eclipse.org/r/142415 was merged to [master]. Commit: http://git.eclipse.org/c/jdt/eclipse.jdt.debug.git/commit/?id=b8828be9fef42f3494814ea93a54a243511afc3c
Verified using Build id: I20190520-1805. Thanks Jesper and Andrey!
I see new NPE coming from this bug. Not reproducible yet, it happened while I've opened Variables view and the debugger was stopped at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Display, WorkbenchAdvisor) line 574 System.setProperty(org.eclipse.e4.ui.workbench.IWorkbench.XMI_URI_ARG // BP here An internal error occurred during: "Debug child count update". java.lang.NullPointerException at org.eclipse.jdt.internal.debug.core.model.JDIStackFrame$LambdaASTVisitor.visit(JDIStackFrame.java:433) at org.eclipse.jdt.core.dom.LambdaExpression.accept0(LambdaExpression.java:195) at org.eclipse.jdt.core.dom.ASTNode.accept(ASTNode.java:2836) at org.eclipse.jdt.core.dom.ASTNode.acceptChildren(ASTNode.java:2907) at org.eclipse.jdt.core.dom.MethodInvocation.accept0(MethodInvocation.java:228) at org.eclipse.jdt.core.dom.ASTNode.accept(ASTNode.java:2836) at org.eclipse.jdt.core.dom.ASTNode.acceptChild(ASTNode.java:2884) at org.eclipse.jdt.core.dom.CastExpression.accept0(CastExpression.java:155) at org.eclipse.jdt.core.dom.ASTNode.accept(ASTNode.java:2836) at org.eclipse.jdt.core.dom.ASTNode.acceptChild(ASTNode.java:2884) at org.eclipse.jdt.core.dom.Assignment.accept0(Assignment.java:303) at org.eclipse.jdt.core.dom.ASTNode.accept(ASTNode.java:2836) at org.eclipse.jdt.core.dom.ASTNode.acceptChild(ASTNode.java:2884) at org.eclipse.jdt.core.dom.ExpressionStatement.accept0(ExpressionStatement.java:136) at org.eclipse.jdt.core.dom.ASTNode.accept(ASTNode.java:2836) at org.eclipse.jdt.core.dom.ASTNode.acceptChildren(ASTNode.java:2907) at org.eclipse.jdt.core.dom.Block.accept0(Block.java:128) at org.eclipse.jdt.core.dom.ASTNode.accept(ASTNode.java:2836) at org.eclipse.jdt.core.dom.ASTNode.acceptChild(ASTNode.java:2884) at org.eclipse.jdt.core.dom.MethodDeclaration.accept0(MethodDeclaration.java:617) I will push patch in a second.
New Gerrit change created: https://git.eclipse.org/r/142511
Gerrit change https://git.eclipse.org/r/142511 was merged to [master]. Commit: http://git.eclipse.org/c/jdt/eclipse.jdt.debug.git/commit/?id=4ad9eaa437b24815201e93fb151930a6cef6aa39
(In reply to Andrey Loskutov from comment #31) > I see new NPE coming from this bug. > > Not reproducible yet, it happened while I've opened Variables view and the > debugger was stopped at > > org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Display, > WorkbenchAdvisor) line 574 > > System.setProperty(org.eclipse.e4.ui.workbench.IWorkbench.XMI_URI_ARG // BP > here It IS reproducible, if the Variables view was closed before and opened on the BP hit above. (In reply to Eclipse Genie from comment #33) > Gerrit change https://git.eclipse.org/r/142511 was merged to [master]. > Commit: > http://git.eclipse.org/c/jdt/eclipse.jdt.debug.git/commit/ > ?id=4ad9eaa437b24815201e93fb151930a6cef6aa39 This fixes the problem. I hope this was the last regression here :-)
Verified again with I20190521-1800. No NPE. However this bug solution only works for projects that are available as source projects in the workspace. For libraries, even if we have attached sources, we still don't show the proper lambda variables, but just arg$1 etc. I've closed org.eclipse.ui.workbench project and then debugged the lambda inside org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Display, WorkbenchAdvisor). What is also surprising, the debug hovers seem not to find *any* element to show in that case - lambda locals or not, independently. I will create another bug for that, because majority of developers don't have all the libraries as source code in their workspaces.
