#135 fix generated compile errors for nested and generic dataype params

[bergmanngabor]

The classes generated from this pattern now compile correctly:

pattern bar(e: java ^java.util.Map.Entry, u: java Integer) {
	e == eval(^java.util.Collections.singletonMap(1,2).entrySet.head);
	u == 4;
}

(And the query results are correct as well)

[bergmanngabor]

Fixes #135

[ujhelyiz]

The implementation looks fine by me, but I really would like to have two tests:

1. Adds something like query/tests/org.eclipse.viatra.query.patternlanguage.emf.ui.tests/src/org/eclipse/viatra/query/patternlanguage/emf/tests/generator/HintCompilerTest.xtend to test that the compiler works as expected. The pattern written in the description would be a fine test case here.
2. I'd also like to have some test where the functionality is tested with the query interpreter as well.

[bergmanngabor]

Huge thanks for asking for more tests! You made the right call - the more extensive test coverage revealed that the solution was not quite perfect, since there were serious underlying classloading problems as well (some related, some unrelated to inner classes or generic classes). This lead me down a rabbit hole that culminated in the introduction of a caching mechanism to the Java project based classloader.

[bergmanngabor]

(Note that what I have introduced is resource-based caching, so one classloader per vql file. If you can tell me how to have one shared classloader per source project rather than resource, I am all ears. Even better: can related projects have related classloaders?)

[ujhelyiz]

I have a technical-level suggestion and a more generic question (added them as specific discussion).

(Note that what I have introduced is resource-based caching, so one classloader per vql file. If you can tell me how to have one shared classloader per source project rather than resource, I am all ears. Even better: can related projects have related classloaders?)

To be honest, I have no idea about this, I had to have a deep dive into the codebase of Eclipse/Xtext to at least have an idea what I should be talking about. I will think a bit about how to reuse class loaders between calls, but at the first glance I am averse this Resource-based caching solution...

[ujhelyiz]

Wouldn't it work to replace the @Inject injector declaration in the top of the file with a @Inject JavaReflectAccess javaReflect instead? I'd assume this method is stateless (based on the naming convention Xtext uses), so it could be reused between calls, thus simplifying the code.

[bergmanngabor]

Well javaReflect is not stateless, see 344. This is why I am afraid of reusing it - but maybe this is ok in a single-threaded app.

[ujhelyiz]

What/how will break if the classpath is stale? Furthermore, did you see a case where you have produced such a stale cache issue? How easy it is to get that out?

The reason I am asking this is that I'm really afraid to add another known cache invalidation issue to the codebase without knowing the possible consequences.

[bergmanngabor]

There is a narrow window of error if classpath is stale: the user may e.g. change some classes in source files without touching the vql, and Wuery Explorer would still work with the old versions of the classes. Note that this cannot happen to classes that are compiled and part of the runtime Eclipse.

On the other hand, there is a huge window of error if the classloader is never reused. For instance, the test patterns added with this PR would break in a runtime Eclipse UI (but frustratingly would still pass automated tests that use the Simple classload mechanism). See, the Enum BackendRequirement loaded as a java constant expression or eval and the one decorating the header as a parameter type filter will be different Class<> objects from different classloaders, and hence the type filtering constraint will always fail.

[bergmanngabor]

I have tested the behaviour with a debugger, and was unable to produce a state of staleness. Apparently, Xtext/Xbase will evict the cache even if the change only affects other files (I experimented with breaking the superclass relationship between two Java classes, neither of them directly mentioned in the .vql). Whether Xtext will always do this eviction when necessary, I cannot tell. But we will be no worse than Xbase itself.

I currently see no reason to reject the application of this cache, and am looking forward to both correctness and performance improvements enabled by this PR.

[ujhelyiz]

Understood, but please give me a bit of time to think about this approach, I am still wary of this. If I cannot find anything better before we are finalizing version 2.9, I promise to merge this change as it is now, but please give me a few days to think about other possible approaches.

[bergmanngabor]

Fixes #125 as well.