From c08c30906993c0c6376f15b5dc4620e62afa22d0 Mon Sep 17 00:00:00 2001 From: Mitchell Dalvi Rosen Date: Mon, 25 Nov 2024 13:56:41 -0500 Subject: [PATCH 1/2] add failing transcript --- unison-src/transcripts/fix-5464.md | 37 ++++++++ unison-src/transcripts/fix-5464.output.md | 107 ++++++++++++++++++++++ 2 files changed, 144 insertions(+) create mode 100644 unison-src/transcripts/fix-5464.md create mode 100644 unison-src/transcripts/fix-5464.output.md diff --git a/unison-src/transcripts/fix-5464.md b/unison-src/transcripts/fix-5464.md new file mode 100644 index 0000000000..61bb5f3297 --- /dev/null +++ b/unison-src/transcripts/fix-5464.md @@ -0,0 +1,37 @@ +```ucm +scratch/main> builtins.merge lib.builtin +``` + +```unison +foo : Nat +foo = + baz = bar.baz + bar.baz + 19 + +bar.baz : Nat +bar.baz = 20 + +qux : Nat +qux = foo + foo +``` + +```ucm +scratch/main> add +``` + +```unison +foo : Nat +foo = + baz = bar.baz + bar.baz + 20 + +bar.baz : Nat +bar.baz = 20 +``` + +This update should succeed, but it fails because `foo` is incorrectly printed with a `use bar baz` statement, which +causes references to `bar.baz` to be captured by its locally-bound `baz`. + +```ucm:error +scratch/main> update +``` diff --git a/unison-src/transcripts/fix-5464.output.md b/unison-src/transcripts/fix-5464.output.md new file mode 100644 index 0000000000..76755a8147 --- /dev/null +++ b/unison-src/transcripts/fix-5464.output.md @@ -0,0 +1,107 @@ +``` ucm +scratch/main> builtins.merge lib.builtin + + Done. + +``` +``` unison +foo : Nat +foo = + baz = bar.baz + bar.baz + 19 + +bar.baz : Nat +bar.baz = 20 + +qux : Nat +qux = foo + foo +``` + +``` 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`: + + bar.baz : Nat + foo : Nat + qux : Nat + +``` +``` ucm +scratch/main> add + + ⍟ I've added these definitions: + + bar.baz : Nat + foo : Nat + qux : Nat + +``` +``` unison +foo : Nat +foo = + baz = bar.baz + bar.baz + 20 + +bar.baz : Nat +bar.baz = 20 +``` + +``` 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: + + ⊡ Previously added definitions will be ignored: bar.baz + + ⍟ These names already exist. You can `update` them to your + new definition: + + foo : Nat + +``` +This update should succeed, but it fails because `foo` is incorrectly printed with a `use bar baz` statement, which +causes references to `bar.baz` to be captured by its locally-bound `baz`. + +``` 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 = + use Nat + + use bar baz + baz = baz + baz + 20 + +bar.baz : Nat +bar.baz = 20 + +-- The definitions below no longer typecheck with the changes above. +-- Please fix the errors and try `update` again. + +qux : Nat +qux = + use Nat + + foo + foo + +``` + From f37d8449a503284f1345702c9315aae34325d745 Mon Sep 17 00:00:00 2001 From: Mitchell Dalvi Rosen Date: Mon, 25 Nov 2024 14:44:58 -0500 Subject: [PATCH 2/2] bugfix: don't consider shortening variables with use statements --- .../src/Unison/Syntax/TermPrinter.hs | 1 - unison-src/transcripts/fix-5464.md | 6 ++-- unison-src/transcripts/fix-5464.output.md | 29 +++---------------- 3 files changed, 7 insertions(+), 29 deletions(-) diff --git a/parser-typechecker/src/Unison/Syntax/TermPrinter.hs b/parser-typechecker/src/Unison/Syntax/TermPrinter.hs index f506467a39..e516fb404a 100644 --- a/parser-typechecker/src/Unison/Syntax/TermPrinter.hs +++ b/parser-typechecker/src/Unison/Syntax/TermPrinter.hs @@ -1285,7 +1285,6 @@ instance Monoid PrintAnnotation where suffixCounterTerm :: (Var v) => PrettyPrintEnv -> Set Name -> Set Name -> Term2 v at ap v a -> PrintAnnotation suffixCounterTerm n usedTm usedTy = \case - Var' v -> countHQ mempty $ HQ.unsafeFromVar v Ref' r -> countHQ usedTm $ PrettyPrintEnv.termName n (Referent.Ref r) Constructor' r | noImportRefs (r ^. ConstructorReference.reference_) -> mempty Constructor' r -> countHQ usedTm $ PrettyPrintEnv.termName n (Referent.Con r CT.Data) diff --git a/unison-src/transcripts/fix-5464.md b/unison-src/transcripts/fix-5464.md index 61bb5f3297..2bec8ec9e3 100644 --- a/unison-src/transcripts/fix-5464.md +++ b/unison-src/transcripts/fix-5464.md @@ -29,9 +29,9 @@ bar.baz : Nat bar.baz = 20 ``` -This update should succeed, but it fails because `foo` is incorrectly printed with a `use bar baz` statement, which -causes references to `bar.baz` to be captured by its locally-bound `baz`. +This update used to fail because `foo` would incorrectly print with a `use bar baz` statement, which caused references +to `bar.baz` to be captured by its locally-bound `baz`. -```ucm:error +```ucm scratch/main> update ``` diff --git a/unison-src/transcripts/fix-5464.output.md b/unison-src/transcripts/fix-5464.output.md index 76755a8147..aa1c7aa6ea 100644 --- a/unison-src/transcripts/fix-5464.output.md +++ b/unison-src/transcripts/fix-5464.output.md @@ -68,8 +68,8 @@ bar.baz = 20 foo : Nat ``` -This update should succeed, but it fails because `foo` is incorrectly printed with a `use bar baz` statement, which -causes references to `bar.baz` to be captured by its locally-bound `baz`. +This update used to fail because `foo` would incorrectly print with a `use bar baz` statement, which caused references +to `bar.baz` to be captured by its locally-bound `baz`. ``` ucm scratch/main> update @@ -79,29 +79,8 @@ scratch/main> update 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. + Everything typechecks, so I'm saving the results... -``` -``` unison :added-by-ucm scratch.u -foo : Nat -foo = - use Nat + - use bar baz - baz = baz + baz - 20 - -bar.baz : Nat -bar.baz = 20 - --- The definitions below no longer typecheck with the changes above. --- Please fix the errors and try `update` again. - -qux : Nat -qux = - use Nat + - foo + foo + Done. ``` -