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

Yet more errors in code execution order #11756

Open
soronpo opened this issue Oct 2, 2019 · 2 comments

Comments

@soronpo
Copy link

commented Oct 2, 2019

This is a following bug to #11397
Indeed that bug was fixed, but as it turns out the general problem was not fixed. Here is a minimized code that throws an exception when the problem exists.

trait MyAny {
  type TSomething
  type This <: MyAny
}

class MyBool extends MyAny {
  type TSomething = DummyImplicit
  type This = MyBool
}

object Test extends App {
  val i = new MyBool
  var call = 0
  var callList : List[Int] = List()

  final class MyMatch[MV <: MyAny](val matchVal : MV)  {
    def myCase[MC](pattern : Boolean)(block : => Unit)(
      implicit patternBld : matchVal.TSomething
    ) : MyMatch[MV] = {
      call -= 1
      block
      this
    }
  }

  def myMatch[MV <: MyAny](matchValue : MV) : MyMatch[matchValue.This] = {
    callList = callList :+ call
    call += 1
    new MyMatch[matchValue.This](matchValue.asInstanceOf[matchValue.This])
  }

  myMatch(i)
    .myCase(true) {
      myMatch(i)
        .myCase(true) {

        }
    }
    .myCase(true) {
      myMatch(i)
        .myCase(true) {

        }
    }
  
  assert(callList == List(0, 0, -1))
}
@retronym

This comment has been minimized.

Copy link
Member

commented Oct 2, 2019

The by-name argument of myCase is not taken into account.

Rewriting these as explicit Function0 fixes the scope at which the stabilizers are inserted and avoids the assertion error:

trait MyAny {
  type TSomething
  type This <: MyAny
}

class MyBool extends MyAny {
  type TSomething = DummyImplicit
  type This = MyBool
}

object Test extends App {
  val i = new MyBool
  var call = 0
  var callList : List[Int] = List()

  final class MyMatch[MV <: MyAny](val matchVal : MV)  {
    def myCase[MC](pattern : Boolean)(block : () => Unit)(
      implicit patternBld : matchVal.TSomething
    ) : MyMatch[MV] = {
      call -= 1
      block()
      this
    }
  }

  def myMatch[MV <: MyAny](matchValue : MV) : MyMatch[matchValue.This] = {
    callList = callList :+ call
    call += 1
    new MyMatch[matchValue.This](matchValue.asInstanceOf[matchValue.This])
  }

  myMatch(i)
    .myCase(true) { () =>
      myMatch(i)
        .myCase(true) { () =>

        }
    }
    .myCase(true) { () =>
      myMatch(i)
        .myCase(true) { () =>

        }
    }
  
  assert(callList == List(0, 0, -1))
}
    {
      <synthetic> <stable> <artifact> val stabilizer$1: Test.MyMatch[Test.i.This] @scala.reflect.internal.annotations.uncheckedBounds = Test.this.myMatch[MyBool](Test.this.i);
      <synthetic> <stable> <artifact> val stabilizer$3: Test.MyMatch[Test.i.This] @scala.reflect.internal.annotations.uncheckedBounds = stabilizer$1.myCase[Nothing](true)((() => {
        <synthetic> <stable> <artifact> val stabilizer$2: Test.MyMatch[Test.i.This] @scala.reflect.internal.annotations.uncheckedBounds = Test.this.myMatch[MyBool](Test.this.i);
        {
          stabilizer$2.myCase[Nothing](true)((() => ()))(scala.this.DummyImplicit.dummyImplicit);
          ()
        }
      }))(scala.this.DummyImplicit.dummyImplicit);
      stabilizer$3.myCase[Nothing](true)((() => {
        <synthetic> <stable> <artifact> val stabilizer$4: Test.MyMatch[Test.i.This] @scala.reflect.internal.annotations.uncheckedBounds = Test.this.myMatch[MyBool](Test.this.i);
        {
          stabilizer$4.myCase[Nothing](true)((() => ()))(scala.this.DummyImplicit.dummyImplicit);
          ()
        }
      }))(scala.this.DummyImplicit.dummyImplicit)
    };

This is the part of the logic that needs to be nuanced:
https://github.com/scala/scala/blob/f755dc5e3435dc67a505147eb46374d2260da71c/src/compiler/scala/tools/nsc/typechecker/Typers.scala#L5202

My gut feel is that we should consider a slight change to the implementation. Rather looking at the input (untyped) trees in enclosing contexts to exactly determine where the belong, maybe we should just record that there are pending stabilizers due for insertion, but wait until actually typechecking the outer tree before trying to figure out the appropriate insertion context.

@som-snytt

This comment has been minimized.

Copy link

commented Oct 2, 2019

I'll take a look today, as I have a related PR where I also said improve by deferring the insert. I'll update here if I don't make progress.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.