https://kotlinlang.org logo
#compiler
Title
# compiler
d

dmitriy.novozhilov

07/06/2022, 2:04 PM
Fork of previous thread
y

Youssef Shoaib [MOD]

07/06/2022, 2:07 PM
If that idea is a no-go, could we have a hook into `FirCallResolver`to provide our own `Candidate`s? I think it can achieve the same goal of turning red code green by providing a valid candidate. Or, how about a
doAnalysis
-style hook that just allows a plugin to go wild and resolve an expression from the start however it wants?
d

dmitriy.novozhilov

07/06/2022, 2:09 PM
Call can be completed with some outer call
Copy code
fun <K> materialize(): K = null!!
fun <T> id(x: T): T = x
fun takeInt(x: Int) {}

fun test() {
    takeInt(id(materialize()))
}
In this example
materialize
and
id
will be completed only during completion of call
takeInt
So if you change candidate (and, potentially, return type) of inner call (
materialize
) then it require to reanalyze all calls in chain, because with new type of argument some other functions for outer calls can be applicable
could we have a hook into
FirCallResolver
to provide our own
Candidate
This is also is a bad idea, because call resolution is already extremely complicated (did you read that part of specification?), and providing custom rules to it from plugins will blow everybody minds (compiler devs, plugin devs and regular users)
But you can provide new declarations with
FirDeclarationGenerationExtension
, why it's not enough for you?
Or, how about a
doAnalysis
-style hook that just allows a plugin to go wild and resolve an expression from the start however it wants?
With this approach Kotlin with such plugin won't be Kotlin anymore, because there are no guarantees that plugin will do everything according to specification
y

Youssef Shoaib [MOD]

07/06/2022, 2:55 PM
Well, let's make this concrete because I think maybe my use case just so happens to not maybe fall into some of those pitfalls. I'm trying to design a DI plugin that, whenever there's a missing context receiver, it provides it based on some annotated function. (I'm aware that
addNewImplicitReceivers
exists, and in fact that's what arrow-inject uses, but I wanted something that wouldn't require the user to explicitly say what receivers they want) From my observations, when there's multiple candidates that can work, except that both have some missing context receiver, I got a
ConeAmbiguityError
so already there isn't one candidate that can work, and so my plugin just helps choose one of them, while if there's only one candidate that can work (but is missing a context receiver), I got an
InapplicableCandidateError
, and so my plugin then doesn't change the candidate to a different function, but simply allows the candidate to be considered applicable because a new context receiver gets added into the tower data. In the case of the `materialize`example (which is actually quite apt because it is similar to a function I've already tested) the return type of materialize would either be determined by explicit type arguments, by the type that id expects, or if there's no other type hints at all, then by the context receiver it is missing (i.e. if materialize had a
context(MyWrapper<K>)
and there's only one `MyWrapper`that the plugin knows about with some concrete type). The return type of materialize then changes in 2 cases: • If it had no type hints, then now it has a return type, which I think is considered okay • If it knows what type it needed to be, then materialize's return type is either that required type or some subtype of it, but if it is a subtype then I'd and takeInt should still be satisfied (keep in mind that if
takeInt
had 2 overloads with one of them taking a subtype of the other then already it would be trying to choose which overload wins based on the argument) I hope that this makes some sense, hopefully. I'm on mobile rn so it's kinda difficult to provide actual code. Thank you, by the way, because this has helped open my eyes to some incorrect assumptions that I had about the compiler.
r

raulraja

07/06/2022, 5:19 PM
@dmitriy.novozhilov This is somewhat of the same use case we have in arrow-inject where we need to provide our own code to resolve and complete a call given a set of candidates. @Youssef Shoaib [MOD] we are doing something very similar or the same plugin at https://github.com/arrow-kt/arrow-inject/blob/e5a7be736eb01e5eab014d9b36eef2e61cf2[…]er/plugin/fir/resolution/resolver/ProofResolutionStageRunner.kt
y

Youssef Shoaib [MOD]

07/06/2022, 5:35 PM
@raulraja I'm really glad you saw this thread as I've been meaning to ask about arrow-inject. 1. How do you actually run your ProofResolutionStageRunner? I haven't looked around your codebase in an IDE so sorry if it's too obvious. I couldn't figure out how to run one myself, couldn't really find a hook anywhere 2. I'm basically trying to do the same general idea as arrow-inject, but without the need for
context<Foo<Int>>()
for instance (i.e. I want the context to be available without the user explicitly writing out the type so that e.g. in your polymorphic provider test I could call a function that expects a
Foo<Int>
in its context and that would be resolved without requiring a
context<Foo<Int>>()
right before the call). The reason behind this is that I want to extensively use contexts with generics without having to type out what generic type it needs (e.g. if I have functions that take `Foo<String> Foo<Int>`or even a
Foo<Foo<Foo<List<Any>>>>
I would like all of those function calls to be resolved without any explicit types).
r

raulraja

07/06/2022, 5:38 PM
I'm interested in seeing an example of what you mean, seems what you are doing is adding context receivers automatically where needed. At some point though you need to set what your instances for those types are and the final call needs nested
with
. We have replicated more or less what the ResolutionStageRunner does in the compiler to have the ability to resolve a fake call given a set of candidates.
Also overall I'm hoping Kotlin provides at some point a way to inject or make it easier to make concrete instances without a set of nested `run`or nested
with
y

Youssef Shoaib [MOD]

07/06/2022, 5:50 PM
Basically what I mean is this:
Copy code
@Provider internal fun intProvider(): Int = 42
@Provider internal fun stringProvider(): String = "hello world"
data class Foo<A>(val n: A)

context(A)
@Provider internal fun <A> fooProvider(): Foo<A> = Foo(this@A)

context(Foo<Int>) fun needsInt() = TODO()
context(Foo<String>) fun needsString() = TODO()

fun box(): String {
  needsInt() //in FIR, it'll have a call to fooProvider as its first contextReceiverArgument, and in turn that call to fooProvider has a call to intProvider as its first contextReceiverArgument (or could be done in IR, doesn't matter)
  needsString() // should have first CR argument as call to fooProvider, which has first CR argument as call to stringProvider 
}
I want that to compile with no errors, and on the two
needsX
calls, the context receiver should basically be a new Foo instance with the Int/String inside of it (which is what to be expected from the providers). With arrow-inject right now I'd need to add the apt calls of arrow's
context<>()
function, but that feels redundant to me.
r

raulraja

07/06/2022, 6:42 PM
I see, this is even more implicit but can work in the same way. We have in IR both the calls to
contextual
but also the error expression types resulting from
addNewImplicitReceivers
when the fir call has errors. https://github.com/arrow-kt/arrow-inject/blob/e5a7be736eb01e5eab014d9b36eef2e61cf2[…]/fir/resolution/contexts/ContextProvidersResolutionExtension.kt
Right now we are just looking at the type args of the
contextOf
function but could just look at the error types in resolution to look up the providers.
y

Youssef Shoaib [MOD]

07/06/2022, 7:03 PM
Yes but how would you then resolve those errors, as in, how would you change the callee reference then to not be an error-signalling one. What it sounds like is that FIR most likely won't support such an API. I guess you could just suppress all context-related errors, but that doesn't seem like a good idea to me. Especially because, you could have 2 overloads where only one is valid based on a provider e.g.:
Copy code
context(Int) fun foo() = TODO()
context(String) fun foo() = TODO()
@Provider internal fun intProvider(): Int = 42

fun main() {
  foo() // should run the one expecting Int
}
Suppressing the errors in this case won't make resolution run correctly here, and if there's no way to alter the callee reference (at least no officially supported API) then such a case won't be possible. I think that an API to support something like that should exist therefore.
r

raulraja

07/06/2022, 7:25 PM
Are you currently using
override fun addNewImplicitReceivers(functionCall: FirFunctionCall): List<ConeKotlinType>
? I'm speculating that when you add the expected types there then the function call should resolve fine because that is added prior to resolving calls.
Resolution then in our case proceeds fine to IR but it gets to IR with some error types that still need to be replaced for the proper nested with and expressions needed around the call https://github.com/arrow-kt/arrow-inject/blob/e5a7be736e/arrow-inject-compiler-plu[…]ow/inject/compiler/plugin/ir/ProofsIrContextReceiversCodegen.kt
y

Youssef Shoaib [MOD]

07/06/2022, 7:32 PM
addNewImplicitReceivers
is actually called after the call is completely resolved. Obviously that doesn't matter for arrow's case, but in my case I basically get the
BodyResolveComponents
using a hack (I register a custom
FirSessionComponent
like this to get access to all the Body Resolve Components then I filter for the one I want) and then inside
addNewImplicitReceivers
I figure out the missing context receiver types, find the right providers for them, add them to the towerDataContext, call the expressions transformer (which can be accessed thru BodyResolveComponents), and then remove them from towerDataContext. Finally I use replaceContextReceiverArguments to make them function calls instead (but I could use the arrow approach in IR instead, doesn't matter too much). And so, I need BodyResolveComponents inside
addNewImplicitReceivers
so that I can access the transformer (or alternatively, I could use the callResolver and callCompleter, but both of which are still inside BodyResolveComponents). I could create my own BodyResolveComponents, but they'll be missing all the surrounding receivers for instance since I won't have access to the real towerDataContext.
d

dmitriy.novozhilov

07/07/2022, 7:34 AM
You want to do really scary things. For me it looks like you are trying to make implicits from Scala from context receivers. And we put all our effort to not to do it during design of context, because implicits can easily transform code to a not understandable mess Generally we don't want to allow plugins to interfere with call resolution process (because of reasons I described above), but we can probably reconsider it if we have really good usecases which can not be achieved with existing API
r

raulraja

07/07/2022, 11:51 AM
In our case even when the injection is explicit to the type we want to inject we need a form of call resolution to select the right provider for the call. There can be more than one provided with compatible types due to polymorphism and generics. The most specific one has to be chosen. I think in general compile time DI requires implicit resolution and the plugins attempt to implement a form of DI. People will use DI whether we think it's a good idea or not because the places they are at. There is a fairly large amount of people on Android using Dagger. Dagger codegens and generates a lot of unnecessary code where it could all be done at compile time. Trying to leverage context receivers as much as we can we still have the need to resolve them at some point. I'm not familiar with compile time DI that does not look a bit like implicits at least in resolution / injection but happy to explores new ways if there is a better one than this.
d

dmitriy.novozhilov

07/07/2022, 12:48 PM
I also didn't explore compile time DI methods yet I plan to do it sometime in future, but definitely not in the nearest one
11 Views