Double-check bytecode differences due to #3191

[stephan-herrmann]

Y-builds show a difference between class files as reported in eclipse-platform/eclipse.platform.releng.aggregator#2793 (comment)

I reduced one of the affected classes to this:

interface Provider<T> {
   T get();
}
interface IObjectDescriptor {}
interface IInjector {}
class PrimaryObjectSupplier {}
class ProviderImpl<T> implements Provider<T> {
	public ProviderImpl(IObjectDescriptor descriptor, IInjector injector, PrimaryObjectSupplier provider) {
	}
	@Override
	public T get() {
		return null;
	}
}

public class AnnotationLookup {
	@FunctionalInterface
	private interface ProviderFactory {
		Object create(IObjectDescriptor descriptor, IInjector injector, PrimaryObjectSupplier provider);
	}
	
	private static final ProviderFactory PROVIDER_FACTORY;
	static {
		ProviderFactory factory;
		try {
			class JavaxCompatibilityProviderImpl<T> extends ProviderImpl<T> implements Provider<T> {
				public JavaxCompatibilityProviderImpl(IObjectDescriptor descriptor, IInjector injector,
						PrimaryObjectSupplier provider) {
					super(descriptor, injector, provider);
				}
			}
			factory = JavaxCompatibilityProviderImpl::new;
			factory.create(null, null, null);
		} catch (NoClassDefFoundError e) {
			factory = ProviderImpl::new;
		}
		PROVIDER_FACTORY = factory;
	}
}

In master the constructor reference ProviderImpl::new creates these bytes:

         0: invokedynamic #10,  0             // InvokeDynamic #0:create:()LAnnotationLookup$ProviderFactory;
...
  private static java.lang.Object lambda$2(IObjectDescriptor, IInjector, PrimaryObjectSupplier);
    descriptor: (LIObjectDescriptor;LIInjector;LPrimaryObjectSupplier;)Ljava/lang/Object;
    flags: (0x100a) ACC_PRIVATE, ACC_STATIC, ACC_SYNTHETIC
    Code:
      stack=5, locals=3, args_size=3
         0: new           #30                 // class AnnotationLookup$1JavaxCompatibilityProviderImpl
         3: dup
         4: aload_0
         5: aload_1
         6: aload_2
         7: invokespecial #32                 // Method AnnotationLookup$1JavaxCompatibilityProviderImpl."<init>":(LIObjectDescriptor;LIInjector;LPrimaryObjectSupplier;)V
        10: areturn
      LineNumberTable:
        line 39: 0
}
...
BootstrapMethods:
  0: #44 REF_invokeStatic java/lang/invoke/LambdaMetafactory.metafactory:(Ljava/lang/invoke/MethodHandles$Lookup;Ljava/lang/String;Ljava/lang/invoke/MethodType;Ljava/lang/invoke/MethodType;Ljava/lang/invoke/MethodHandle;Ljava/lang/invoke/MethodType;)Ljava/lang/invoke/CallSite;
    Method arguments:
      #45 (LIObjectDescriptor;LIInjector;LPrimaryObjectSupplier;)Ljava/lang/Object;
      #48 REF_invokeStatic AnnotationLookup.lambda$2:(LIObjectDescriptor;LIInjector;LPrimaryObjectSupplier;)Ljava/lang/Object;
      #49 (LIObjectDescriptor;LIInjector;LPrimaryObjectSupplier;)Ljava/lang/Object;

Whereas in JAVA_BETA24 we no longer create the lambda method but directly emit:

         0: invokedynamic #10,  0             // InvokeDynamic #0:create:()LAnnotationLookup$ProviderFactory;
...
  0: #38 REF_invokeStatic java/lang/invoke/LambdaMetafactory.metafactory:(Ljava/lang/invoke/MethodHandles$Lookup;Ljava/lang/String;Ljava/lang/invoke/MethodType;Ljava/lang/invoke/MethodType;Ljava/lang/invoke/MethodHandle;Ljava/lang/invoke/MethodType;)Ljava/lang/invoke/CallSite;
    Method arguments:
      #39 (LIObjectDescriptor;LIInjector;LPrimaryObjectSupplier;)Ljava/lang/Object;
      #45 REF_newInvokeSpecial AnnotationLookup$1JavaxCompatibilityProviderImpl."<init>":(LIObjectDescriptor;LIInjector;LPrimaryObjectSupplier;)V
      #46 (LIObjectDescriptor;LIInjector;LPrimaryObjectSupplier;)Ljava/lang/Object;

The change is brought in by #3198

[stephan-herrmann]

@srikanth-sankaran do you have a minute to double-check my understanding?

Class JavaxCompatibilityProviderImpl is a local class in a static context. Despite the staticness, the implementation in master marks the constructor reference with tagAsAccessingEnclosingInstanceStateOf(). If this outer instance would indeed be passed into instantiation, then a lambda method might be justified. But in this case the lambda method does nothing but passing through all arguments it received.

Since in BETA_JAVA24 we avoid tagAsAccessingEnclosingInstanceStateOf() codegen concludes that it suffices to pass REF_newInvokeSpecial ... into the bootstrap method. No indirection via a lambda method needed.

Is this sound reasoning?

[stephan-herrmann]

The second class with differences can be reduced to

import java.util.stream.Stream;

interface IApiComponent {}
public class UtilTest {
//	String name = "UtilTest";
	public void testRegexExcludeList() {
		
		class LocalApiComponent implements IApiComponent {
			public LocalApiComponent(String symbolicName) {
//				System.out.println(name+":"+symbolicName);
			}
		}
		IApiComponent[] components = Stream.of("str1", "str2")
			.map(LocalApiComponent::new).toArray(IApiComponent[]::new);
	}
	
	public static void main(String... args) {
		new UtilTest().testRegexExcludeList();
	}
}

Now the context of the local class is no longer static. For a moment I figured that this would break once we enable the lines in comments requiring access to the enclosing instance. But even without the lambda method, the enclosing instance is passed into the constructor alright. The need to do so is detected using nestedType.syntheticEnclosingInstanceTypes().

Here's the relevant byte code portion when compiled with BETA_JAVA24:

        17: aload_0
        18: invokedynamic #29,  0             // InvokeDynamic #0:apply:(LUtilTest;)Ljava/util/function/Function;
...
BootstrapMethods:
  0: #64 REF_invokeStatic java/lang/invoke/LambdaMetafactory.metafactory:(Ljava/lang/invoke/MethodHandles$Lookup;Ljava/lang/String;Ljava/lang/invoke/MethodType;Ljava/lang/invoke/MethodType;Ljava/lang/invoke/MethodHandle;Ljava/lang/invoke/MethodType;)Ljava/lang/invoke/CallSite;
    Method arguments:
      #66 (Ljava/lang/Object;)Ljava/lang/Object;
      #72 REF_newInvokeSpecial UtilTest$1LocalApiComponent."<init>":(LUtilTest;Ljava/lang/String;)V
      #74 (Ljava/lang/String;)LUtilTest$1LocalApiComponent;

In both examples javac does create a lambda method, but these don't seem to add any value.

[stephan-herrmann]

The actual classes can be found here:

• https://github.com/eclipse-platform/eclipse.platform/blob/master/runtime/bundles/org.eclipse.e4.core.di/src/org/eclipse/e4/core/internal/di/AnnotationLookup.java
• https://github.com/eclipse-pde/eclipse.pde/blob/master/apitools/org.eclipse.pde.api.tools.tests/src/org/eclipse/pde/api/tools/util/tests/UtilTests.java

[srikanth-sankaran]

@srikanth-sankaran do you have a minute to double-check my understanding?

Class JavaxCompatibilityProviderImpl is a local class in a static context. Despite the staticness, the implementation in master marks the constructor reference with tagAsAccessingEnclosingInstanceStateOf(). If this outer instance would indeed be passed into instantiation, then a lambda method might be justified. But in this case the lambda method does nothing but passing through all arguments it received.

Since in BETA_JAVA24 we avoid tagAsAccessingEnclosingInstanceStateOf() codegen concludes that it suffices to pass REF_newInvokeSpecial ... into the bootstrap method. No indirection via a lambda method needed.

Is this sound reasoning?

Sounds reasonable to me. However this has been a frustrating area in my experience to exactly and consistently pin down things. I have seen some back and forth in both compilers - no ready references to all tickets unfortunately but here is one:
#1590

[stephan-herrmann]

Sounds reasonable to me. However this has been a frustrating area in my experience to exactly and consistently pin down things. I have seen some back and forth in both compilers - no ready references to all tickets unfortunately but here is one:
#1590

I boldly take this as approval :) -- in particular because my question was not about error reporting, but about code gen - although of course both parts rely on some common logic in the compiler.