Named argument constructor doesn't work for inner classes

Attaching a patch for review.

I somehow remember a problem with Class#getEnclosingClass. Some of these methods are working only if the class file bytecode version is really 1.5 or higher. Our default is still 1.4. But since your example is not using 1.5 code and is still working that might not be the case here. Otherwise the patch looks just fine to me

1) My understanding was that Groovy version >= 1.7, we only generate V1_5 bytecode (major version = 49) and that's why we don't have JDK 1.4 builds anymore for these Groovy versions. Is that incorrect?

2) Even if our intention is to generate JDK 1.4-compliant code based on features (annotations, generics) being used in the Groovy code, I don't think it will ever do it in practice. The reason being that ResolveVisitor.resolveGenericsTypes() unconditionally sets usesGenerics = true on every classnode it processes. Due to that the bytecode being generated for class Outer in the example code of this issue is still V1_5 and not V1_3.

Am I misreading something?

I am quite sure we don't produce V1_5 always. But anyway, we should make it the default for all branches

So, r u saying that I can assume it is ok to assume V1_5 is default and apply the patch?

No... I am saying we have to ensure 1.5 is the default first.

To me it looks like we only produce V1_5 bytecode anyway. Ensuring that we only produce V1_5 code should be simple by having AsmClassGenerator.getBytecodeVersion() always return Opcodes.V1_5.

The real questions are:

a) are we we clear that it's no longer our intention to try and produce V1_3 bytecode (as was the confusion in an earlier comment here).

b) your doubt that we don't always produce V1_5 bytecode as of now even in newer Groovy versions (>= 1.7). When is that, and why is that? If it is really needed, then it's not my call to always ensure V1_5 code.

(a) I spoke with Guillaume about that and we agreed that we should migrate to V1_5.
(b) If you want to see an example of when we produce V1_3 code I suggest you take class X { class Y {}} and look at the generated bytecode. I see there the usage of class version 47, which is V1_3. I would have said we don't need to go that far and can take the example above, but it actually shows that there is a very uneven handling of these things. Outer get the class version 49, Outer.Inner 47. Since we need 49 on Inner there is still the danger of your patch not working. Anayway... I think the point that 49 is not always used is clear now.

As for AsmClassGenerator.getBytecodeVersion().. I guess we can change it to always return V1_5 for now, but I think in the long term we have to adjust the logic with CompilerConfiguration as well.

True. Misread ResolveVisitor.resolveGenericsTypes()'s "if (types == null) return". Sorry.

So, I will change the bytecode version in ASM to V1_5 on 1.8.x and master and then apply this patch. If it becomes clear where adjustments need to be made to CompilerConfiguration, will do that as well.

Thanks.

I am wondering if we should push that to 1.7 as well

Not sure if it makes sense to remove support for V1_3 after 1.7."10".

I would rather 1.7.11 also has the support, but then this fix and ensure-V1_5 go hand in hand. If we want to push this feature to 1.7.x line, we will need to remove support for V1_3 in whatever shape it exists currently.

I think you are right. Btw, thanks for working on this Roshan