Update some impl IR APIs to more modern alternatives #734
Conversation
Just a little cleanup to use more stable IR factory functions rather than direct impl types
| varargElementType = IrDynamicTypeImpl(null, emptyList(), INVARIANT), | ||
| elements = contributedModules | ||
| valueArgument = irVararg( | ||
| elementType = pluginContext.irBuiltIns.kClassClass.starProjectedType, |
There was a problem hiding this comment.
This is a change from before, but I think this is more correct (Array<KClass<*>>) as dynamic is only really intended for JS targets iirc
There was a problem hiding this comment.
What do you mean by this being a change from before?
There was a problem hiding this comment.
Before it was surprisingly the kotlin dynamic type, which I suspect was for lack of a better star alternative: https://kotlinlang.org/docs/dynamic-type.html
There was a problem hiding this comment.
The dynamic type is not supported in code targeting the JVM. - mildly concerning :D, apparently it works though 😆. Good catch.
| pluginContext.irBuiltIns.createIrBuilder(declaration.symbol) | ||
| .generateDaggerAnnotation( |
There was a problem hiding this comment.
Creating an IR builder exposes more factory extension functions, including some of the ones used below
There was a problem hiding this comment.
Do we need to add any followup build() equivalent calls for the IrBuilder? Seems like the answer is no but just wanted to verify
There was a problem hiding this comment.
Not as far as I know, it's more of a context thing than a true builder
| // These are always IrExpression instances too | ||
| values = varargArguments.flatMap { it.elements } | ||
| .filterIsInstance<IrExpression>() |
There was a problem hiding this comment.
Are we sure this is guaranteed? Wondering why the extension function enforces IrExpressions when the impl class doesn't
There was a problem hiding this comment.
Yeah if you look at the impl that's all it allows. My guess is it's a bit of a marker interface tomfoolery
| varargElementType = IrDynamicTypeImpl(null, emptyList(), INVARIANT), | ||
| elements = contributedModules | ||
| valueArgument = irVararg( | ||
| elementType = pluginContext.irBuiltIns.kClassClass.starProjectedType, |
There was a problem hiding this comment.
What do you mean by this being a change from before?
| pluginContext.irBuiltIns.createIrBuilder(declaration.symbol) | ||
| .generateDaggerAnnotation( |
There was a problem hiding this comment.
Do we need to add any followup build() equivalent calls for the IrBuilder? Seems like the answer is no but just wanted to verify
Just a little cleanup to use more stable IR factory functions rather than direct impl types