Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

private types displayed in error messages #8812

Open
scabug opened this issue Aug 23, 2014 · 7 comments
Open

private types displayed in error messages #8812

scabug opened this issue Aug 23, 2014 · 7 comments
Labels
Milestone

Comments

@scabug
Copy link

scabug commented Aug 23, 2014

Oh good, another bug involving access. Just throw it on the dead letter pile.

class Foo(x: Int) {
  // a private type should never be displayed
  private type This = Foo
  def + (x: This): This = this
}
class Bar {
  new Foo(5) + Nil    // required: Foo
  val v = new Foo(5)
  v + Nil             // required: Bar.this.v.This (which expands to) Foo
}
@scabug
Copy link
Author

scabug commented Aug 23, 2014

Imported From: https://issues.scala-lang.org/browse/SI-8812?orig=1
Reporter: @paulp
Affected Versions: 2.11.5

@scabug
Copy link
Author

scabug commented Feb 20, 2015

@retronym said (edited on Feb 20, 2015 12:05:40 AM UTC):
The difference between the two errors is related to #9156. In the second case, the existence of the stable prefix v for the type means we don't need to capture an existential for this in an asSeenFrom, which ends up dealiasing This to Foo.

It seems a poor design to use the private type alias in the signature of a public method. We don't prevent that outright. Why not?

Using a smaller example:

trait Test {
  private type T = Int
  val t: T
}

checkNoEscaping detects that the RHS of line 3 contains a hidden symbol. But, rather than issuing the error, it instead discards that hidden symbol and continues analysing as though it you wrote val t: Int.

            case TypeRef(_, sym, args) =>
              checkNoEscape(sym)
              if (!hiddenSymbols.isEmpty && hiddenSymbols.head == sym &&
                  sym.isAliasType && sameLength(sym.typeParams, args)) {
                hiddenSymbols = hiddenSymbols.tail
                t.dealias
              } else t

The sameLength check smells funny, and reveals an inconsistency when you introduce tcpoly:

trait Test {
  private type T[_] = Int
  val t: M[T] // error: private type T escapes its defining scope as part of type M[Test.this.T]
}
trait M[N[_]]

But after we admit val t: T, no further attempt is made to actually typecheck it as val t: Int, which would be needed to avoid it showing up down the line in a "found / required" error message.

This dealiasing-rather-than-forbidding dates back to the dawn of existential types: scala/scala@8414eba

Next steps to improve on this would be:

  • make checkNoEscaping stricter (ie, no dealiasing to avoid the errors) to see what breaks. This ought to pretty quickly reverse engineer the motivation.
  • (maybe) add a stricter mode to the compiler to turn it off.

@scabug
Copy link
Author

scabug commented Feb 20, 2015

@paulp said:

It seems a poor design to use the private type alias in the signature of a public method. We don't prevent that outright. Why not?

Well, it has always been like that, so it is most likely unchangeable. And it's useful, or would be if it worked sensibly. If a private type alias to a public type is expanded when it appears in a public type, and one is stuck repeating a type like "Dingo with Foo with Bar" fifty times in the method signatures of the interface, one is glad to be able to call it This. But I'd be crazy to start talking about access. I'll just refer you to my fifty open access-related tickets, anything I might say about this has to be in four of them.

@scabug
Copy link
Author

scabug commented Feb 20, 2015

@retronym said:
Why not just make the alias public? If it a pain to write the A with B with C, it will be an even bigger pain for clients to read the expanded type in the API docs.

@scabug
Copy link
Author

scabug commented Feb 20, 2015

@paulp said:
It's actually not quite as painted in my case... value classes make you duplicate everything directly in the class if you don't want to be boxed. If I have a bunch of Int ops and a bunch of Long ops which are identical except for whether they're acting on Ints and Longs, I can write it in terms of an alias and use the identical boilerplate in each. Otherwise I have to turn the placeholder type into Int in one and Long in one and it complicates the whole pointless exercise.

I'm not saying this is a compelling use case, even though it's all inflicted on me by scala, but it's the principle of the thing. I shouldn't have to introduce a type alias into the API just because I want one for the hygiene of a single source file.

@scabug
Copy link
Author

scabug commented Feb 20, 2015

@paulp said:
I guess https://github.com/scala/scala/blob/2.11.x/src/library/scala/collection/mutable/IndexedSeqView.scala offers an adequate illustration of why one might want to do this. Note that you couldn't make the type alias public even if you were inclined to do so because of variance. So every one of those Thises would be IndexedSeqView[A, Coll], with all that that implies.

trait IndexedSeqView[A, +Coll] ... {
  private[this] type This = IndexedSeqView[A, Coll]

  override def filter(p: A => Boolean): This       
  override def init: This                          
  override def drop(n: Int): This                  
  override def take(n: Int): This                  
  override def slice(from: Int, until: Int): This  
  override def dropWhile(p: A => Boolean): This    
  override def takeWhile(p: A => Boolean): This    
  override def span(p: A => Boolean): (This, This) 
  override def splitAt(n: Int): (This, This)       
  override def reverse: This                       
}

I don't think a self-respecting language can make someone type IndexedSeqView[A, Coll] twelve times in ten lines.

@scabug
Copy link
Author

scabug commented Feb 20, 2015

@retronym said:
Good point. We just about want:

@inline private[this] type This = IndexedSeqView[A, Coll]

I'm still not sure if private type or private[this] type really makes sense as the trigger for this. (Then again, it is no worse than final val /**notype**/ for constant folding.)

@scabug scabug added the access label Apr 7, 2017
@scabug scabug added this to the Backlog milestone Apr 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants