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.

Sign up for GitHub

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

in update, don't bother typechecking again if we haven't changed the unison file #4446

Merged
merged 2 commits into from
Nov 29, 2023
Merged

in update, don't bother typechecking again if we haven't changed the unison file #4446

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
011718f
in update, don't bother typechecking again if we haven't changed the …
mitchellwrosen Nov 29, 2023
6ffe0fa
make same-unison-file check go faster
mitchellwrosen Nov 29, 2023
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
40 changes: 27 additions & 13 deletions unison-cli/src/Unison/Codebase/Editor/HandleInput/Update2.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,19 +128,33 @@ handleUpdate2 = do

pure (pped, bigUf)

-- - typecheck it
Cli.respond Output.UpdateStartTypechecking
parsingEnv <- makeParsingEnv currentPath namesIncludingLibdeps
prettyParseTypecheck bigUf pped parsingEnv >>= \case
Left prettyUf -> do
Cli.Env {isTranscript} <- ask
maybePath <- if isTranscript then pure Nothing else Just . fst <$> Cli.expectLatestFile
Cli.respond (Output.DisplayDefinitionsString maybePath prettyUf)
Cli.respond Output.UpdateTypecheckingFailure
Right tuf -> do
Cli.respond Output.UpdateTypecheckingSuccess
saveTuf (findCtorNames namesExcludingLibdeps ctorNames Nothing) tuf
Cli.respond Output.Success
-- If the new-unison-file-to-typecheck is the same as old-unison-file-that-we-already-typechecked, then don't bother
-- typechecking again.
secondTuf <- do
let smallUf = UF.discardTypes tuf
let noChanges =
and
[ UF.dataDeclarations smallUf == UF.dataDeclarations bigUf,
UF.effectDeclarations smallUf == UF.effectDeclarations bigUf,
UF.terms smallUf == UF.terms bigUf, -- no need to sort these, though it wouldn't hurt
UF.watches smallUf == UF.watches bigUf
]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like you should maybe just have the code that generates bigUf return a Bool of whether it had to pull in additional dependents.

Also seems like you could just compare sizes here. The bigUf can only have additional elements, since you don't remove anything, only add dependents.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 I changed this to just compare sizes. Making bigBuildUnisonFile return whether it changed anything is a bit more invasive to thread that state through everywhere, so I'm not sure it's worth it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No objections here, just thought I'd mention it.

if noChanges
then pure tuf
else do
Cli.respond Output.UpdateStartTypechecking
parsingEnv <- makeParsingEnv currentPath namesIncludingLibdeps
secondTuf <-
prettyParseTypecheck bigUf pped parsingEnv & onLeftM \prettyUf -> do
Cli.Env {isTranscript} <- ask
maybePath <- if isTranscript then pure Nothing else Just . fst <$> Cli.expectLatestFile
Cli.respond (Output.DisplayDefinitionsString maybePath prettyUf)
Cli.returnEarly Output.UpdateTypecheckingFailure
Cli.respond Output.UpdateTypecheckingSuccess
pure secondTuf

saveTuf (findCtorNames namesExcludingLibdeps ctorNames Nothing) secondTuf
Cli.respond Output.Success

-- TODO: find a better module for this function, as it's used in a couple places
prettyParseTypecheck ::
Expand Down
4 changes: 0 additions & 4 deletions unison-src/transcripts/addupdatemessages.output.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,10 +140,6 @@ Expected: `x` is now `2` and `X` is `Two`. UCM says the old definition was also
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...

Comment on lines -143 to -146
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like that these go away.

Done.

```
4 changes: 0 additions & 4 deletions unison-src/transcripts/fix-1381-excess-propagate.output.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,6 @@ a = "an 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.

```
Expand Down
4 changes: 0 additions & 4 deletions unison-src/transcripts/fix1356.output.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,10 +72,6 @@ Step 4: I add it and expect to see it
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.

.trunk> docs x
Expand Down
4 changes: 0 additions & 4 deletions unison-src/transcripts/merges.output.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,10 +230,6 @@ master.frobnicate n = n + 1
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.

.> view master.y
Expand Down
12 changes: 0 additions & 12 deletions unison-src/transcripts/move-namespace.output.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,6 @@ unique type a.T = T1 | T2
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.

```
Expand Down Expand Up @@ -151,10 +147,6 @@ b.termInB = 11
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.

```
Expand Down Expand Up @@ -247,10 +239,6 @@ b.termInB = 11
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.

.existing> move.namespace a b
Expand Down
4 changes: 0 additions & 4 deletions unison-src/transcripts/reset.output.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,10 +181,6 @@ foo/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.

foo/main> reset /topic
Expand Down
4 changes: 0 additions & 4 deletions unison-src/transcripts/update-ignores-lib-namespace.output.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,6 @@ foo = 200
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.

.> names foo
Expand Down
4 changes: 0 additions & 4 deletions unison-src/transcripts/update-on-conflict.output.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,10 +88,6 @@ x = 3
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.

.merged> view.patch
Expand Down
4 changes: 0 additions & 4 deletions unison-src/transcripts/update-term-aliases-in-different-ways.output.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,6 @@ bar = 7
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.

.> view foo bar
Expand Down
4 changes: 0 additions & 4 deletions unison-src/transcripts/update-term-to-different-type.output.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,6 @@ foo = +5
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.

.> view foo
Expand Down
4 changes: 0 additions & 4 deletions unison-src/transcripts/update-term-with-alias.output.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,6 @@ foo = 6
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.

.> view foo bar
Expand Down
4 changes: 0 additions & 4 deletions unison-src/transcripts/update-term.output.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,6 @@ foo = 6
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.

.> view foo
Expand Down
4 changes: 0 additions & 4 deletions unison-src/transcripts/update-test-to-non-test.output.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,6 @@ After updating `foo` to not be a test, we expect `view` to not render it like a
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.

.> view foo
Expand Down
4 changes: 0 additions & 4 deletions unison-src/transcripts/update-type-add-constructor.output.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,6 @@ unique type Foo
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.

.> view Foo
Expand Down
4 changes: 0 additions & 4 deletions unison-src/transcripts/update-type-add-field.output.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,6 @@ unique type Foo = Bar Nat Nat
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.

.> view Foo
Expand Down
4 changes: 0 additions & 4 deletions unison-src/transcripts/update-type-add-new-record.output.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,6 @@ unique type Foo = { bar : Nat }
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.

.> view Foo
Expand Down
4 changes: 0 additions & 4 deletions unison-src/transcripts/update-type-add-record-field.output.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,6 @@ unique type Foo = { bar : Nat, baz : Int }
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.

.> view Foo
Expand Down
4 changes: 0 additions & 4 deletions unison-src/transcripts/update-type-constructor-alias.output.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,6 @@ Bug: we leave `Foo.BarAlias` in the namespace with a nameless decl.
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.

.> find.verbose
Expand Down
4 changes: 0 additions & 4 deletions unison-src/transcripts/update-type-delete-constructor.output.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,6 @@ unique type Foo
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.

.> view Foo
Expand Down
4 changes: 0 additions & 4 deletions unison-src/transcripts/update-type-no-op-record.output.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,6 @@ Bug: this no-op update should (of course) succeed.
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.

```
4 changes: 0 additions & 4 deletions unison-src/transcripts/update-type-stray-constructor-alias.output.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,6 @@ Bug: we leave `Stray.BarAlias` in the namespace with a nameless decl.
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.

.> find.verbose
Expand Down
4 changes: 0 additions & 4 deletions unison-src/transcripts/update-type-turn-non-record-into-record.output.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,6 @@ unique type Foo = { bar : Nat }
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.

.> view Foo
Expand Down
Loading