Hey ho, i recently tested the "given" construct (h...

# arrow-meta

[Hanno]

Hey ho, i recently tested the "given" construct (however it is called now) with meta compiler plugin, which is SO nice, this is so much what kotlin needs for type extensions :)
However, i found sth i am not sure whether i misunderstood it or not. When i have an interface over a type T and two implementations as top level object, it always resolves to the first declared instance, even though the generic type should prevent that. Is this expected? I could give my example when it's not clear what i mean.

[raulraja]

Hi Hanes, can you provide an example? Also if you want feel free to add a test case directly in the compiler-plugin module for your use case as a PR and we can include it in the suite if it turns out to be a bug

[Hanno]

I can create a ticket and attach a pr If you want but as i said i even don't know whether i am Holding it wrong :) this would be my usecase. https://github.com/hannespernpeintner/arrow-meta/commit/1c466538731510a7249b0a69a9a47bd6954f6fdc

[raulraja]

this is indeed a bug where it may find two candidates and it’s not choosing the most concrete or ascribed one as in your case. Would you mind creating an issue and assigning to @Imran/Malic. @Imran/Malic is currently working on resolution and this is the same area. Thanks!

[Imran/Malic]

Hi @Hanno the Proof resolution is in an incomplete state as of now there is a PR one can follow https://github.com/arrow-kt/arrow-meta/pull/683 and the behaviour, which your observing is correct as in master, but the following tests cover how the resolution should be behaving, there will be more tests as the PR matures to a more complete state in the compiler.
I am currently working on fixing some compiler error’s for type-proofs in 1.4.0. Once, that is done I’ll focus on finishing that PR.

[raulraja]

@Imran/Malic does @Hanno test already pass on your branch?

[Imran/Malic]

Nop, that part isn’t implemented, yet.
The current implementation searches for all applicable Proofs and doesn’t narrow it down to the most specific one - from there it picks the first in the list.
Thanks @Hanno for raising this. A PR to that file with the other tests is highly welcomed, the branch is called is-proof-resolution

[raulraja]

There should be a function somewhere called chooseMostSpecificCandidate or similar if I recall correctly.

[Imran/Malic]

Yes that is correct, that would fix it.

[Hanno]

I invested some time here.

The choseMostSpecific method is used already but only when collectAllCandidates is false, which it never is. I don't know why this is cobfigurable but setting it to false breaks nearly all tests :) from my point of view the Resolution of candidates is already wrong because type arguments are replaced with star projections in includeInCandidates in ProofResolution.kt . Removing it makes my test and all the given tests green, but i doubt that star projection was added for fun, so i doubt this is the solution... It breaks other tests as Well.

I was only able to use my given test, i tried to write a coercion test similar to the ones in the file you linked but i get compile errors for that one instead of resolution failure :(

[raulraja]

This code originally came from the compiler and start projections where needed to resolve generic args back then but it may have changed and we may be able to rewire that to always get the most specific candidate or at least the list sorted in that order so it selects the right one. Maybe star projections are not needed any more.

[raulraja]

https://github.com/arrow-kt/arrow-meta/blob/master/compiler-plugin/src/main/kotlin/arrow/meta/plugins/proofs/phases/ir/ProofsIrCodegen.kt#L71

[raulraja]

This is where the hack is to just get the first out of the matched ones

[raulraja]

most likely the other instance is in the list as well but discarded. The minimal fix is to ensure the list is ordered by most specific. The star projections let it pass through and that is why there is multiple candidates.

[Imran/Malic]

Yes maybe you have to declare your coercion proof as internal, when your using 3rd party Types

[raulraja]

that need to be fixed regardless

[raulraja]

because selection needs to happen both in resolution and codegen

[raulraja]

@Imran/Malic

Yes maybe you have to declare your coercion proof as internal, when your using 3rd party Types

That is correct but the compiler should have bailed in this case telling @Hanno to declare it internal if it refered to an external type or bailed on ambiguity if it refered to a type they own.

[raulraja]

And IR still needs to resolve it again properly to the selected one if it compiled and gets to codegen. We are gonna have the same issue for this case on the arrow migration so it's something we need to address now since the migration will start next week now that 1.4 is in core.

[Hanno]

Jap, the candidates are all there, i wasnt able to figure out how to sort them by specificness because the context is different from the Resolution context, i think the list contains IrCalls. I realized that removing star projections alone wouldn't help, so ordering is necessary because instead of float and int i could also have number and int. So we need either ordering or ordering and No star projections :)