Skip to content

ContiguousMap indexer set on an existing key replaces the stored key with the caller's key #69

Description

@matt-edmondson

What's wrong

ContiguousMap<TKey,TValue>'s indexer setter (Containers/ContiguousMap.cs:178) handles an existing key with:

if (keyToIndex.TryGetValue(key, out int index))
{
    // Key exists, update the value
    items[index] = new Entry(key, value);
}

It writes the caller's key into the entry, not the key that was stored first. With a custom comparer, the key in items stops matching the key held in keyToIndex.

Repro

var m = new ContiguousMap<string, int>(StringComparer.OrdinalIgnoreCase);
m["Apple"] = 1;
m["APPLE"] = 2;
// Keys / foreach / AsSpan() now report "APPLE"
// keyToIndex still holds "Apple"
  • Actual: Keys, enumeration and AsSpan() return "APPLE".
  • Expected: "Apple", with value 2. Dictionary<,> behaves this way, and so do the sibling InsertionOrderMap and OrderedMap, which update only the value.

I confirmed this in a scratch console app against the current main.

Why it matters

Updating a value silently changes a key's identity, and only in this one map type. Two cases are affected:

  • Code that swaps InsertionOrderMap/Dictionary for ContiguousMap for performance gets different keys back.
  • Any case-insensitive or otherwise normalising comparer gets its keys rewritten by whichever caller spelling came last.

Suggested fix

items[index] = new Entry(items[index].Key, value);

Add a regression test that uses a case-insensitive comparer and asserts the original key's casing survives an update through the indexer.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions