unisonweb / unison

A friendly programming language from the future
https://unison-lang.org
Other
5.75k stars 269 forks source link

mysterious "pure code can't perform I/O" bug during tests but no failing tests #4518

Open ceedubs opened 9 months ago

ceedubs commented 9 months ago

When I run test in this project, I see two bug traces that say "pure code can't perform I/O". That would be fine if I just needed to remove some IO from my tests, but ... it also says that all of my tests pass and doesn't indicate where this IO might be happening. Any idea what's going on here or how I could get more information?

debug.clear-cache doesn't help.

Doing a view on the hashes in the stack trace (ex #tis66bdq67) says that the names aren't in the codebase.

myproject/main> test                                                                                                                                                                                                                                                      

  βœ…                                         

  πŸ’”πŸ’₯

  I've encountered a call to builtin.bug with the following value:

    "pure code can't perform I/O"

  Stack trace:
    lib.base_2_9_1.bug
    #tis66bdq67

  πŸ’”πŸ’₯

  I've encountered a call to builtin.bug with the following value:

    "pure code can't perform I/O"

  Stack trace:
    lib.base_2_9_1.bug
    #pignu03k1t

    New test results:

  β—‰ ByteCount.toText.tests                               Passed
  β—‰ location.LocationId.fromUuidText.roundTrip            : Passed 3 tests.
  β—‰ up.base.Ordering.medianOf3By.test                     : Proved.
  β—‰ up.codec.Decode.compressed.zlib.roundtrip            Passed
  β—‰ up.codec.Decode.fixedSizeOrFail.tests.success        Passed
  β—‰ up.codec.Decode.fixedSizeOrFail.tests.tooFewBytes    Passed
  β—‰ up.codec.Decode.fixedSizeOrFail.tests.tooManyBytes   Passed
  β—‰ up.codec.encode.variableSizeBytes.tests.success      Passed
  β—‰ up.json.Decoder.object.optionalAt.tests.absent       Passed
  β—‰ up.json.Decoder.object.optionalAt.tests.present      Passed
  β—‰ up.List.partition.tests.empty                         : Proved.
  β—‰ up.List.partition.tests.mixed                         : Proved.
  β—‰ up.Stream.diffs.tests.empty                          Passed
  β—‰ up.Stream.diffs.tests.multiple                       Passed
  β—‰ up.Stream.diffs.tests.repeatSingle                   Passed
  β—‰ up.Stream.diffs.tests.single                         Passed
  β—‰ up.Stream.diffs.tests.singlePair                     Passed
  β—‰ up.Stream.diffs.tests.singlePair2                    Passed

  βœ… 18 test(s) passing
ceedubs commented 9 months ago

By doing a diff of these results and find : [test.Result], I did manage to find 3 tests that don't show up in this test output. But strangely none of them seem to contain IO, and if I add them to another project and run test it seems to work fine. Maybe some ghosts from Unison past?

ceedubs commented 9 months ago

If I delete these two tests the problem goes away:

  up.base.reflection.Link.Term.isBuiltin.tests.negative : [test.Result]
  up.base.reflection.Link.Term.isBuiltin.tests.negative = 
    verify do
      term =
        base_2_9_1.abilities.Each.each
          [termLink printLine, termLink Map.insert, termLink base_2_9_1.data.List.map]
      ensuring '(Boolean.not (isBuiltin term))

  up.base.reflection.Link.Term.isBuiltin.tests.positive : [test.Result]
  up.base.reflection.Link.Term.isBuiltin.tests.positive = 
    verify do
      term =
        base_2_9_1.abilities.Each.each
          [termLink (Nat.+), termLink validateSandboxed, termLink open.impl]
      ensuring '(isBuiltin term)

Those do termLink on things like printLine and open.impl, which I could maybe see triggering a false positive on performing IO, since they reference (but don't call) sandboxed terms.

But what's baffling is that I can't seem to reproduce the behavior by copy/pasting those definitions into a new project.

ceedubs commented 9 months ago

For people with access to the @cloud/nimbus repo, this transcript will recreate the issue:

```ucm
.> pull @cloud/nimbus/releases/6.1.3
.> test
ceedubs commented 6 months ago

Okay I figured out what is going on here. Using termLink on a tracked term flags the pure code sandbox, but using it on a non-tracked term does not.

Here is a transcript that reproduces the issue. Note that it suffers from some of the issues documented in #4685.

```ucm
.> builtins.merge

  Done.
untrackedTermLink : [Result]
untrackedTermLink = [if (termLink Nat.toText) == (termLink Nat.toText) then Ok "Passed" else Fail "termLink isn't equal to itself"]

  Loading changes detected in scratch.u.

  I found and typechecked these definitions in scratch.u. If you
  do an `add` or `update`, here's how your codebase would
  change:

    ⍟ These new definitions are ok to `add`:

      untrackedTermLink : [Result]
trackedTermLink : [Result]
trackedTermLink = [if (termLink openFile.impl) == (termLink openFile.impl) then Ok "Passed" else Fail "termLink isn't equal to itself"]

  Loading changes detected in scratch.u.

  I found and typechecked these definitions in scratch.u. If you
  do an `add` or `update`, here's how your codebase would
  change:

    ⍟ These new definitions are ok to `add`:

      trackedTermLink : [Result]
.> add

  ⍟ I've added these definitions:

    trackedTermLink : [Result]

.> test

  βœ…  

  πŸ’”πŸ’₯

  I've encountered a call to builtin.bug with the following
  value:

    "pure code can't perform I/O"

  Stack trace:
    builtin.bug
    #ir7mc8u81u

πŸ›‘

The transcript failed due to an error in the stanza above. The error is:

πŸ’”πŸ’₯

I've encountered a call to builtin.bug with the following value:

"pure code can't perform I/O"

Stack trace: builtin.bug

ir7mc8u81u

aryairani commented 6 months ago

What’s a tracked term?

ceedubs commented 6 months ago

There is a flag on each builtin for whether it is Tracked or Untracked that determines whether it needs to be explicitly enabled when sandboxing. As a heuristic, builtins that perform IO are Tracked. But a builtin being tracked doesn't mean that creating a term link for it should violate the sandbox.