Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement parameter adaptation in LambdaMetaFactory #1078

Merged

Conversation

jonmathews-ensoft
Copy link

This is based on work started in #701, and has been rebased on #1056 to incorporate additional fixes.

Summary:

  1. Implemented modeling of parameter adaptation. This is required to handle conversion of primitives to their wrapper types (boxing and unboxing).

  2. Enabled writing generated classes to another format, such as .class.

  3. Generated class names follow naming pattern based on the class enclosing the original invoke dynamic instruction.

  4. Removed BridgeMethodSource; ThunkMethodSource handles all cases. The implementation follows Better lambda support via modeling of LambdaMetaFactory #701: samMethodType and the bridges are signatures to be implemented by the functional object, and each one delegates to the implMethod. Bridge methods should only be necessary for the altMetaFactory, and in practice we have found no cases in the wild from the OpenJDK compiler (and only one from the Eclipse compiler).

  5. Although WIP: Synthetic LambdaMetaFactory implementation to handle invokeDynamic #1056 contains fixes for invocation style handling, this may add even more fixes (this was implemented prior to WIP: Synthetic LambdaMetaFactory implementation to handle invokeDynamic #1056, and I have not performed a detailed comparison).

Why these changes are needed:

The following class contains a method reference which requires a parameter adaptation from int to Integer. This conversion has to be implemented within the thunk class; it cannot be performed at the call site in parameterBoxing(). See the LambdaMetaFactory javadoc and JLS 5.1.7 for details.

public class Adapt1 {
	interface IntInterfaceParam {
		public void adapt(int p);
	}
	interface IntegerInterfaceParam {
		public void adapt(Integer p);
	}
	public void parameterBoxing(IntegerInterfaceParam I1param) {
		// JLS 5.1.7
		// int -> Integer
		IntInterfaceParam I0var = I1param::adapt;
		I0var.adapt(1);
	}
};

Testing:

For dynamic testing, we converted the JavaPoet library (which uses lambdas), and all JUnits continued to pass.

In addition, we wrote some tests which we converted to Jimple text format and visually inspected. These include enumerated cases for parameter adaptation (Adapt.java), and a method reference to an array constructor (ArrayConstructorReference.java). These may be worth turning into test cases, I have included them in the zip file.

Additional observations:

  1. Synchronization issues persist in WIP: Synthetic LambdaMetaFactory implementation to handle invokeDynamic #1056. Added synchronization to LambdaMetaFactory.makeLambdaHelper() as a workaround, but this is likely the wrong way to fix the race conditions. Two distinct stack traces are included in the zip file which result if the method is unsynchronized. Perhaps the data structures in Scene are not properly synchronized with respect to whatever operations the LambdaMetaFactory performs?

  2. Before integration, consider making this feature optional. When writing transformed class files, the invokedynamic instructions will have been replaced (by design). This might be undesirable for some use cases.

  3. The names of generated classes are potentially non-deterministic because a single counter is used to name the classes, and method conversion may occur in parallel. This issue need not block integration, but it might complicate creating test cases.

LMF-attachments.zip

Implemented modeling of parameter adaptation. This is required to
handle conversion of primitives to their wrapper types
(boxing and unboxing).

Enabled writing generated classes to another format, such as .class.

Generated class names follow an inner class naming pattern.

Removed BridgeMethodSource; ThunkMethodSource handles all cases.
The samMethodType and the bridges are
signatures to be implemented by the functional object,
and each one delegates to the implMethod.
@mbenz89
Copy link
Contributor

mbenz89 commented Dec 10, 2018

This looks very nice, I'll have a look in the following days!

@mbenz89
Copy link
Contributor

mbenz89 commented Dec 19, 2018

That synchronization issue you are talking about, can it be reproduced by reading-in and writing-out the JavaPoet library?

@mbenz89 mbenz89 merged commit 7dbba98 into soot-oss:GrammaTech-develop Dec 20, 2018
@mbenz89
Copy link
Contributor

mbenz89 commented Dec 20, 2018

Thanks for your contribution!

Would be great if you tell me how to reproduce the stated exceptions still.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants