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

delay @ContributesSubcomponent generation until the last analysis rounds #946

Merged

Conversation

RBusarow
Copy link
Collaborator

@RBusarow RBusarow commented Apr 2, 2024

The ContributesSubcomponentHandlerGenerator is responsible for generating these:

@MergeSubcomponent(scope = Any::class)
public interface SubcomponentInterfaceA : SubcomponentInterface {
  // ...
}

It used to generate those interfaces as soon as it scanned the triggers, but that can cause problems if another @ContributesSubcomponent interface is being generated, and that intends to replace a human-written one.

Now, the handler is a PrivateCodeGenerator and it doesn't start parsing/generating until all the "public" code generation is done. This means it gets the full picture of the possible graph, but it still finishes generating before any IR merging happens.

…ounds

The `ContributesSubcomponentHandlerGenerator` is responsible for generating these:

```kotlin
@MergeSubcomponent(scope = Any::class)
public interface SubcomponentInterfaceA : SubcomponentInterface {
  // ...
}
```

It used to generate those interfaces as soon as it scanned the triggers,
but that can cause problems if another `@ContributesSubcomponent` interface is
being generated, and that intends to replace a human-written one.

Now, the handler is a `PrivateCodeGenerator` and it doesn't start parsing/generating until all the "public" code generation is done.  This means it gets the full picture of the possible graph, but it still finishes generating before any IR merging happens.
@RBusarow RBusarow force-pushed the rick/ContributesSubcomponentHandlerGenerator-private branch from e036426 to 4bc2151 Compare April 2, 2024 18:39
@@ -118,10 +118,8 @@ internal class CodeGenerationExtension(
anvilModule.addFiles(files)

val generatedFiles = generateCode(
codeGenerators = codeGenerators,
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just class-level, so no need to pass the variable around internally.

@RBusarow RBusarow marked this pull request as ready for review April 2, 2024 19:18
Copy link
Collaborator

@ZacSweers ZacSweers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm though I don't think this fixes the issue @gabrielittner was reporting as his was KSP-specific

@RBusarow RBusarow merged commit 95ade2c into main Apr 3, 2024
17 checks passed
@RBusarow RBusarow deleted the rick/ContributesSubcomponentHandlerGenerator-private branch April 3, 2024 05:01
@gabrielittner
Copy link
Contributor

though I don't think this fixes the issue @gabrielittner was reporting as his was KSP-specific

Yes, it's still happening with this change. The issue is that since there is no KSP implementation of ContributesSubcomponentHandlerGenerator the SubcomponentInterfaceA types won't be generated at all when enabling KSP mode.

@gabrielittner
Copy link
Contributor

@RBusarow This change seems to break custom code generators in embedded mode. https://github.com/freeletics/khonshu on branch anvil-snapshots if you run ./gradlew -p sample/simple assembleDebug there is no output from custom generators. The build does not fail but checking for example sample/simple/feature/main/implementation/build/anvil there should be a generated file called KhonshuMainScreen which doesn't exist. The branch is using the latest snapshot but I've locally tested that this change is the one that broke it (commit a3cf4876 is still working, 95ade2c1 is broken).

// parsed files until no new files are generated.
newFiles = nonPrivateCodeGenerators.generateCode(newFiles)
fun List<CodeGenerator>.loopGeneration() {
var newFiles = generateAndCache(anvilModule.allFiles.toList())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously generateAndCache (which was called generateCode before) was called once with files (like 258 of the removed code var newFiles = nonPrivateCodeGenerators.generateCode(files)). Always using anvilModule.allFiles.toList() can break custom code generators that use topLevelFunctionReferences or topLevelPropertyReferences because it won't contain any KtFile that doesn't contain a KtClassOrObject. So a KtFile that only contains top level functions and/or properties will not be passed to code generators anymore and because of that topLevel*References won't include functions/properties from those files.

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

Successfully merging this pull request may close these issues.

3 participants