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

DFA regression. Branches of AST trees are missed in control flow graph. #11095

Closed
cooldome opened this issue Apr 23, 2019 · 1 comment

Comments

Projects
None yet
2 participants
@cooldome
Copy link
Member

commented Apr 23, 2019

Test case

type
  MyVal[T] = object
    f: ptr T

proc `=destroy`[T](x: var MyVal[T]) =
  if x.f != nil:
    dealloc(x.f)

proc `=sink`[T](x1: var MyVal[T], x2: MyVal[T]) =
  if x1.f != x2.f:
    `=destroy`(x1)
    x1.f = x2.f

proc `=`[T](x1: var MyVal[T], x2: MyVal[T]) {.error.}

proc newVal[T](x: sink T): MyVal[T] =
  result.f = create(T)
  result.f[] = x
 
proc set[T](x: var MyVal[T], val: T) =
  x.f[] = val

proc sinkMe[T](x: sink MyVal[T]) =
  discard

var flag = false

proc main =
  var y = case flag
    of true:
      var x1 = newVal[float](1.0)
      var x2 = newVal[float](2.0)
      (newVal(x1), newVal(x2))
 
    of false:
      var x1 = newVal[float](1.0)
      var x2 = newVal[float](2.0)
      (newVal(x1), newVal(x2))

  sinkMe y[0]
  sinkMe y[1]
  echo "works"

main()

Compilation fails with:

 Error: '=' is not available for type <tuple of (MyVal[MyVal[system.float]], MyVal[MyVal[system.float]])>; requires a copy because it's not the last read of '
  var x1 = newVal(1.0)
  var x2 = newVal(2.0)
  (newVal(x1), newVal(x2))'

The root cause is surprising:
isLastRead returns false because first branch of case expression is missing completely from control flow graph. Caused by 045e026.

The first branch of case expression has the following AST:

 CaseStmt
   Sym "flag"
   OfBranch
       IntLit 1
       HiddenSubConv
          Empty
          StmtListExpr

From

of PathKinds:

it is seen that for node HiddenSubConv the analysis doesn't recurse into n[1] . genUse also can't handle things like stmtListExpr.

Please consider a priority.

@cooldome

This comment has been minimized.

Copy link
Member Author

commented Apr 23, 2019

One thing worth mentioning: weird HiddenSubConv node always appears only on the first branch of case expression, other branches doesn't have HiddenSubConv nodes despite identical code.

@Araq Araq self-assigned this Apr 24, 2019

@Araq Araq added the Showstopper label Apr 24, 2019

Araq added a commit that referenced this issue Apr 24, 2019

Araq added a commit that referenced this issue Apr 24, 2019

@Araq Araq closed this in eb9043c Apr 25, 2019

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