Restructure binding module code gen #877
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Duplicated here since we already have it on the ContributesTo annotation
ZacSweers
changed the title
ZacSweers
changed the title
RBusarow
reviewed
Mar 8, 2024
Comment on lines
+42
to
+46
| val suffix = "As" + | ||
| contribution.boundType.generateClassNameString().capitalize() + | ||
| "To" + | ||
| contribution.scope.generateClassNameString().capitalize() + | ||
| bindingModuleNameSuffix |
There was a problem hiding this comment.
Could we break this up with some underscores?
There was a problem hiding this comment.
Yeah we could I was less worried about readability given this is never accessed directly by users and trying to minimize chars to avoid too-long class names
This was referenced Mar 11, 2024
RBusarow
added a commit
that referenced
this pull request
Mar 12, 2024
If we make any changes while syncing the local state with our internal cache, that will affect the source files and PSI parsing, but it doesn't affect the state represented by the ModuleDescriptor. In order to get things in sync, we need to return a `RetryWithAdditionalRoots`. While doing that analysis, the compiler will also sync its own files -- which means it will delete any `.class` files that are no longer current. See discussion here: #877 (comment)
RBusarow
added a commit
that referenced
this pull request
Mar 12, 2024
If we make any changes while syncing the local state with our internal cache, that will affect the source files and PSI parsing, but it doesn't affect the state represented by the ModuleDescriptor. In order to get things in sync, we need to return a `RetryWithAdditionalRoots`. While doing that analysis, the compiler will also sync its own files -- which means it will delete any `.class` files that are no longer current. See discussion here: #877 (comment)
RBusarow
approved these changes
Mar 12, 2024
|
@RBusarow wanna take a look at the updated 1:1 impl before I merge? |
15 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR reimplements binding module generation to perform all merging at the IR level and eliminate intermediate merged binding modules.
At a high level, this does the following
@ContributesBindingand@ContributesMultiBindingto instead generate binding modules per-binding.@InternalBindingMarkerannotation that contains a minimal amount of metadataoriginClass- the origin class that contributed the bindingisMultibinding- indicates if the binding is a multibindingpriority- corresponds toContributesBinding.Priority.qualifierKey- the computed "qualifier key"@ContributesToand propagated via that infrastructure.ModuleMergerIr, these binding modules are merged and metadata read from@InternalBindingMarkerto properly allow forexcludes,replaces, priority, etc all work correctly.This also functionally completes #751!
ModuleMergerIr+ new modeling using@InternalBindingMarkerModuleMergerIr. This is a little tricky becauseBindingModuleMergerspeaks classes butModuleMergerIrspeaks modules.