scala / bug

Scala 2 bug reports only. Please, no questions — proper bug reports only.
https://scala-lang.org
232 stars 21 forks source link

ArrayBuffer in 2.13.13 not binary compatible with ArrayBuffer 2.13.12 #13021

Closed noresttherein closed 3 months ago

noresttherein commented 4 months ago

Scala version: 2.13.13

In 2.13.12 and before, ArrayBuffer has method

private[mutable] def ensureAdditionalSize(size :Int) :Unit

Problem

It is no longer present in 2.13.13, being replaced with regular ensureSize. I would like to point out that when I reported that overriding protected[name] methods is possible by classes outside the package (which came as a surprise), it was deemed to work as intended and a perfectly fine situation, so the irony bites me in my behind.

Proly a @deprecatedOverriding would be safer for at least one version.

som-snytt commented 4 months ago

https://github.com/scala/scala/pull/10448

Midnight here, so I'm not sure where your report was, but I do remember an old paulp ticket complaining about lack of enforcement for private[X], and an Odersky comment saying it would nice to pretend that it works (but why keep creating tickets for it, I guess).

Is a lint called for, to warn about migration-unsafety of private[X] or protected[X] members? Is the problem just open packages? Normally you're not "supposed" to add subclasses in mutable.

sjrd commented 4 months ago

I would like to point out that when I reported that overriding protected[name] methods is possible by classes outside the package (which came as a surprise), it was deemed to work as intended and a perfectly fine situation, so the irony bites me in my behind.

Overriding (and accessing) protected[name] is possible by subclasses outside the package. Accessing private[name] members is not, however. We are allowed to remove package-private members at any time, but not package-protected (except in fully-sealed hierarchies).

SethTisue commented 3 months ago

So do we agree that this ticket should be closed as not-a-bug, since the class in question was private to a scala. package?

@noresttherein I'm curious how you even ran into this?

sjrd commented 3 months ago

Yes, afaict this is not-a-bug.

som-snytt commented 3 months ago

Here is the ticket on why overriding private[p] m is not a bug. https://github.com/scala/bug/issues/11339

I made that mistake in my midnight comment.

This is a MiMa bug. And @deprecatedOverriding would have been friendlier. "Little Red Overriding Bug" is my favorite of the classic children's fairy tales.

SethTisue commented 3 months ago

This is a MiMa bug

how so?

som-snytt commented 3 months ago

how so?

The qualified private member is public in bytecode. Or does MiMa not care because everything is public in bytecode.

eed3si9n commented 3 months ago

So I wonder if this is more of a source compatibility, not binary compatibility.

compiles in Scala 2.13.12

package example

class A[A1] extends scala.collection.mutable.ArrayBuffer[A1] {
  override def ensureAdditionalSize(size: Int): Unit =
    println(s"hihi $size")
}

On Scala 2.13.13 you get:

[error]   override def ensureAdditionalSize(size: Int): Unit =
[error]                ^
[error] one error found

compiles in Scala 2.13.13

package example

class A[A1] extends scala.collection.mutable.ArrayBuffer[A1] {
  def ensureAdditionalSize(size: Int): Unit =
    println(s"hihi $size")
}
som-snytt commented 3 months ago

Yes, what I meant to say is that it's so annoying when Seth reminds me that MiMa is for binary not source compat. I also went over to the project site to re-read the Readme. Sorry for the misdirection.

It's still true that @deprecatedOverriding was called for, even if there is no tooling that calls for it. Inheritance is brittle, but we must treat extension as an API contract, as Josh Bloch wrote two decades ago. I think the qualified private case is so tricky that it needs tooling support; we didn't just overlook, we commented that you can slap qualified private on a member and assume immunity, which is incorrect.