unexpected closure in testing block

[JakeWheat]

The testing block is picking up the "wrong b": I expected the 'b in test' message to be 1, but it seems to be picking up the shadow b that is defined after the block that references it.

var a = 1
b = 1

print("b before test: " + torepr(b))

check:
  print("a in test: " + torepr(a))
  print("b in test: " + torepr(b))
end

print("a before assign: " + torepr(a))

a := 2

print("a after assign: " + torepr(a))

shadow b = 2

check:
  # errors: "The identifier c is unbound"
  # print("c in test: " + torepr(c))
  true
end

var c = 1

output:

b before test: 1
"b before test: 1"
a before assign: 1
"a before assign: 1"
a after assign: 2
"a after assign: 2"
a in test: 2
b in test: 2

[jpolitz]

Agreed this should print 1 in that test. Unfortunately, since scope resolution happens after check blocks are “moved” to the end of the file, I'm not sure the fix for this will be super quick.

Thanks for finding this!

[JakeWheat]

To recreate the behaviour of pyret in my toy implementation, I thought about creating a lambda where the check block is that runs the test, and 'registering' it and then it only gets executed at the end of the script. This way it captures the correct closure. I came up with this when trying to figure out how a where block in a function nested e.g. in the body of another function could be implemented:

fun a():
  x = 1
  ...
  fun b():
    ...
  where:
    something(x) is ...
  end
end

I saw in the official implementation, this where block will generate a new check block test result each time a is called, and if it isn't called at all, the where block never gets run.

[blerner]

Err, no, isn't this working as intended? Because we want to be able to write examples blocks before the functions they're example'ing are defined, and they can use whatever helpers are needed (other functions, other constants, etc). So we've deliberately had the semantics that examples/check blocks get "sunk" to the end of the file, and are run after everything else in the file is defined...

[JakeWheat]

Something is surprising and/or inconsistent to me - this works:

check:
  f(1) is 3
end

#a = 3

fun f(_):
  a = 3
  a
end

This gives an error:

check:
  f(1) is 3
end

a = 3

fun f(_):
  #a = 3
  a
end

[blerner]

Ok, that second example is weird, but is unrelated to the original problem. The (implementation-level) reason here is because desugar-scope-block turns s-check statements into add-letrec-bindss of s-underscores to the result of the check block. Then, in the first example, the s-check's letrec is combined with the letrec of the subsequent definition of f, so they're in the same scope, and name resolution works. In the failing example, the definition of a is a non-recursive s-let, which breaks up the combined letrec group, so the scope fails. (This is deliberate behavior of how let-bindings and letrec-group-bindings work, but it's rather inconvenient here.)

We can see this in the output from show-compilation. If we move the definition of a before the check block, we get desugaring output of

let  a = 3:
  letrec  _ = check: f(1) is 3 end, f = lam(_): a end:
    let 
        shadow result-after-checks1 =
          builtins.trace-value(builtin("dummy location"), nothing):
...

but in the failing case, we get

letrec  _ = check: f(1) is 3 end:
  let  a = 3:
    letrec  f = lam(_): a end:
      let 
          shadow result-after-checks1 =
            builtins.trace-value(builtin("dummy location"), nothing):
...

If we mark the definition of a as rec a = 3, in between the check and f definitions, we get:

letrec  _ = check: f(1) is 3 end, a = 3, f = lam(_): a end:
  let 
      shadow result-after-checks1 =
        builtins.trace-value(builtin("dummy location"), nothing):
...

which again works.

I don't know whether or how to fix this, honestly.

[blerner]

The original error reported in this bug is complicated. Looking at the show-compilation output, the Resolved names pass shows

let  var a = 1, b = 1:
  builtins.trace-value(srcloc("file:///home/blerner/pyret-lang/test1.arr", 4, 0, 17, 4, 36, 53),
    print("b before test: " + torepr(b)))
  letrec 
      $underscore =
        check:
          print("a in test: " + torepr(!a))
          print("b in test: " + torepr(b))
        end:
    builtins.trace-value(srcloc("file:///home/blerner/pyret-lang/test1.arr", 11, 0, 137, 11, 38, 175),
      print("a before assign: " + torepr(!a)))
    builtins.trace-value(srcloc("file:///home/blerner/pyret-lang/test1.arr", 13, 0, 177, 13, 6, 183),
      a := 2)
    builtins.trace-value(srcloc("file:///home/blerner/pyret-lang/test1.arr", 15, 0, 185, 15, 37, 222),
      print("a after assign: " + torepr(!a)))
    let  shadow b = 2:
      letrec  $underscore = check: print(true) end:
        let 
            var c = 1,
            shadow result-after-checks1 =
              builtins.trace-value(builtin("dummy location"), nothing):
          builtins.current-checker().run-checks("file:///home/blerner/pyret-lang/test1.arr",
            [list: 
              {
                name: "check-block-1",
                run: lam() block:
                    print("a in test: " + torepr(!a))
                    print("b in test: " + torepr(b))
                  end,
                keyword-check: true,
                location: makeSrcloc(srcloc("file:///home/blerner/pyret-lang/test1.arr", 6, 0, 55, 9, 3, 135))
              },
              {
                name: "check-block-2",
                run: lam() block: print(true) end,
                keyword-check: true,
                location: makeSrcloc(srcloc("file:///home/blerner/pyret-lang/test1.arr", 19, 0, 238, 23, 3, 341))
              }
            ])
          Module(Answer : result-after-checks1,
          DefinedValues :
            [a : a, b : b, c : c, result-after-checks1 : result-after-checks1],
          DefinedTypes : [], checks : builtins.current-checker().results())
        end
      end
    end
  end
end

which shows that we've duplicated the code in the check blocks -- it's present as a binding to $underscore and also in the check-block-1 lambda. This is almost certainly a mistake of some kind...

[JakeWheat]

For what it's worth, I only found these bugs because I was digging around in exactly how check and closures worked together for the purposes of learning more about the implementation and behaviour of Pyret itself. If these bugs haven't been seen before then maybe they are quite esoteric.

[michaelballantyne]

Re: the duplication @blerner mentions, it looks like the check-visitor gathers checks and inserts their desugaring at the end of the block, but doesn't remove the original checks.

Right now it looks like they pass through desugar and get dropped in desugar-post-tc, in contradiction of the pre- and post- condition statements.

Perhaps desugar-check should also apply the no-checks-visitor after the check-visitor in order to remove them.

[jpolitz]

@michaelballantyne you are right in nearly all the ways that matter. Unfortunately right now the type checker is relying on one placement of the check blocks, while desugar-checks is relying on another, so it's more complex to remove.