Bug 571310

Comment 1 Gayan Perera 2021-02-18 16:46:29 EST

The problem seems to be related to how the code is generated at org.eclipse.jdt.internal.eval.CodeSnippetMessageSend.generateCode(BlockScope, CodeStream, boolean)

The root cause is for this.publicMethod() it delegate the code generation to receiver which fails with synthetics lookup.

Comment 2 Gayan Perera 2021-02-18 16:51:20 EST

@Sarika what is the meaning of IsCapturedOuterLocal and DepthMASK ?

Comment 3 Gayan Perera 2021-02-20 06:09:09 EST

@Sarika and @Aundrey i need some help on the following test in CodeSnippetTest

public void testBug571310_ThisReciever() {
	if (this.complianceLevel < ClassFileConstants.JDK1_8) {
		return;
	}
	this.context.setImports(new char[][] {"java.util.function.*".toCharArray()});
	evaluateWithExpectedDisplayString(buildCharArray(new String[] {
			"class Outer {",
			"	public int outerFoo() {",
			"		return 10;",
			"	}",
			"	public int boo() {",
			"		Function<Integer,Integer> f = i -> this.outerFoo() + i;",
			"		return f.apply(5);",
			"	}",
			"};",
			"(new Outer()).outerFoo();"}),
			"15".toCharArray());
}

The strange thing is when i write this code as a scrapbook page and it runs fine. The problem seems to be related with the lambda in the test. When i remove the lambda line it work.

Comment 4 Sarika Sinha 2021-02-20 07:52:55 EST

(In reply to Gayan Perera from comment #2)
> @Sarika what is the meaning of IsCapturedOuterLocal and DepthMASK ?

I don't have exact answer to this.
@Manoj,
Will you be able to help with this and CodeSnippetTest?

Comment 5 Gayan Perera 2021-02-21 15:11:12 EST

@Sarika @Manoj after 2 days of vigorous debugging i found the problem. may be i spend to much time digging into compiler code, but it was fun :).

The problem is that when we have lambdas, the resulting inner class has some changes compared to a normal inner class. This cause the index issue in org.eclipse.jdt.core.tests.eval.target.CodeSnippetRunner classname resolution. This is not a issue in LocalEvaluationEngine because it actually gets the class file names from caller.

So for tests either we can fix the org.eclipse.jdt.core.tests.eval.target.CodeSnippetRunner class name resolution when synthetic methods are added. But i'm not sure if its only when synthetic methods or if the problem is with all kind of synthetic attributes.

Another solution is we pass in the class names from the tests into this remote evaluator so that don't need to play around with class bytes.

I feel that second option is better since we don't need to keep on updating this when ever the class format changes.

WDYT ?

Comment 6 Sarika Sinha 2021-02-22 00:11:12 EST

Sorry, Forgot to add Manoj :) 


>>Another solution is we pass in the class names from the tests into this remote >>evaluator so that don't need to play around with class bytes.

For tests, this should be fine.

Comment 7 Manoj N Palat 2021-02-22 00:30:44 EST

(In reply to Sarika Sinha from comment #6)
> Sorry, Forgot to add Manoj :) 
> 

Thanks @Sarika!

> >>Another solution is we pass in the class names from the tests into this remote >>evaluator so that don't need to play around with class bytes.
> 
> For tests, this should be fine.


@Gayan: I need to candidly admit that I am not familiar with the eval part of the code and hence cannot provide you with an answer that has some value at this point in time. However, if the issue is not yet resolved until after Java 16, I will try to spend some time on this.

Comment 8 Gayan Perera 2021-02-22 01:45:13 EST

@Manoj sure i will create a patch and add you as an reviewer. I found this snippet test problem when i try to fix and run tests the original bug this issue is reported for. So until we get codesnippet tests running its hard to give a proper fix for snippet compiler.

Comment 9 Gayan Perera 2021-02-22 12:59:16 EST

@Manoj i fix the code runner, i feel that adding class names was kind of a bigger change. So i added the remaining constant pool constants into the code. Now the CodeSnippetRunner should be capable of reading class file name up to java 15.

Comment 10 Gayan Perera 2021-02-22 13:00:01 EST

Now i will focus in fixing the regression issue i found and lets see if it will be good to include in this release if i can finish it soon.

Comment 11 Eclipse Genie 2021-02-23 12:55:31 EST

New Gerrit change created: https://git.eclipse.org/r/c/jdt/eclipse.jdt.core/+/176748

Comment 12 Eclipse Genie 2021-06-18 06:15:18 EDT

Gerrit change https://git.eclipse.org/r/c/jdt/eclipse.jdt.core/+/176748 was merged to [master].
Commit: http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?id=63e0f757b2acc1528310b44b3463bd2bd4858310

Comment 13 Sarika Sinha 2021-06-18 08:37:08 EDT

@Gayan,
Please mark the bug as resolved after releasing the changes.

Comment 14 Sarika Sinha 2021-07-06 09:28:30 EDT

@Gayan,
Can we add a Debug test case for the fixed case :
assertFunctionalExpression(obj -> this.publicMethod() + 8, this, 13)

Comment 15 Gayan Perera 2021-07-06 11:54:44 EDT

Sure @Sarika, is it ok i add it tomorrow as a different gerrit ?

Comment 16 Sarika Sinha 2021-07-06 13:43:28 EDT

(In reply to Gayan Perera from comment #15)
> Sure @Sarika, is it ok i add it tomorrow as a different gerrit ?

Yes, perfect.

Comment 17 Eclipse Genie 2021-07-06 14:44:49 EDT

New Gerrit change created: https://git.eclipse.org/r/c/jdt/eclipse.jdt.debug/+/182821

Comment 18 Eclipse Genie 2021-07-07 05:57:48 EDT

Gerrit change https://git.eclipse.org/r/c/jdt/eclipse.jdt.debug/+/182821 was merged to [master].
Commit: http://git.eclipse.org/c/jdt/eclipse.jdt.debug.git/commit/?id=4cb87a554926ffffff65f4d6c9e38453c31f6777