Skip to content

Commit

Permalink
fix let-capture issue in term renderer
Browse files Browse the repository at this point in the history
  • Loading branch information
mitchellwrosen committed Dec 18, 2024
1 parent eca815c commit 28884e3
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 35 deletions.
29 changes: 15 additions & 14 deletions parser-typechecker/src/Unison/Syntax/TermPrinter.hs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ module Unison.Syntax.TermPrinter
where

import Control.Lens (unsnoc)
import Control.Monad.Reader (ask, local)
import Control.Monad.State (evalState)
import Control.Monad.State qualified as State
import Data.Char (isPrint)
Expand Down Expand Up @@ -53,7 +54,7 @@ import Unison.Referent (Referent)
import Unison.Referent qualified as Referent
import Unison.Syntax.HashQualified qualified as HQ (unsafeFromVar)
import Unison.Syntax.Lexer.Unison (showEscapeChar)
import Unison.Syntax.Name qualified as Name (isSymboly, parseText, parseTextEither, toText, unsafeParseText)
import Unison.Syntax.Name qualified as Name (isSymboly, parseText, parseTextEither, toText, unsafeParseText, unsafeParseVar)
import Unison.Syntax.NamePrinter (styleHashQualified'')
import Unison.Syntax.NameSegment qualified as NameSegment (toEscapedText)
import Unison.Syntax.Precedence (InfixPrecedence (..), Precedence (..), increment, isTopLevelPrecedence, operatorPrecedence)
Expand All @@ -68,7 +69,6 @@ import Unison.Util.Pretty qualified as PP
import Unison.Util.SyntaxText qualified as S
import Unison.Var (Var)
import Unison.Var qualified as Var
import Control.Monad.Reader (ask)

type SyntaxText = S.SyntaxText' Reference

Expand Down Expand Up @@ -217,11 +217,13 @@ pretty0
}
term =
specialCases term \case
Var' v -> do
Var' (Var.reset -> v) -> do
env <- ask
let name =
if Set.member v env.boundTerms
then HQ.fromName (Name.makeAbsolute (Name.unsafeParseVar v))
else elideFQN im $ HQ.unsafeFromVar v
pure . parenIfInfix name ic $ styleHashQualified'' (fmt S.Var) name
where
-- OK since all term vars are user specified, any freshening was just added during typechecking
name = elideFQN im $ HQ.unsafeFromVar (Var.reset v)
Ref' r -> do
env <- ask
let name = elideFQN im $ PrettyPrintEnv.termName env.ppe (Referent.Ref r)
Expand Down Expand Up @@ -688,10 +690,9 @@ printLetBindings context = \case
LetrecBindings bindings -> traverse (printLetrecBinding context) bindings

printLetBinding :: (MonadPretty v m) => AmbientContext -> (v, Term3 v PrintAnnotation) -> m (Pretty SyntaxText)
printLetBinding context (v, binding) =
if Var.isAction v
then pretty0 context binding
else
printLetBinding context (v, binding)
| Var.isAction v = pretty0 context binding
| otherwise =
-- For a non-recursive let binding like "let x = y in z", variable "x" is not bound in "y". Yet, "x" may be free
-- in "y" anyway, referring to some previous binding.
--
Expand All @@ -702,10 +703,10 @@ printLetBinding context (v, binding) =
--
-- So, render free "x" in "y" with a leading dot. This is because we happen to know that the only way to have
-- a free "x" in "y" is if "x" is a top-level binding.
let
v1 = Var.reset v
in
renderPrettyBinding <$> prettyBinding0' context (HQ.unsafeFromVar v1) binding
renderPrettyBinding
<$> local (over #boundTerms (Set.insert v1)) (prettyBinding0' context (HQ.unsafeFromVar v1) binding)
where
v1 = Var.reset v

printLetrecBinding :: (MonadPretty v m) => AmbientContext -> (v, Term3 v PrintAnnotation) -> m (Pretty SyntaxText)
printLetrecBinding context (v, binding) =
Expand Down
30 changes: 9 additions & 21 deletions unison-src/transcripts/idempotent/fix-5427.md
Original file line number Diff line number Diff line change
Expand Up @@ -122,36 +122,24 @@ bar =
foo : Nat
```

This should succeed, but `bar` gets printed incorrectly\!
Previously, `bar` would incorrectly print with a `foo = foo` line. Now, it works.

``` ucm :error
``` ucm
scratch/main> update
Okay, I'm searching the branch for code that needs to be
updated...
That's done. Now I'm making sure everything typechecks...
Typechecking failed. I've updated your scratch file with the
definitions that need fixing. Once the file is compiling, try
`update` again.
```

``` unison :added-by-ucm scratch.u
foo : Nat
foo = 18
bar : Nat
bar =
foo = foo
foo
Everything typechecks, so I'm saving the results...
-- The definitions below no longer typecheck with the changes above.
-- Please fix the errors and try `update` again.
Done.
baz : Nat
baz =
use Nat +
foo + foo
scratch/main> view bar
bar : Nat
bar =
foo = .foo
foo
```

0 comments on commit 28884e3

Please sign in to comment.