XtendJvmModelInferrer and treatment of anonymous classes

[LorenzoBettini]

The Xtend inferrer does this:

	public void inferLocalClass(
			AnonymousClass anonymousClass,
			String localClassName,
			JvmFeature container) {
		final JvmGenericType inferredType = typesFactory.createJvmGenericType();
		inferredType.setSimpleName(localClassName);
		inferredType.setAnonymous(!hasAdditionalMembers(anonymousClass));
...
	}
	
	protected boolean hasAdditionalMembers(AnonymousClass anonymousClass) {
		for(XtendMember member: anonymousClass.getMembers()) {
			if(member instanceof XtendField ||	
				(member instanceof XtendFunction && !((XtendFunction) member).isOverride())) 
				return true;
		}
		return false;
	}

Thus, an anonymous Xtend class is NOT mapped to a JvmGeneritcType.isAnonymous if it contains a field or a non-overridden method.

Is there a specific reason?

While working on #2880 I'd like to analyze also JvmGenericTypes corresponding to anonymous classes, but with the above behavior, I cannot correctly detect them: if isAnonymous returns false it can still correspond to an anonymous class. Some checks on anonymous classes must be different from standard classes or interfaces. First, an anonymous class's supertype can be a class or interface. For standard classes, I have to distinguish between the intended superclass and the intended implemented interfaces.

cc @szarnekow

[szarnekow]

Even though a class is an anonymous class in Xtend, it’s not necessary possible to compile it to an anonymous Java class. In some cases , it’s necessary to compile to a named, local class.

[LorenzoBettini]

@szarnekow but if I understand correctly, that's only a code generation strategy; I mean, that check could be delayed to code generation instead of at the JVM inferer level. I did a quick experiment (setting isAnonymous to true) and the failing test only concern code generation. Can it be?

[szarnekow]

I might be confused. The JVM model is the input model for the code generator, no?

[LorenzoBettini]

@szarnekow but from a JVM model's point of view, Xtend anonymous classes could always be mapped to anonymous JvmGenericTypes; it's just that, as you said, in some cases, the Java code generation must be different. I was proposing to do the check when generating code, not when inferring the JVM model. This would allow me to always detect JVM anonymous classes (in #2880), while not changing the Xtend compilation.

I'm doing some experiments in that direction.

[szarnekow]

I don‘t think it’s true. To make the additionally defined members accessible in Java from the enclosing method, you need a named type in Java8. It‘s not a given that these members are ever accessed, but we don’t know that during model inference. I’m curious about your findings though.

[LorenzoBettini]

@szarnekow here's my experiment (build is green)

LorenzoBettini@055d211

the changes are rather minimal:

• the inferred always sets the inferred type for AnonymousClass as anonymous
• the compiler, i.e., the XtendGenerator and XtendCompiler are modified to perform the check that the inferrer used to do (I used the name canBeCompiledAsJavaAnonymousClass instead of hasAdditionalMembers and inverted the semantics of the return value to make it more informative, hopefully); in this experiment, the check is duplicated in both classes, but of course, it can be factored out in a util class. Note that compileLocalTypeStubs has to modify the anonymous feature to false to let the rest of XbaseCompiler work as expected from then on.

All the tests are still green (https://github.com/LorenzoBettini/xtext/actions/runs/7365585524): in particular, the compiled code is the same as before.
Of course, I had to adapt only LorenzoBettini@055d211#diff-8d2c98464642a8554f494c6f4c1de6f865cbec0d6cb5b288412da41e6067cba1R222 because the inferrer has changed.

I'm not sure I understand the case you mentioned above.
Do you mean something like that?

class Foo {
	def m() {
		val e = new Exception("") {
			int i = 0;
		}
		e.i = 3
	}
}

This behaves as before: type inference and validation work as before, and the Java code is generated as before, that is, with a local class in the generated Java method.

From what I understand, as long as the inferrer does inferredType.getSuperTypes().add(jvmTypesBuilder.inferredType(anonymousClass)); as before, everything is fine from the typing point of view.

[LorenzoBettini]

@szarnekow I have another working proposal for this issue that does not touch the inferred model.

I created a PR on my fork LorenzoBettini#1 because the proposal is built on top of #2890;
without #2890 there would be a few more work to do, so I thought I'd base my proposal on #2890 (hoping it gets merged). It also uses the change proposed in #2889. This way, you should be able to see only the changes relevant to this issue.

the changes are rather minimal:

• the inferrer always sets the inferred type for AnonymousClass as anonymous
• a new XtendCompilerHelper performs the check the inferred used to do, i.e., the anonymous class does not contain fields or methods that are not overridden
• the XtendGenerator uses this helper to know whether it has to create nested local classes instead of anonymous classes; previously, it only checked JvmGenericType.isAnonymous.
• the XtendCompiler redefines the new XbaseCompiler.canCompileToJavaAnonymousClass(XConstructorCall constructorCall) introduced in Xbase/Xtend refactoring #2890 and delegates to XtendCompilerHelper
• a custom TreeAppendable, I called that AnonymousClassAwareTreeAppendable but it could be called XtendTreeAppendable, uses a custom LightweightTypeReferenceSerializer to decide what to do with JvmGenericType that are "anonymous", again, using the XtendCompiler. This is useful to intercept all the serialization of types related to anonymous classes correctly. While the previous customizations catch anonymous classes in "new" expressions, this one catches the occurrences of related types in other parts, e.g., in local variables in case anonymous classes are compiled into nested local classes.

This way, during the Xtend compilation process, anonymous classes are correctly generated as Java anonymous classes or nested local classes. All the tests are green. All the generated code is as before. No test has been touched apart from this one https://github.com/LorenzoBettini/xtext/pull/1/files#diff-8d2c98464642a8554f494c6f4c1de6f865cbec0d6cb5b288412da41e6067cba1L222 of course.

If #2890 gets merged, and you think that LorenzoBettini#1 makes sense I'll create a PR for this repository.
Thanks in advance for the feedback.

[LorenzoBettini]

@szarnekow disregard the previous comment, which is now stale. I've created the new #2898, where I updated the comments.

[LorenzoBettini]

Fixed by #2898