Community
Participate
Working Groups
The current implemention of type parameters and parameterized type references (scoping and validation) do not work in the case several Jvm elements are inferred for the same parameterized element in the original code. I experienced this while implementing this language https://github.com/LorenzoBettini/xtraitj but I extracted the interesting parts and reproduced the problem in a simple example that can be found here: https://bitbucket.org/lbettini/xtext-generics (in particular, the problem can be found in the branch 'reproduced_problem'). I discussed this issue with Sebastian at XtextCon, and he came up with the solution (that can be found in the above bitbucket example) and he asked me to prepare a patch based on that. The problem: Say that you have a type parameterized entity, and for such entity you infer: - a parameterized Java interface - a parameterized Java class where type parameters are copied. The current scoping implementation for type parameters (org.eclipse.xtext.xbase.scoping.XImportSectionNamespaceScopeProvider.getLocalElementsScope) is based on the type parameters of the associated Jvm elements; when you have more than one inferred element, this implementation will work only for one of the inferred element, while for the other ones the resolved cross links are of course invalid. (In https://github.com/LorenzoBettini/xtraitj I had solved the problem by manually rebind all type parameter references; of course, that is a hack, besides requiring a big effort :) The solution would be to implement such scoping directly for the inferred elements (of type JvmTypeParameterDeclarator); but this raises another problem of the current implementation: the context passed to the scope provider is the type reference of the original model, NOT the type reference of the Jvm model (which indeed triggered the cross reference resolution) due to the encoded URIs. The solution provided by Sebastian consists in - JvmTypesBuilder.cloneAndAssociate should modify cross reference proxy URIs so that they contain information about the copied type references; this way, the scope provider will get the right context (i.e., the type reference in the Jvm model, instead of the type reference of the original DSL model) - org.eclipse.xtext.xbase.scoping.XImportSectionNamespaceScopeProvider.getLocalElementsScope should first check whether the passed context is a Jvm model element and in case, use its type parameters. Connected to the same context, there's another small problem in the XbaseValidator.checkTypeParameterNotUsedInStaticContext which raises errors for type parameter references even though they are correctly resolved to the type parameters of the Jvm model. As I said, I'll prepare a pull request in the next few days. (https://bitbucket.org/lbettini/xtext-generics already contains an example of patched implementation).
Hi Lorenzo, it would be wrong to always change the proxy URIs on clone as you do in the example project, because generally you should never scope the same syntactic cross reference in two different ways. Imagine you would generate the class and the interface into two different namespaces. And copy the same type reference to both classes. If the scope for the type reference differs one could be broken while the other is valid. E.g: entity MyEntity { a : MyEntityImpl // broken in interface MyEntity b : MyEntity // broken in class MyEntityImpl } The case for TypeParameters is special. Although the scope is different because the actual TypeParameter is different, the names are the same. So it somehow does what the user expects in this case. In order to make this work for all TypeReferences, we need to also adjust the scoping such that all inferred types are visible by their simple name.
(In reply to Lorenzo Bettini from comment #0) > (In https://github.com/LorenzoBettini/xtraitj I had solved the problem by > manually rebind all type parameter references; of course, that is a hack, > besides requiring a big effort :) I don't think that is a hack. What you actually want is not the same type reference but a symmetric one, the current approach leverages the fact that the type parameters all have the same name. What really matters is the identity, so going over cross references afterwards and making sure they point to the right type parameter is IMHO better.
Hi Sven So do you think it would be better to provide some kind of "TypeParameterAwareCopier" that takes care of type parameters and performs this "rebinding" of type references while copying? I mean, if you think that it could be useful for others as well. Probably mine is a strange use case ;) but as soon as you want to infer even two different type parameterized Jvm operations from the same type parameterized model element the current implementation of generic types won't work. I'm also adding Sebastian to CC, since the original solution was his own, I only rearranged the code to submit the patch.
I discussed this issue with Sebastian this morning, and we came to the conclusion that a dedicated moveIntoContext(EObject container, JvmTypeReference typeRef) should be added, that has a lot of JavaDoc that basically explains in what situations you might consider using it and what the positive and negative effects are. It might make sense to have the general logic of changing the context EObject for lazy-linking URIs somewhere else than in JvmTypesBuilder.
Just to make sure I understand: should this new moveIntoContext replace proxy URIs (like the original proposal of this bug) or replace already resolved type parameter references (like in my DSL)?
It should change the proxy URI like in the original proposal. So clients need to make a copy first.
(In reply to Sven Efftinge from comment #6) > It should change the proxy URI like in the original proposal. > So clients need to make a copy first. Personally I think #copyIntoNewContext would be better since it would do both operations at once and reduce the chance for failures.
In an offline discussion :-) we decided to have the methods JvmTypesBuilder#cloneWithAdjustedProxies(JvmTypeReference) JvmTypesBuilder#cloneWithAdjustedProxies(JvmTypeParameter) which do the typical clone but changes the proxyURIs in getType() to point to the newly created JvmTypeReference (i.e. what you proposed).
OK, I'll try to work on that :) Should I keep working, for the moment, on https://bitbucket.org/lbettini/xtext-generics and only later port it to Xtext code base for a pull-request? With that respect, I think that the small DSL of https://bitbucket.org/lbettini/xtext-generics could be added as an Xbase test language, e.g., in the project org.eclipse.xtext.xbase.testlanguages and the corresponding tests to org.eclipse.xtext.xbase.tests; what do you think?
I'd be fine with that. It's usually a good idea to have dedicated test languages for bugs so if the lang doesn't do any other fancy things, that be nice.
Dear Sven and Sebastian I tried to implement what you suggested on this branch: https://bitbucket.org/lbettini/xtext-generics/branch/xtext-bug-468174 In particular, - https://bitbucket.org/lbettini/xtext-generics/src/d5f07aec654f1bbed6a6c92f16205f7cdcd3f305/org.xtext.example.generics/src/org/xtext/example/generics/jvmmodel/PatchedJvmTypesBuilder.java?at=xtext-bug-468174 where I extracted the EcoreUtil.Copier to avoid code duplication - and how cloneWithAdjustedProxies is used in the inferrer: https://bitbucket.org/lbettini/xtext-generics/src/d5f07aec654f1bbed6a6c92f16205f7cdcd3f305/org.xtext.example.generics/src/org/xtext/example/generics/jvmmodel/GenericsJvmModelInferrer.xtend?at=xtext-bug-468174 This solution is based on the following assumptions: - proxies are NOT resolved during the inferrer; this requires to implement in the inferrer a custom toGetter (see https://bugs.eclipse.org/bugs/show_bug.cgi?id=468641 and the discussion https://github.com/eclipse/xtext/pull/256 ) - cloneWithAdjustedProxies must be used everywhere in the inferrer, or some type parameter references will not be solved correctly; it seems to be enough to use it only when inferring the first JvmGenericType and its JvmMembers (i.e., in this example, when inferring the Java interface), but I think that it is better to use it consistently so that scoping for type parameter references will always use the adjusted proxies. (Recall that the XbaseValidator has to be tweaked as well - also in the original proposal anyway, https://bitbucket.org/lbettini/xtext-generics/src/d5f07aec654f1bbed6a6c92f16205f7cdcd3f305/org.xtext.example.generics/src/org/xtext/example/generics/validation/PatchedXbaseValidator.java?at=xtext-bug-468174). In general I don't like this solution much... what do you think? Would it make more sense to make the logic/strategy of cloning in the JvmTypesBuilder injectable (I mean the one used by the standard cloneWithProxies for JvmTypeReference) so that if one needs to adjust proxies then he can select the AdjustingProxiesCopier instead of the default one JvmTypeReferenceCopier (these are the class names I have already extracted) and then the inferrer can be written more easily (of course, the custom toGetter has still to be implemented)? If I understand correctly, as Sven stressed, it is crucial that proxies are NOT adjusted by default; but when one needs them to be adjusted then all type reference proxies must be adjusted, and selecting the cloning strategy is less error prone than carefully call cloneWithAdjustedProxies in all the places.
Scheduled for 2.9. Review pending.
