Implementation of lambda merging for Kotlin lambda classes #257
Conversation
| - uses: actions/[email protected] | ||
| with: | ||
| repository: heckej/proguard-core | ||
| path: proguard-core |
There was a problem hiding this comment.
| - uses: actions/[email protected] | |
| with: | |
| repository: heckej/proguard-core | |
| path: proguard-core |
| @@ -20,5 +20,6 @@ include 'gui' | |||
| include 'gradle-plugin' | |||
| include 'ant' | |||
| include 'annotations' | |||
| includeBuild "../proguard-core" | |||
There was a problem hiding this comment.
| includeBuild "../proguard-core" |
| @@ -66,6 +66,10 @@ | |||
| public static final String ASSUME_VALUES_OPTION = "-assumevalues"; | |||
| public static final String ALLOW_ACCESS_MODIFICATION_OPTION = "-allowaccessmodification"; | |||
| public static final String MERGE_INTERFACES_AGGRESSIVELY_OPTION = "-mergeinterfacesaggressively"; | |||
| public static final String PRINT_LAMBDAGROUP_MAPPING_OPTION = "-printlambdagroupmapping"; | |||
| public static final String MERGE_KOTLIN_LAMBDA_CLASSES_OPTION = "-mergekotlinlambdaclasses"; | |||
There was a problem hiding this comment.
Can you add the lambda merging to the -optimizations option instead, that will be more consistent e.g.
-optimizations class/merging/kotlinlambda
base/src/main/java/proguard/optimize/kotlin/KotlinLambdaMerger.java
Outdated
Show resolved
Hide resolved
base/src/main/java/proguard/optimize/kotlin/KotlinLambdaMerger.java
Outdated
Show resolved
Hide resolved
base/src/main/java/proguard/optimize/kotlin/KotlinLambdaMerger.java
Outdated
Show resolved
Hide resolved
base/src/main/java/proguard/optimize/kotlin/KotlinLambdaMerger.java
Outdated
Show resolved
Hide resolved
| { | ||
| return true; | ||
| } | ||
| MethodReferenceFinder methodReferenceFinder = new MethodReferenceFinder(invokeMethod); |
There was a problem hiding this comment.
Can we use ReferencedMemberVisitor here?
Maybe something like:
new ReferencedMemberVisitor(
new MemberVisitor()
{
@Override
public void visitAnyMember(Clazz clazz, Member member)
{
if (member == myMember) member.accept(clazz, counter);
}
}
);
Classes involved are: InnerClasssInfoClassConstantVisitor, ClassConstantReferenceUpdater, FieldReferenceFinder, FieldRenamer, MethodCopier, MethodReferenceFinder and PackageGrouper
Co-authored-by: James Hamilton <[email protected]>
|
Kudos, SonarCloud Quality Gate passed!
|
|
Hi, what's the status of this pull request? |
|
👀 |
|
Since this pull request was made, a lot of changes have been made to the internals of ProGuard and ProGuard CORE, if I'm correct. I currently do not have the time to do the required changes to make the code compatible. Besides, this is mainly a proof of concept, developed for my thesis, which would benefit a lot from qualitative improvements or perhaps a new start almost from scratch. |
|
I raised the issue upstream in Kotlin to get some attention to edge cases that generate class-based lambdas instead of the new |
This will add the ability to ProGuard to apply an optimisation called lambda merging which combines the implementations of multiple Kotlin lambda classes in a common class, called a lambda group. This pull request depends on a pull request in ProGuard-CORE: Classes needed for lambda merging in ProGuard #36.