Skip to content

Commit

Permalink
Fixing Kotlin ABI issues (facebook#2596)
Browse files Browse the repository at this point in the history
* Fixing source abi gen for Kotlin

Summary:
Adding @JvmStatic support for kotlin methods to class abi generation

Adding kotlin awareness to class abi generation

BREAKGLASS - no metadata in buck repo.

Test Plan: Built Android monorepo

Reviewers: naveen.narayanan

Reviewed By: naveen.narayanan

Revert Plan: git revert

Differential Revision: https://code.uberinternal.com/D5734339

* Fixing Kotlin ABI test to reflect synthetic
  • Loading branch information
tyvsmith authored Mar 29, 2021
1 parent 7449238 commit 78f3d37
Show file tree
Hide file tree
Showing 6 changed files with 27 additions and 18 deletions.
11 changes: 5 additions & 6 deletions src/com/facebook/buck/jvm/java/abi/AbiFilteringClassVisitor.java
Original file line number Diff line number Diff line change
Expand Up @@ -46,18 +46,17 @@ class AbiFilteringClassVisitor extends ClassVisitor {
private boolean hasVisibleConstructor = false;
private Set<String> includedInnerClasses = new HashSet<>();
private List<String> nestMembers = new ArrayList<>();

public AbiFilteringClassVisitor(ClassVisitor cv, List<String> methodsWithRetainedBody) {
this(cv, methodsWithRetainedBody, null);
}
private final boolean isKotlinClass;

public AbiFilteringClassVisitor(
ClassVisitor cv,
List<String> methodsWithRetainedBody,
@Nullable Set<String> referencedClassNames) {
@Nullable Set<String> referencedClassNames,
boolean isKotlinClass) {
super(Opcodes.ASM7, cv);
this.methodsWithRetainedBody = methodsWithRetainedBody;
this.referencedClassNames = referencedClassNames;
this.isKotlinClass = isKotlinClass;
}

@Override
Expand Down Expand Up @@ -213,7 +212,7 @@ private boolean shouldInclude(int access) {
return false;
}

return (access & (Opcodes.ACC_SYNTHETIC | Opcodes.ACC_BRIDGE)) != Opcodes.ACC_SYNTHETIC;
return (isKotlinClass || (access & (Opcodes.ACC_SYNTHETIC | Opcodes.ACC_BRIDGE)) != Opcodes.ACC_SYNTHETIC);
}

private boolean isInterface(int access) {
Expand Down
13 changes: 8 additions & 5 deletions src/com/facebook/buck/jvm/java/abi/StubJarClassEntry.java
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ class StubJarClassEntry extends StubJarEntry {
private final Path path;
private final ClassNode stub;
private final boolean retainEverything;
private final boolean isKotlinClass;

@Nullable
public static StubJarClassEntry of(
Expand All @@ -70,7 +71,7 @@ public static StubJarClassEntry of(
// used within an inline function, and in these cases we need to retain the whole class.
input.visitClass(path, stub, false);
return new StubJarClassEntry(
path, stub, Collections.emptySet(), Collections.emptyList(), true);
path, stub, Collections.emptySet(), Collections.emptyList(), true, isKotlinClass);
}
ClassNode dummyStub = new ClassNode(Opcodes.ASM7);
input.visitClass(path, dummyStub, true);
Expand All @@ -94,7 +95,7 @@ public static StubJarClassEntry of(
// ABI methods and fields, and will use that information later to filter the InnerClasses table.
ClassReferenceTracker referenceTracker = new ClassReferenceTracker(stub);
ClassVisitor firstLevelFiltering =
new AbiFilteringClassVisitor(referenceTracker, methodBodiesToRetain);
new AbiFilteringClassVisitor(referenceTracker, methodBodiesToRetain, null, isKotlinClass);

// If we want ABIs that are compatible with those generated from source, we add a visitor
// at the very start of the chain which transforms the event stream coming out of `ClassNode`
Expand All @@ -112,7 +113,7 @@ public static StubJarClassEntry of(
|| retainAllMethodBodies
|| stub.name.endsWith("/package-info")) {
return new StubJarClassEntry(
path, stub, referenceTracker.getReferencedClassNames(), methodBodiesToRetain, false);
path, stub, referenceTracker.getReferencedClassNames(), methodBodiesToRetain, false, isKotlinClass);
}

return null;
Expand All @@ -123,12 +124,14 @@ private StubJarClassEntry(
ClassNode stub,
Set<String> referencedClassNames,
List<String> methodBodiesToRetain,
boolean retainEverything) {
boolean retainEverything,
boolean isKotlinClass) {
this.path = path;
this.stub = stub;
this.referencedClassNames = referencedClassNames;
this.methodBodiesToRetain = methodBodiesToRetain;
this.retainEverything = retainEverything;
this.isKotlinClass = isKotlinClass;
}

@Override
Expand All @@ -146,7 +149,7 @@ private InputStream openInputStream() {
ClassVisitor visitor = writer;
if (!retainEverything) {
visitor = new InnerClassSortingClassVisitor(stub.name, visitor);
visitor = new AbiFilteringClassVisitor(visitor, methodBodiesToRetain, referencedClassNames);
visitor = new AbiFilteringClassVisitor(visitor, methodBodiesToRetain, referencedClassNames, isKotlinClass);
}

stub.accept(visitor);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ public CompileToJarStepFactory configure(
ToolchainProvider toolchainProvider) {
CoreArg kotlinArgs = Objects.requireNonNull((CoreArg) args);
Path pathToAbiGenerationPluginJar =
shouldGenerateSourceAbi() ? kotlinBuckConfig.getPathToAbiGenerationPluginJar() : null;
shouldGenerateSourceAbi(kotlinArgs, kotlinBuckConfig) ? kotlinBuckConfig.getPathToAbiGenerationPluginJar() : null;
return new KotlincToJarStepFactory(
kotlinBuckConfig.getKotlinc(),
kotlinBuckConfig.getKotlinHomeLibraries(),
Expand Down Expand Up @@ -142,6 +142,10 @@ public boolean sourceAbiCopiesFromLibraryTargetOutput() {
return true;
}

private static boolean shouldGenerateSourceAbi(CoreArg kotlinArgs, KotlinBuckConfig kotlinBuckConfig) {
return kotlinArgs.getAbiGenerationMode().orElse(kotlinBuckConfig.getAbiGenerationMode()).isSourceAbi();
}

private static ImmutableList<SourcePath> getFriendSourcePaths(
BuildRuleResolver buildRuleResolver,
ImmutableSortedSet<BuildTarget> friendPaths,
Expand Down
4 changes: 2 additions & 2 deletions src/com/facebook/buck/jvm/kotlin/KotlincToJarStepFactory.java
Original file line number Diff line number Diff line change
Expand Up @@ -294,9 +294,9 @@ public void createCompileStep(
.add(MODULE_NAME)
.add(moduleName);

Path tmpSourceAbiFolder;
if (abiGenerationPlugin != null) {
tmpSourceAbiFolder = JavaAbis.getTmpGenPathForSourceAbi(projectFilesystem, invokingRule);
Path tmpSourceAbiFolder = JavaAbis.getTmpGenPathForSourceAbi(projectFilesystem, invokingRule);
addCreateFolderStep(steps, projectFilesystem, buildContext, tmpSourceAbiFolder);
extraArguments.add("-Xplugin=" + abiGenerationPlugin);
extraArguments.add(
"-P", "plugin:org.jetbrains.kotlin.jvm.abi:outputDir=" + tmpSourceAbiFolder);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ public class AbiFilteringClassVisitorTest {
public void setUp() {
mockVisitor = createMock(ClassVisitor.class);
filteringVisitor =
new AbiFilteringClassVisitor(mockVisitor, ImmutableList.of(), ImmutableSet.of());
new AbiFilteringClassVisitor(mockVisitor, ImmutableList.of(), ImmutableSet.of(), false);
}

@Test
Expand Down Expand Up @@ -92,7 +92,7 @@ public void testExcludesPrivateMethods() {
@Test
public void testIncludesPrivateMethodsWhenRetained() {
filteringVisitor =
new AbiFilteringClassVisitor(mockVisitor, ImmutableList.of("foo"), ImmutableSet.of());
new AbiFilteringClassVisitor(mockVisitor, ImmutableList.of("foo"), ImmutableSet.of(), false);
testIncludesMethodWithAccess(Opcodes.ACC_PRIVATE);
}

Expand Down Expand Up @@ -166,7 +166,7 @@ public void testIncludesInnerClassEntryForInnerClass() {
@Test
public void testIncludesInnerClassEntryForReferencedOtherClassInnerClass() {
filteringVisitor =
new AbiFilteringClassVisitor(mockVisitor, ImmutableList.of(), ImmutableSet.of("Bar$Inner"));
new AbiFilteringClassVisitor(mockVisitor, ImmutableList.of(), ImmutableSet.of("Bar$Inner"), false);

visitClass(mockVisitor, "Foo");
mockVisitor.visitInnerClass("Bar$Inner", "Bar", "Inner", Opcodes.ACC_PUBLIC);
Expand Down
5 changes: 4 additions & 1 deletion test/com/facebook/buck/jvm/java/abi/StubJarTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -1612,7 +1612,10 @@ public void kotlinClassWithInlineExtensionMethodThatUsesRunnable() throws IOExce
"",
" // access flags 0x0",
" <init>(Lkotlin/jvm/functions/Function0;)V",
"}")
"",
" // access flags 0x1011",
" public final synthetic run()V",
"}")
.createAndCheckStubJar();
}

Expand Down

0 comments on commit 78f3d37

Please sign in to comment.