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

dealiasing broken where abstract types are involved #8046

Closed
scabug opened this issue Dec 7, 2013 · 9 comments
Closed

dealiasing broken where abstract types are involved #8046

scabug opened this issue Dec 7, 2013 · 9 comments
Assignees
Labels
Milestone

Comments

@scabug
Copy link

scabug commented Dec 7, 2013

In the situation where you have

a) an abstract type
b) a type alias which dealiases to the abstract type
c) an environment where the abstract type has been implemented concretely

Then the type alias should be equivalent to the concrete class which implements the abstract type. It isn't because substitution is broken. Observe the unimplemented type includes T1 and R, which are Function1's type parameters completely escaping.

If the type alias definition is moved from One into Two, then this compiles as given.

trait One {
  type Op[A]
  type Alias[A] = Op[A]
}

trait Two extends One {
  trait Op[A] extends (A => A)

  // This compiles
  class View1 extends Op[Int] { def apply(xs: Int) = xs }

  // ??? base class View2 not found in basetypes of class View2
  // ./a.scala:9: error: class View2 needs to be abstract, since \
  //   method apply in trait Function1 of type (v1: T1)R is not defined
  // (Note that T1 does not match Int)
  //   class View2 extends Alias[Int] { def apply(xs: Int) = xs }
  //         ^
  // one error found
  class View2 extends Alias[Int] { def apply(xs: Int) = xs }
}

Here is another angle on it. f1, f2, f3 all compile. The fact that f2 compiles tells you some trivial substitution is taking place, but it is missing (at least) parents and bounds.

trait Three extends One {
  trait Op[A] extends (A => A)

  def f1(f: Op[Int])            = f(5)
  def f2(f: Alias[Int])         = f(5)
  def f3[T <: Op[Int]](f: T)    = f(5)
  def f4[T <: Alias[Int]](f: T) = f(5)
  // ./a.scala:12: error: type mismatch;
  //  found   : Int(5)
  //  required: T1
  //   def f4[T <: Alias[Int]](f: T) = f(5)
  //                                     ^
}
@scabug
Copy link
Author

scabug commented Dec 7, 2013

Imported From: https://issues.scala-lang.org/browse/SI-8046?orig=1
Reporter: @paulp
Affected Versions: 2.10.3
See #6161

@scabug
Copy link
Author

scabug commented Dec 7, 2013

@paulp said:
I discovered this compiles:

trait One {
  type Op[A]
  type Alias[A] = Op[A]
}

trait Two extends One {
  trait Op[A] extends (A => A)
  override type Alias[A] = Op[A]
  class View2 extends Alias[Int] { def apply(xs: Int) = xs }
}

I guess there's a use for "override type" after all.

@scabug
Copy link
Author

scabug commented Dec 7, 2013

@retronym said:
Linking to #6161 out of kinship. I haven't looked at this enough to say how related they are.

@scabug
Copy link
Author

scabug commented Dec 7, 2013

@retronym said (edited on Dec 7, 2013 10:14:29 PM UTC):
Here lies the telling inconsistency:

> (this, this.info, this.baseClasses, this.info.baseTypeSeq.toList)

(
  class View2,
  Two.this.Alias { def <init>(): Two.this.View2},
  List(class View2, trait Op, trait M, class Object, class Any),
  List(Two.this.View2, Two.this.Op[Int], Object, Any)
)

matchesPrefixAndClass(View2.this.type, trait M)(trait M) yields false because (class View2).isSubClass(trait M) does. It gets its information from this.info.baseTypeIndex, which has lost M.

@scabug
Copy link
Author

scabug commented Dec 8, 2013

@retronym said:
Sprinkling a dealias in computeBaseTypeSeq makes Two, work, but Three remains elusive.

https://github.com/retronym/scala/compare/ticket;8046?expand=1

@scabug
Copy link
Author

scabug commented Dec 8, 2013

@retronym said:
I've just added another commit to that branch that fixes f4. Seems like a pretty fundamental assumption in TypeRef#baseTypeSeqImpl was broken: the symbols BTS can't be mapped element wise if viewing it from a prefix makes an abstract type concrete and contributes new base types.

Both changes in the branch are needed; I was hoping the second would subsume the first. Still feels a bit ad hoc, so I'd appreciate if you could try to poke holes in it, Paul.

@scabug
Copy link
Author

scabug commented Dec 9, 2013

@paulp said:
I've really lost my stomach for looking too hard at the implementation. I very much appreciate your putting time into this, but I'm not a good hole poker at this point. I'm sure I'll still discover plenty of bugs as a "user", and given that this isn't my thing anymore it's plenty of trouble simply to report them. In fact if it weren't for you, I would have retired from opening issues. I'm already in semi-retirement; I'd estimate I discover but don't report 1-2 new bugs per day.

So sure, it looks a bit ad hoc, and think how garishly it would stand out from everything else if it weren't.

@scabug
Copy link
Author

scabug commented Dec 9, 2013

@retronym said (edited on Dec 9, 2013 7:41:56 AM UTC):
I'm more than happy with user-level hole-pokery. You seem to be writing code that exercises these corners: how does it fare with my patch?

@scabug
Copy link
Author

scabug commented Dec 9, 2013

@retronym said:
scala/scala#3242

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