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

Change is volatile #1051

Merged
merged 23 commits into from Feb 9, 2016
Merged

Change is volatile #1051

merged 23 commits into from Feb 9, 2016

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Jan 29, 2016

Fixes #1047 and prepares the way for other realizibility tests involving null and type projections.

@odersky
Copy link
Contributor Author

odersky commented Feb 2, 2016

Also fixes #50 and #1050 and SI-7278.

@odersky
Copy link
Contributor Author

odersky commented Feb 2, 2016

State so far: Problems involving type projections and laziness should be addressed by this PR. Problems involving null need the planned changes to the typesystem which model null explicitly. Problems involving uninitialized fields need to be handled separately.

@odersky
Copy link
Contributor Author

odersky commented Feb 8, 2016

What's the review status of this one? If there are no further comments this will be merged.

@smarter
Copy link
Member

smarter commented Feb 8, 2016

I'll review this today


/** Is this type a path with some part that is initialized on use?
*/
private def isLateInitialized(tp: Type): Boolean = tp.dealias match {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method is never used.

@smarter
Copy link
Member

smarter commented Feb 8, 2016

This patch makes the following code illegal:

trait HasA {
  type A
}
trait Foo[T <: HasA] {
  type B = T#A // error: T is not a legal path since it is not a concrete type
}

However, I think that we can safely allow this, because Foo#B is not a valid path, you need to write Foo[something]#B and we will check at that point if the type is legal or not. What do you think?

@smarter
Copy link
Member

smarter commented Feb 8, 2016

Nevermind, if we allow this then the following would compile:

object Test {
  trait C { type A }

  type T = C { type A >: Any }
  type U = C { type A <: Nothing }

  trait Foo[Elem <: T] {
    def foo: Elem#A = 1
  }

  def main(args: Array[String]) = {
    val a = new Foo[T & U] {}
    val z: String = a.foo
  }
}

@smarter
Copy link
Member

smarter commented Feb 8, 2016

This pull request needs to be rebased.

case SelectFromTypeTree(qual, name) if name.isTypeName =>
checkRealizable(qual.tpe, qual.pos)
case SingletonTypeTree(ref) =>
checkRealizable(ref.tpe, ref.pos)
case _ =>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about AndTypeTree, OrTypeTree, RefinedTypeTree, etc? Maybe we should add a case like:

case typ: TypTree =>
  checkRealizable(typ.tpe, typ.pos)

I think that if we do this, then my proposition at #1051 (comment) becomes safe, because Foo[T & U] will be invalid.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's not how things are done in DOT. We have one scheme to ensure soundness that we know works and a lot of others that we thought worked at some time, but didn't in the end. So we should stick to the one that works. I believe approximating realizability of other types is too strict and is also not necessary.

Replaces isVolatile, which is too weak (and more complicated).
Backwards compatibility with Scala2 is ensured by dropping the
requirement in Scala2 mode.
Fixes scala#1047, which now compiles without inifinite recursion.
This prepares the way for using isRealizable in different contexts.
Distinguish between isStable and isRealizable.
Issue migration warnings for realizibility failures.
Provide error diagnostics why something is not realizable.
EffectivelyFinal came without documentation, so it was not clear
what is was supposed to compute. I looked at the use sites, and it
seems that all they need is "impossible to override". So I changed
the code to do that and dropped the additional condition that
members of modules or final classes were not allowed to be lazy or
mutable. It was not clear to me what that had to do with finality.
Tests with failed projections are moved to pos-scala2, which
was renamed from pos-special. Files in pos-scala2 are compiled
with -language:Scala2 option.
Reason: They might be overridden by other lazy vals
that are not realizable, and therefore risk creating
bad bounds.
Fix wording so that it works for nested errors as well.
Incorparte Tiark's latest example.
If `T` is a member of `p` then

    { import p._; ... T ... }

should be checked in the same way as

    { ... p.T ... }
Need to demand "effecively final" instead of `is(Final)`.
The lines in question now cause an error ("cannot be instantiated...")
which masks the real tests at phase PostTyper.

Also adapt bugcount of hklower test
These are replaced by the realizibility logic.
should be terated analogous to lazy vals for realizability checking.
Move logic from TypeOps to new file CheckRealizable.scala.
Also check realizable fields under strict mode.
Check at phase PostTyper rather than Typer to avoid cycles.
New tests for imports and deep paths.
@Atry
Copy link
Contributor

Atry commented Sep 4, 2017

Is it possible to treat singleton types as stable?
Does it sound?

object O {
  type T = String
}

val stableVal: O.type = O

def stableMethod: stableVal.type = stableVal

def doesItSound(x: stableMethod.T) = x

@smarter
Copy link
Member

smarter commented Sep 4, 2017

@Atry Maybe, but it seems cleaner to never allow defs to be stable. In your example you could just write val stableMethod: O.type = O instead, is there a specific reason why you would want it to be a def?

@Atry
Copy link
Contributor

Atry commented Sep 4, 2017

val stableMethod costs some memory overhead at runtime.

@Atry
Copy link
Contributor

Atry commented Sep 4, 2017

Thinking about a very complex monad transformer chain, each depends on another.

@DhashS
Copy link

DhashS commented Sep 18, 2017

Hey, I'm fairly new to Scala and was curious about just this in something I'm writing.

Having both # and the dot-notation way of accessing the inner type is not particularly ergonomic.
In the function body, I think they do different things. (If i'm understanding https://stackoverflow.com/questions/9443004/what-does-the-operator-mean-in-scala correctly).

The existing behaviour is semi-attractive, but if the # syntax for in declaring function argument types is probably expected behavior, disallowing dot-notation there might make sense.

I'm not super-sure what I've said is sensical, I'm a fairly new Scala programmer, and not a compiler dev.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants