scatterMap.asMap().toList() returns wrong values [310365754]

Fixed

Bug

Status Update

No update yet.

Description

ad...@google.com

created issue #1

Nov 11, 2023 07:35PM

Component used: ScatterMap

Version used: HEAD 949f3a11bf0a5d174b3516f3555a0e20bccd0f0e

Devices/Android versions reproduced on: emulator sdk 31

Running this unit test produces the following result:

    @Test
    fun testScatterMap() {
        val scatterMap = mutableScatterMapOf<Int, Int>()
        scatterMap[0] = 0
        scatterMap[1] = -1
        assertThat(scatterMap.asMap().toList()).containsExactly(0 to 0, 1 to -1)
    }

expected      : [(0, 0), (1, -1)]
but was       : [(1, -1), (1, -1)]

Comments

mo...@google.com <mo...@google.com> #2Nov 13, 2023 05:00PM

Reassigned to ro...@google.com.

Romain, are you interested in taking this? If not assign back to me.

ro...@google.com <ro...@google.com> #3Nov 15, 2023 10:59PM

Accepted by ro...@google.com.

I have a fix but going through the Map wrapper + iterators + the way the list is built is absurdly expensive. I'll also add a more efficient toList() to ScatterMap directly.

ap...@google.com <ap...@google.com> #4Nov 16, 2023 12:38AM

Marked as fixed.

Project: platform/frameworks/support
Branch: androidx-main

commit 7814c21f0c9e7fd21fdd3425e19ceeaafe86c66e
Author: Romain Guy <romainguy@google.com>
Date: Wed Nov 15 14:56:49 2023

ScatterMap.asMap() iterator now return new instances

Clients of ScatterMap.asMap() may rely on the map iterator returning new
instances instead of the same wrapper Entry instance. This is the case for
instance of the toList() utility function in Kotlin's stdlib. As a result,
calling ScatterMap.asMap().toList() would always yield a list containing
the *same* Pair in every slot.

This change forces the map iterators to return new entry instances instead.
It's wasteful from a memory/GC's perspective but it matches existing
expectations and code.

Fixes: 310365754
Test: ScatterMapTest
RelNote: N/A
Change-Id: I2893260c67fcbd3388c74043786c0329570ca5db

M collection/collection/src/commonMain/kotlin/androidx/collection/ScatterMap.kt
M collection/collection/src/commonTest/kotlin/androidx/collection/ScatterMapTest.kt

https://android-review.googlesource.com/2833730

ad...@google.com <ad...@google.com> Nov 16, 2023 12:46AM

Reassigned to ro...@google.com.

ap...@google.com <ap...@google.com> #5Nov 16, 2023 04:48PM

Project: platform/frameworks/support
Branch: androidx-main

commit 3b4832a47fa78bdb1d5ed032bcd7b1d513059ff6
Author: Romain Guy <romainguy@google.com>
Date: Wed Nov 15 17:00:33 2023

Fix obsolete documentation

Bug: 310365754
RelNote: N/A
Test: N/A
Change-Id: Ia57992d7ea57daf9037552fdd72dcbec9f7146fa

M collection/collection/src/commonMain/kotlin/androidx/collection/ScatterMap.kt

https://android-review.googlesource.com/2833537

pr...@google.com <pr...@google.com> #6Nov 29, 2023 09:39PM

The following release(s) address this bug.It is possible this bug has only been partially addressed:

androidx.collection:collection:1.4.0-beta02
androidx.collection:collection-iosarm64:1.4.0-beta02
androidx.collection:collection-iossimulatorarm64:1.4.0-beta02
androidx.collection:collection-iosx64:1.4.0-beta02
androidx.collection:collection-jvm:1.4.0-beta02
androidx.collection:collection-linuxx64:1.4.0-beta02
androidx.collection:collection-macosarm64:1.4.0-beta02
androidx.collection:collection-macosx64:1.4.0-beta02

Issue 310365754