3

In the following code, where MyMap trivially implements Map by delegation to impl:

foo@host:/tmp$ cat Foo.kt
class MyMap <K, V> (val impl : Map <K, V>) : Map<K, V> by impl {
  fun myGetValue (k: K) = impl.getValue(k)
}

fun main() {
  val my_map = MyMap(mapOf('a' to 1, 'b' to 2).withDefault { 42 })
  println(my_map.myGetValue('c'))  // OK
  println(my_map.getValue('c'))    // ERROR
}

Why do I get the following error on the second println?

foo@host:/tmp$ /path/to/kotlinc Foo.kt
foo@host:/tmp$ /path/to/kotlin FooKt
42
Exception in thread "main" java.util.NoSuchElementException: Key c is missing in the map.
        at kotlin.collections.MapsKt__MapWithDefaultKt.getOrImplicitDefaultNullable(MapWithDefault.kt:24)
        at kotlin.collections.MapsKt__MapsKt.getValue(Maps.kt:344)
        at FooKt.main(Foo.kt:8)
        at FooKt.main(Foo.kt)
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
        at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.base/java.lang.reflect.Method.invoke(Method.java:568)
        at org.jetbrains.kotlin.runner.AbstractRunner.run(runners.kt:64)
        at org.jetbrains.kotlin.runner.Main.run(Main.kt:176)
        at org.jetbrains.kotlin.runner.Main.main(Main.kt:186)
foo@bigdev:/tmp$

Update: The compiler and runtime version outputs are:

foo@host:/tmp$ kotlinc -version
info: kotlinc-jvm 1.6.10 (JRE 17.0.1+12-LTS)
foo@host:/tmp$ kotlin -version
Kotlin version 1.6.10-release-923 (JRE 17.0.1+12-LTS)
foo@host:/tmp$ javac -version
javac 17.0.1
foo@host:/tmp$ java -version
openjdk version "17.0.1" 2021-10-19 LTS
OpenJDK Runtime Environment Corretto-17.0.1.12.1 (build 17.0.1+12-LTS)
OpenJDK 64-Bit Server VM Corretto-17.0.1.12.1 (build 17.0.1+12-LTS, mixed mode, sharing)
user2297550
  • 3,142
  • 3
  • 28
  • 39

2 Answers2

2

Though I would have expected your code to work to be honest, this could be bug but we'd have to look at the produced bytecode.

In the documentation it says (emphasis mine):

This implicit default value is used when the original map doesn't contain a value for the key specified and a value is obtained with Map.getValue function, for example when properties are delegated to the map.

The conflict of "contracts" comes from the actual Map interface, which says:

Returns the value corresponding to the given [key], or null if such a key is not present in the map.

The maps default contract must fulfill this, so it can "only" return null when a key is non-existent.

I have found one discussion about this in the Kotlin forums.

Martin Marconcini
  • 26,875
  • 19
  • 106
  • 144
  • 1
    thanks for the early response, but the "conflict of 'contracts'" that you mention exists in both calls to `getValue` so why would just one of them fail? – user2297550 Feb 03 '22 at 09:24
  • furthermore, the `MapWithDefault` interface is private so there's no way to use such a map without the public (and "conflicted") interface. Since the feature was designed to work this way, I cannot see any reason why a delegation `by` member would cause anything to change. – user2297550 Feb 03 '22 at 09:26
  • 1
    also, the discussion you found is not related to this issue as I'm using `getValue` in both places, whereas the discussion is about expectations around `[]` and `get` – user2297550 Feb 03 '22 at 09:28
  • Yes, I have the impression it could be a bug and that it has to do with `types`. But maybe someone already knows and can spare us the debugging hours... I believe the delegate is not propagating the type, but I haven't looked at how it is implemented to be honest. – Martin Marconcini Feb 03 '22 at 16:00
  • Yeah, delegates don't propagate their types. The class that uses a delegate only has the types it declares. – Tenfour04 Feb 03 '22 at 16:12
  • 1
    we have a definitive answer by @Tenfour04 but after reviewing things carefully, you are indeed right about the "conflict of 'contracts'" I think the way `withDefault` functionality was added is a snafu haha .. they should have made `MapWithDefault` a public interface imho and added a new method rather than the mess of extension functions, anti-OO, and the conflicted contract .. thanks for your constructive contribution +1 – user2297550 Feb 03 '22 at 16:16
  • There's no great solution. If they did it that way, they'd be shadowing the `getValue` extension function with the `MapWithDefault` abstract `getValue` function. Then `getValue` would behave differently on the same instance depending on whether it is currently known as `Map` vs `MapWithDefault` in the current scope, leading to a lot of surprising situations. – Tenfour04 Feb 03 '22 at 16:20
  • You're welcome; in the end, Kotlin (at least when targeting the JVM) has to abide as much as possible to the constrains imposed by the Java programming language so type erasure, object handling, etc. are still just java bytecode with a lot of sugar on top. In your case (and after looking at this) I'd probably not use `by impl` and just write your own abstraction. Good Luck! – Martin Marconcini Feb 04 '22 at 09:22
2

This is occurring because of the slightly unexpected way in which withDefault is implemented. The wrapper that withDefault produces doesn't override getValue() as this is impossible because getValue() is an extension function. So unfortunately, what we have instead is a classic OOP anti-pattern: getValue() does an is check to see if it's being called on the internal MapWithDefault interface, and only uses the default value if that is the case. I don't see any way they could have avoided this situation without breaking the Map contract.

myGetValue calls getValue on the underlying delegate, which is a MapWithDefault, so it works fine.

getValue called on your MyMap instance will fail the internal is MapWithDefault check because MyMap is not a MapWithDefault, even though its delegate is. The delegates other types are not propagated up to the class that delegates to it, which makes sense. Like if we delegated to a MutableMap, we might want the class to be considered only a read-only Map.

Tenfour04
  • 83,111
  • 11
  • 94
  • 154
  • thanks, would you say that if `MapWithDefault` were a public interface, this situation could have been ameliorated by allowing users like me to subclass `MapWithDefault` instead? – user2297550 Feb 03 '22 at 16:09
  • 1
    Yes, then you could use `MapWithDefault by impl` and it would work. Probably they didn't want to because it's kind of ugly/messy. There are read-only and mutable versions, and it exposes a public function that would never be used outside of the `getValue()` extension function. – Tenfour04 Feb 03 '22 at 16:14
  • yes, i would have still preferred to have a permutation of `MapWithDefault`+`mutable` but with a new function rather than overloading and conflicting with the public documented contract of `getValue` .. sigh .. lesson learned is extension functions are not all that awesome .. thanks for your contribution; accepted +1 – user2297550 Feb 03 '22 at 16:19