From 8f8f99c565dc02fe8f48de610aac08addc22530f Mon Sep 17 00:00:00 2001 From: Mitchell Dalvi Rosen Date: Mon, 9 Dec 2024 11:54:47 -0500 Subject: [PATCH 1/2] allow add/update of a unison file with failing watches --- .../src/Unison/Codebase/Editor/HandleInput.hs | 4 +- .../Codebase/Editor/HandleInput/Load.hs | 51 +++++++++++-------- .../Unison/Codebase/Editor/HandleInput/Run.hs | 4 +- unison-src/transcripts/idempotent/fix-5354.md | 45 ++++++++++++++++ 4 files changed, 80 insertions(+), 24 deletions(-) create mode 100644 unison-src/transcripts/idempotent/fix-5354.md diff --git a/unison-cli/src/Unison/Codebase/Editor/HandleInput.hs b/unison-cli/src/Unison/Codebase/Editor/HandleInput.hs index 7f585cb329..4967878424 100644 --- a/unison-cli/src/Unison/Codebase/Editor/HandleInput.hs +++ b/unison-cli/src/Unison/Codebase/Editor/HandleInput.hs @@ -1469,7 +1469,9 @@ displayI outputLoc hq = do let filePPED = PPED.makePPED (PPE.hqNamer 10 namesWithDefinitionsFromFile) (suffixify namesWithDefinitionsFromFile) let suffixifiedFilePPE = PPE.biasTo bias $ PPE.suffixifiedPPE filePPED - (_, watches) <- evalUnisonFile Sandboxed suffixifiedFilePPE unisonFile [] + (_, watches) <- + evalUnisonFile Sandboxed suffixifiedFilePPE unisonFile [] & onLeftM \err -> + Cli.returnEarly (Output.EvaluationFailure err) (_, _, _, _, tm, _) <- Map.lookup toDisplay watches & onNothing (error $ "Evaluation dropped a watch expression: " <> Text.unpack (HQ.toText hq)) let ns = UF.addNamesFromTypeCheckedUnisonFile unisonFile names diff --git a/unison-cli/src/Unison/Codebase/Editor/HandleInput/Load.hs b/unison-cli/src/Unison/Codebase/Editor/HandleInput/Load.hs index 350ae30b09..dd1f62eb02 100644 --- a/unison-cli/src/Unison/Codebase/Editor/HandleInput/Load.hs +++ b/unison-cli/src/Unison/Codebase/Editor/HandleInput/Load.hs @@ -77,11 +77,13 @@ loadUnisonFile sourceName text = do when (not . null $ UF.watchComponents unisonFile) do Timing.time "evaluating watches" do - (bindings, e) <- evalUnisonFile Permissive ppe unisonFile [] - let e' = Map.map go e - go (ann, kind, _hash, _uneval, eval, isHit) = (ann, kind, eval, isHit) - when (not (null e')) do - Cli.respond $ Output.Evaluated text ppe bindings e' + evalUnisonFile Permissive ppe unisonFile [] >>= \case + Right (bindings, e) -> do + when (not (null e)) do + let f (ann, kind, _hash, _uneval, eval, isHit) = (ann, kind, eval, isHit) + Cli.respond $ Output.Evaluated text ppe bindings (Map.map f e) + Left err -> Cli.respond (Output.EvaluationFailure err) + #latestTypecheckedFile .= Just (Right unisonFile) where withFile :: @@ -174,29 +176,34 @@ evalUnisonFile :: TypecheckedUnisonFile Symbol Ann -> [String] -> Cli - ( [(Symbol, Term Symbol ())], - Map Symbol (Ann, WK.WatchKind, Reference.Id, Term Symbol (), Term Symbol (), Bool) + ( Either + Runtime.Error + ( [(Symbol, Term Symbol ())], + Map Symbol (Ann, WK.WatchKind, Reference.Id, Term Symbol (), Term Symbol (), Bool) + ) ) evalUnisonFile mode ppe unisonFile args = do - Cli.Env {codebase, runtime, sandboxedRuntime, nativeRuntime} <- ask + env <- ask + let theRuntime = case mode of - Sandboxed -> sandboxedRuntime - Permissive -> runtime - Native -> nativeRuntime + Sandboxed -> env.sandboxedRuntime + Permissive -> env.runtime + Native -> env.nativeRuntime let watchCache :: Reference.Id -> IO (Maybe (Term Symbol ())) watchCache ref = do - maybeTerm <- Codebase.runTransaction codebase (Codebase.lookupWatchCache codebase ref) + maybeTerm <- Codebase.runTransaction env.codebase (Codebase.lookupWatchCache env.codebase ref) pure (Term.amap (\(_ :: Ann) -> ()) <$> maybeTerm) Cli.with_ (withArgs args) do - (nts, errs, map) <- - Cli.ioE (Runtime.evaluateWatches (Codebase.codebaseToCodeLookup codebase) ppe watchCache theRuntime unisonFile) \err -> do - Cli.returnEarly (Output.EvaluationFailure err) - when (not $ null errs) (RuntimeUtils.displayDecompileErrors errs) - for_ (Map.elems map) \(_loc, kind, hash, _src, value, isHit) -> do - -- only update the watch cache when there are no errors - when (not isHit && null errs) do - let value' = Term.amap (\() -> Ann.External) value - Cli.runTransaction (Codebase.putWatch kind hash value') - pure (nts, map) + let codeLookup = Codebase.codebaseToCodeLookup env.codebase + liftIO (Runtime.evaluateWatches codeLookup ppe watchCache theRuntime unisonFile) >>= \case + Right (nts, errs, map) -> do + when (not $ null errs) (RuntimeUtils.displayDecompileErrors errs) + for_ (Map.elems map) \(_loc, kind, hash, _src, value, isHit) -> do + -- only update the watch cache when there are no errors + when (not isHit && null errs) do + let value' = Term.amap (\() -> Ann.External) value + Cli.runTransaction (Codebase.putWatch kind hash value') + pure (Right (nts, map)) + Left err -> pure (Left err) diff --git a/unison-cli/src/Unison/Codebase/Editor/HandleInput/Run.hs b/unison-cli/src/Unison/Codebase/Editor/HandleInput/Run.hs index 9cf1cbeaff..072c0a07fa 100644 --- a/unison-cli/src/Unison/Codebase/Editor/HandleInput/Run.hs +++ b/unison-cli/src/Unison/Codebase/Editor/HandleInput/Run.hs @@ -56,7 +56,9 @@ handleRun native main args = do let pped = PPED.makePPED (PPE.hqNamer 10 namesWithFileDefinitions) (PPE.suffixifyByHash namesWithFileDefinitions) let suffixifiedPPE = PPED.suffixifiedPPE pped let mode | native = Native | otherwise = Permissive - (_, xs) <- evalUnisonFile mode suffixifiedPPE unisonFile args + (_, xs) <- + evalUnisonFile mode suffixifiedPPE unisonFile args & onLeftM \err -> + Cli.returnEarly (Output.EvaluationFailure err) mainRes :: Term Symbol () <- case lookup magicMainWatcherString (map bonk (Map.toList xs)) of Nothing -> diff --git a/unison-src/transcripts/idempotent/fix-5354.md b/unison-src/transcripts/idempotent/fix-5354.md new file mode 100644 index 0000000000..14ad221b26 --- /dev/null +++ b/unison-src/transcripts/idempotent/fix-5354.md @@ -0,0 +1,45 @@ +``` ucm +scratch/main> builtins.mergeio + + Done. +``` + +``` unison :error +> todo "" + +foo = 42 +``` + +``` ucm :added-by-ucm + 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`: + + foo : Nat + + Now evaluating any watch expressions (lines starting with + `>`)... Ctrl+C cancels. + + 💔💥 + + I've encountered a call to builtin.todo with the following + value: + + "" + + Stack trace: + todo + #0k89ebstt4 +``` + +``` ucm +scratch/main> add + + ⍟ I've added these definitions: + + foo : Nat +``` From 7f76fd8f2a6aef72ede34d2a628f8951447f90bf Mon Sep 17 00:00:00 2001 From: Mitchell Dalvi Rosen Date: Mon, 9 Dec 2024 14:47:20 -0500 Subject: [PATCH 2/2] run transcript --- unison-src/transcripts/idempotent/fix-5354.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/unison-src/transcripts/idempotent/fix-5354.md b/unison-src/transcripts/idempotent/fix-5354.md index 14ad221b26..84de08b65f 100644 --- a/unison-src/transcripts/idempotent/fix-5354.md +++ b/unison-src/transcripts/idempotent/fix-5354.md @@ -18,7 +18,7 @@ foo = 42 change: ⍟ These new definitions are ok to `add`: - + foo : Nat Now evaluating any watch expressions (lines starting with