private[this] inference does not work for constructor parameters

[yaniskas]

Compiler version

3.6.3

Minimized code

import scala.collection.mutable.ArrayBuffer

class PrivateTest[-M](private val v: ArrayBuffer[M])

Output

-- Error: .\PrivateTest.scala:3:34 ---------------------------------------------
3 |class PrivateTest[-M](private val v: ArrayBuffer[M])
  |                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |contravariant type M occurs in invariant position in type scala.collection.mutable.ArrayBuffer[M] of value v

Expectation

This code compiles correctly if you use the deprecated private[this] syntax, so according to the Scala 3 reference, it looks like the compiler should treat v as if it had been declared private[this], and the code should compile without errors. It also happens to compile correctly if you do not include the private val declaration at all and let the compiler infer it.

[som-snytt]

Apparently, that is an intentional feature:
https://github.com/scala/scala3/blob/3.6.3/compiler/src/dotty/tools/dotc/core/SymDenotations.scala#L2644

It would be nice if the reference clarified that.

The idea is that deciding if a field is required, etc, is already done "by construction" in constructors for class params, so adding val signals that you really want it.

Inferring it "back" to private local means the field can be eliminated, which is not desirable.

A possible nuance would be "don't eliminate inferred private local, only use that for variance checking", but maybe that is too much nuance.

This corner case is served just by introducing a private member "manually".

[yaniskas]

Actually, I feel like that nuance you mentioned would make more sense. I would say that all the old uses of private[this] should be served by just marking things as private.

Also, while the val case is served by omitting the val declaration, if you want to make the field a var you have no other option than to use the manual method you mentioned where you pass a constructor parameter with a different name and then initialize a var field with that value. Isn't this too much boilerplate to replace a deprecated feature?

[yaniskas]

And there's another related issue: if you use the -rewrite option in scalac on a working example with a private[this] constructor parameter, it produces uncompilable code

[som-snytt]

@yaniskas thanks, I'll also check out the rewrite bug. I submitted a PR only to document status quo.

You could open a "Discussion" topic if you feel strongly about the feature. Or I could "convert to discussion" this ticket; my PR doesn't have to "fix" it.

I don't have an opinion. I observe the sometimes-gnarly syntax is "just a convenient shorthand" that is optimized for the common case. var is less common, and caring about fields is less common. Also variance. Or private constructors.

Actually, I do have an opinion: I'd prefer the simplicity of private meaning the same thing, without exemption.

Edit: I misunderstood what you meant about the rewrite: not that it produces bad syntax, just that it requires refactoring in this case. That's interesting, whether it should refuse the rewrite.

[yaniskas]

With the rewrite I meant that it just removes the [this] part, and then fails the variance check. In my opinion, the rewrite is doing the correct thing, and it's the variance check that should pass.

To me it just seems like a regression/unnecessary removal of a feature. And I don't see why the syntax for val declarations should be simpler than for var declarations in this case. I guess it would be nice to convert it to a discussion topic, but will it become less visible that way?

[som-snytt]

I'll leave it here with the triage label so it's not overlooked.

[Gedochao]

To me it just seems like a regression/unnecessary removal of a feature.

I wouldn't treat this a regression, although I agree that this bug is causing -rewrite to produce erroneous code on Scala 3.4+, so it could be argued it is one (which in my book bumps its priority by a notch).