I'm not sure if you can ever get multiple levels of wrapping but isCompatibleCBF(w.unwrap) would also be more concise and avoid repeating the test

done

Is the separate method necessary? Couldn't you put the implementation directly into the second ++ method?

if I chnage to

  override def ++[B1 >: B](xs: GenTraversableOnce[(A, B1)]): Map[A, B1] = ++(xs)(HashMap.canBuildFrom[A, B1])

I get a compile error

Error:(166, 102) type mismatch;
 found   : scala.collection.generic.CanBuildFrom[scala.collection.immutable.HashMap.Coll,(A, B1),scala.collection.immutable.HashMap[A,B1]]
    (which expands to)  scala.collection.generic.CanBuildFrom[scala.collection.immutable.HashMap[_, _],(A, B1),scala.collection.immutable.HashMap[A,B1]]
 required: A
  override def ++[B1 >: B](xs: GenTraversableOnce[(A, B1)]): Map[A, B1] = ++(xs)(HashMap.canBuildFrom[A, B1])

It works if you specify the type parameters:

  override def ++[B1 >: B](xs: GenTraversableOnce[(A, B1)]): Map[A, B1] = ++[(A, B1), Map[A, B1]](xs)(HashMap.canBuildFrom[A, B1])

  override def ++[C >: (A, B), That](that: GenTraversableOnce[C])(implicit bf: CanBuildFrom[HashMap[A, B], C, That]): That = {

The difference must be due to ++ being overloaded. It's strange that it fails with this type error rather than an ambiguous overload error.

done - thanks for the type magic :-)

Is the handling of key and value identities here and in ++: below compatible with the current default (and new fallback) implementation consisting of successive + calls?

I believe so - that was my intention

Is the default merger (aka liftMerger(null)) different from a null merger (as used when calling +)?

yes - liftMerger(null) uses a merger with retainIdentical = true

  private def liftMerger[A1, B1](mergef: MergeFunction[A1, B1]): Merger[A1, B1] =
    if (mergef == null) defaultMerger.asInstanceOf[Merger[A1, B1]] else liftMerger0(mergef)

  private val defaultMerger : Merger[Any, Any] = new Merger[Any, Any] {
    override def apply(a: (Any, Any), b: (Any, Any)): (Any, Any) = a
    override def retainIdentical: Boolean = true
    override val invert: Merger[Any, Any] = new Merger[Any, Any] {
      override def apply(a: (Any, Any), b: (Any, Any)): (Any, Any) = b
      override def retainIdentical: Boolean = true
      override def invert = defaultMerger
    }
  }

this means that the merge code can ignore structurally shared trees

other than that the behaviour is , or should be, identical

This whole branch appears to be identical to the one from the other ++: method above. It could be factored out and shared.

done - and removed the ObjectRef
TODO - should cope with the Maps that now have forEachEntry

did the forEachEntry change, and copied most of that to another PR to reduce the noise in the one

Probably, but the override wouldn't be binary compatible and there's a risk of breaking existing implementations that make ++ in Map call MapBuilder.

Maybe we could do something in 2.15, when that branch starts

Off-topic: I wonder if unsignedCompare (for Int) was ever benchmarked against a more straight-forward version that performs unsigned conversions to Long. It's also confusingly named (compare? for what?)

I think (based in the HashSet changes, this can probably go from this implementation later

removed reference to unsignedCompare from HashMap. There are still some from HashSet until #8589 merges

It looks like this is only called from the first ++ above where the cbf instance is statically known.

doh - changed to be similar to HashMap ++ and addImpl

Why do we need this method that drops the From type instead of making wrapped a val and keeping Nothing?

no idea. Changed it to be a val