Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bugfix: fix suffixification logic in update #5311

Merged
merged 2 commits into from
Aug 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions unison-cli/src/Unison/Codebase/Editor/HandleInput/Update2.hs
Original file line number Diff line number Diff line change
Expand Up @@ -325,16 +325,18 @@ makePPE ::
Names ->
DefnsF (Map Name) TermReferenceId TypeReferenceId ->
PrettyPrintEnvDecl
makePPE hashLen names initialFileNames dependents =
makePPE hashLen namespaceNames initialFileNames dependents =
PPED.addFallback
(PPED.makeFilePPED (initialFileNames <> Names.fromUnconflictedReferenceIds dependents))
( let names = initialFileNames <> Names.fromUnconflictedReferenceIds dependents
in PPED.makePPED (PPE.namer names) (PPE.suffixifyByName (Names.shadowing names namespaceNames))
)
( PPED.makePPED
(PPE.hqNamer hashLen names)
(PPE.hqNamer hashLen namespaceNames)
-- We don't want to over-suffixify for a reference in the namespace. For example, say we have "foo.bar" in the
-- namespace and "oink.bar" in the file. "bar" may be a unique suffix among the namespace names, but would be
-- ambiguous in the context of namespace + file names.
--
-- So, we use `shadowing`, which starts with the LHS names (the namespace), and adds to it names from the
-- RHS (the initial file names, i.e. what was originally saved) that don't already exist in the LHS.
(PPE.suffixifyByHash (Names.shadowing names initialFileNames))
(PPE.suffixifyByHash (Names.shadowing namespaceNames initialFileNames))
)
24 changes: 24 additions & 0 deletions unison-src/transcripts/fix-5276.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
```ucm
scratch/main> builtins.merge lib.builtin
```

```unison
x = 17

a.y = 18
b.y = x + 1

c = b.y + 1
```

```ucm
scratch/main> add
```

```unison
x = 100
```

```ucm
scratch/main> update
```
73 changes: 73 additions & 0 deletions unison-src/transcripts/fix-5276.output.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
``` ucm
scratch/main> builtins.merge lib.builtin

Done.

```
``` unison
x = 17

a.y = 18
b.y = x + 1

c = b.y + 1
```

``` 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`:

a.y : Nat
b.y : Nat
c : Nat
x : Nat

```
``` ucm
scratch/main> add

⍟ I've added these definitions:

a.y : Nat
b.y : Nat
c : Nat
x : Nat

```
``` unison
x = 100
```

``` 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 names already exist. You can `update` them to your
new definition:

x : Nat

```
``` 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...

Everything typechecks, so I'm saving the results...

Done.

```
Loading