[DO NOT MERGE] GHC 964 cabal compilation #5096

neduard · 2024-06-15T18:18:22Z

Overview

This is a proof-of-concept PR which has all the changes required to get Unison to compile with Cabal and GHC 964 - Tested only on Nixos and OpenBSD 7.5.

It is not mergeable nor do I really want to merge it . I would like to split it up and merge the backward compatible bits first instead. Let me know if you have any ideas on how we should do this 😄

Note there's a couple of packages which break backward compatibility

lsp (I've pined to 2.2.0.0 for now)
servant needs to be on 0.20 as 0.19 requires base<4.18 but GHC 964 is on base==4.18

Added self-review comments explaining my changes.

Testing

unison executable compiles, runs, serves local UI, auths and pulls from unison-share
cabal v2-test --project-file=contrib/cabal.project all passes

All testing was done by hand.

Loose ends

Do let me know if this is useful and if so, any thoughts on splitting up by hand. I can make many small PRs but I'm thinking that might also be tedious to review

neduard · 2024-06-15T18:27:58Z

codebase2/codebase-sqlite/U/Codebase/Sqlite/Patch/TermEdit.hs

@@ -22,6 +22,12 @@ type Referent' t h = Referent.Referent' (Reference' t h) (Reference' t h)
 data TermEdit' t h = Replace (Referent' t h) Typing | Deprecate
  deriving (Eq, Ord, Show)

+instance Functor (TermEdit' t) where


interestingly the default deriving doesn't work:

U/Codebase/Sqlite/Patch/TermEdit.hs:23:17: error: [GHC-16437] • Can't make a derived instance of ‘Functor (TermEdit' t)’: Constructor ‘Replace’ must use the type variable only as the last argument of a data type • In the data declaration for ‘TermEdit'’ | 23 | deriving (Eq, Functor, Ord, Show) | ^^^^^^^

~~Not sure, but I'd guess the default Functor can only figure out one type variable?~~

I dunno.

neduard · 2024-06-15T18:28:27Z

codebase2/util-term/U/Util/Type.hs

@@ -61,7 +61,7 @@ flattenEffects es = [es]
 generalize :: (Ord v) => [v] -> TypeR r v -> TypeR r v
 generalize vs t = foldr f t vs
  where
-    f v t = if Set.member v (ABT.freeVars t) then forall v t else t


Just a warning, not strictly necessary:

U/Util/Type.hs:84:1: warning: [GHC-64088] [-Wforall-identifier] The use of ‘forall’ as an identifier will become an error in a future GHC release. Suggested fix: Consider using another name, such as ‘forAll’, ‘for_all’, or ‘forall_’. | 84 | forall v body = ABT.tm () (Forall (ABT.abs () v body)) | ^^^^^^

~~Our build requires we have no warnings, so this change would require a rename at some point.~~

Sorry, I couldn't see on the Conversation page that you'd already renamed it.

neduard · 2024-06-15T18:29:47Z

lib/unison-prelude/src/Unison/Util/Tuple.hs

@@ -1,5 +1,6 @@
 {-# LANGUAGE FunctionalDependencies #-}
 {-# LANGUAGE UndecidableInstances #-}
+{-# LANGUAGE TypeOperators #-}


src/Unison/Util/Tuple.hs:14:13: error: [GHC-58520] [-Wtype-equality-requires-operators, Werror=type-equality-requires-operators] The use of ‘~’ without TypeOperators will become an error in a future GHC release. Suggested fix: Perhaps you intended to use TypeOperators | 14 | instance (x ~ (a, b, c)) => Drop4th (a, b, c, d) x where | ^

neduard · 2024-06-15T18:33:27Z

lib/unison-sqlite/src/Unison/Sqlite/Connection.hs

  logQuery (Sql sql []) Nothing
-  Direct.Sqlite.exec database sql `catch` \(exception :: Sqlite.SQLError) ->
+  Direct.Sqlite.exec (Sqlite.connectionHandle connection) sql `catch` \(exception :: Sqlite.SQLError) ->


sqlite-simple was upgraded 0.4.18.2 -> 0.4.19.0 and now has multiple entries in the record:

https://hackage.haskell.org/package/sqlite-simple-0.4.18.2/docs/src/Database.SQLite.Simple.Internal.html#connectionHandle
vs
https://hackage.haskell.org/package/sqlite-simple-0.4.19.0/docs/src/Database.SQLite.Simple.Internal.html#connectionHandle

neduard · 2024-06-15T18:36:04Z

unison-cli/src/Unison/Cli/Share/Projects.hs

-              (Servant.defaultMakeClientRequest url request)
-                { Http.Client.responseTimeout = Http.Client.responseTimeoutMicro (60 * 1000 * 1000 {- 60s -})
-                }
+          { Servant.makeClientRequest = \url request -> do


makeClientRequest in servant-client-0.20 now returns IO Request https://hackage.haskell.org/package/servant-client-0.20/docs/Servant-Client.html#v:defaultMakeClientRequest

neduard · 2024-06-15T18:39:49Z

unison-cli/src/Unison/CommandLine/Completion.hs

@@ -196,12 +196,6 @@ completeWithinNamespace compTypes query currentPath = do
    namesInBranch :: Int -> V2Branch.Branch Sqlite.Transaction -> Sqlite.Transaction [(CompletionType, Bool, Text)]
    namesInBranch hashLen b = do
      nonEmptyChildren <- V2Branch.nonEmptyChildren b
-      let textifyHQ :: (NameSegment -> r -> HQ'.HashQualified NameSegment) -> Map NameSegment (Map r metadata) -> [(Bool, Text)]


this one was weird... not sure what happens.

Without it we get this really weird error where it's expecting the wrong type! Curious if anybody has any idea what's happening 🤔

src/Unison/CommandLine/Completion.hs:213:68: error: [GHC-83865] • Couldn't match type: Reference.Reference' Text Unison.Hash.Hash with: Referent.Referent' Reference.Reference Reference.Reference Expected: NameSegment -> Referent.Referent -> HQ'.HashQualified NameSegment Actual: NameSegment -> Reference.Reference -> HQ'.HashQualified NameSegment Type synonyms expanded: Expected type: NameSegment -> Referent.Referent' Reference.Reference Reference.Reference -> HQ'.HashQualified NameSegment Actual type: NameSegment -> Reference.Reference' Text Unison.Hash.Hash -> HQ'.HashQualified NameSegment • In the first argument of ‘textifyHQ’, namely ‘(hqFromNamedV2Reference hashLen)’ In the first argument of ‘($)’, namely ‘textifyHQ (hqFromNamedV2Reference hashLen)’ In the second argument of ‘map’, namely ‘(textifyHQ (hqFromNamedV2Reference hashLen) $ V2Branch.types b)’ | 213 | (map (\(x, y) -> (TypeCompletion, x, y)) (textifyHQ (hqFromNamedV2Reference hashLen) $ V2Branch.types b)), b | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ .stack-work/install/x86_64-linux/a063c0a73e60c13f68571667e6977d457cc001cca1e544d0d7876989a155d05e/9.2.8/lib src/Unison/CommandLine/Completion.hs:213:102: error: [GHC-83865] • Couldn't match type: Reference.Reference' Text Unison.Hash.Hash with: Referent.Referent' Reference.Reference Reference.Reference Expected: Map NameSegment (Map Referent.Referent (Sqlite.Transaction V2Branch.MdValues)) Actual: Map NameSegment (Map Reference.Reference (Sqlite.Transaction V2Branch.MdValues)) Type synonyms expanded: Expected type: Map NameSegment (Map (Referent.Referent' Reference.Reference Reference.Reference) (Sqlite.Transaction V2Branch.MdValues)) Actual type: Map NameSegment (Map (Reference.Reference' Text Unison.Hash.Hash) (Sqlite.Transaction V2Branch.MdValues)) • In the second argument of ‘($)’, namely ‘V2Branch.types b’ In the second argument of ‘map’, namely ‘(textifyHQ (hqFromNamedV2Reference hashLen) $ V2Branch.types b)’ In the second argument of ‘Monoid.whenM’, namely ‘(map (\ (x, y) -> (TypeCompletion, x, y)) (textifyHQ (hqFromNamedV2Reference hashLen) $ V2Branch.types b))’ | 213 | (map (\(x, y) -> (TypeCompletion, x, y)) (textifyHQ (hqFromNamedV2Reference hashLen) $ V2Branch.types b)), | ^^^^^^^^^^^^^^^^

Yes, without the forall r., GHC infers local functions to have a concrete type according to the first encountered usage. This one must be getting used more than once with different r. Why remove the type annotation though?

Yeah so that was my thinking as well initially so I tried to add a forall r. in the type definition but it was still erroring out 🤷

Why remove the type annotation though?

Ah sorry, I should've selected the whole change when adding my comment: I just moved the whole function so here it only shows up as the removed line but if you look at the diff I re-added it a few lines below (not sure if that's what you meant?)

neduard · 2024-06-15T18:41:49Z

contrib/cabal.project

@@ -1,4 +1,5 @@
 packages:
+  ../fuzzyfind


local copy of fuzzyfind as we haven't built the new packages following the update yet

neduard · 2024-06-15T18:43:46Z

unison-cli/package.yaml

@@ -44,7 +44,7 @@ dependencies:
  - ki
  - lens
  - lock-file
-  - lsp >= 2.2.0.0


lsp has a couple of small breaking changes that we depend on:

Remove getCompletionPrefix haskell/lsp#552

Remove pointless IO from VFS haskell/lsp#523

aryairani · 2024-06-17T16:26:05Z

codebase2/codebase-sqlite/U/Codebase/Sqlite/Patch/TypeEdit.hs

@@ -17,7 +17,7 @@ type TypeEdit = TypeEdit' Db.TextId Db.ObjectId
 type HashTypeEdit = TypeEdit' Text ComponentHash

 data TypeEdit' t h = Replace (Reference' t h) | Deprecate
-  deriving (Eq, Ord, Show)
+  deriving (Eq, Functor, Ord, Show)


Why add Functor on this? part of auto-deriving something else?

Yup, if I don't add it then it errors out with:

U/Codebase/Sqlite/Patch/TypeEdit.hs:32:10: error: [GHC-39999] No instance for ‘Functor (TypeEdit' a)’ arising from the head of a quantified constraint arising from the superclasses of an instance declaration In the instance declaration for ‘Bifunctor TypeEdit'’ | 32 | instance Bifunctor TypeEdit' where | ^^^^^^^^^^^^^^^^^^^

aryairani · 2024-06-17T16:28:16Z

Well, I only did a partial review, but certainly we can incorporate any forward-compatible changes.

neduard · 2024-06-18T23:24:04Z

Hey @aryairani thanks for taking a look and for the comments - I've raised #5106 which should contain only forward-compatible changes

* trivial additions of functor * limit imports to just used functions * remove unnecesary imports

-Wforall-identifier

this caused a weird type error

Add -Werror to all others Compiles with ghc928

neduard · 2024-06-27T06:19:38Z

Closing as we've merged #5106 and #5142 is in progress . Thanks all!

neduard commented Jun 15, 2024

View reviewed changes

aryairani reviewed Jun 17, 2024

View reviewed changes

neduard mentioned this pull request Jun 18, 2024

GHC 964 Fixes #5106

Merged

neduard force-pushed the ghc964-compilation branch from 7239004 to 43d7234 Compare June 22, 2024 10:06

neduard added 5 commits June 25, 2024 09:11

Trivial fixes

d745d24

* trivial additions of functor * limit imports to just used functions * remove unnecesary imports

s/forall/forAll/ as it will become a restricted keyword

e51ceab

-Wforall-identifier

move let block outside of inner function

9d8db9b

this caused a weird type error

define non-trivial functor

8a6c7d2

Use TypeOperators

cfda33f

neduard force-pushed the ghc964-compilation branch from 43d7234 to 99b4961 Compare June 25, 2024 08:14

neduard added 5 commits June 25, 2024 09:26

fix redundant pattern match warning

4338758

Add missing cabal packages

efcbfa3

Fix sqlite-simple and lsp versions

57a86d2

Remove unnecessary ghc_options.

8ef3243

Add -Werror to all others Compiles with ghc928

breaking changes

33be385

neduard force-pushed the ghc964-compilation branch from 321a430 to 33be385 Compare June 25, 2024 08:29

neduard closed this Jun 27, 2024

neduard deleted the ghc964-compilation branch July 18, 2024 11:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DO NOT MERGE] GHC 964 cabal compilation #5096

[DO NOT MERGE] GHC 964 cabal compilation #5096

neduard commented Jun 15, 2024 •

edited

Loading

neduard Jun 15, 2024

aryairani Jun 17, 2024 •

edited

Loading

neduard Jun 15, 2024

aryairani Jun 17, 2024 •

edited

Loading

neduard Jun 15, 2024

neduard Jun 15, 2024

neduard Jun 15, 2024

neduard Jun 15, 2024

aryairani Jun 17, 2024

neduard Jun 18, 2024

neduard Jun 15, 2024

neduard Jun 15, 2024

aryairani Jun 17, 2024

neduard Jun 18, 2024

aryairani commented Jun 17, 2024

neduard commented Jun 18, 2024

neduard commented Jun 27, 2024

[DO NOT MERGE] GHC 964 cabal compilation #5096

[DO NOT MERGE] GHC 964 cabal compilation #5096

Conversation

neduard commented Jun 15, 2024 • edited Loading

Overview

Testing

Loose ends

Choose a reason for hiding this comment

aryairani Jun 17, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aryairani Jun 17, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aryairani commented Jun 17, 2024

neduard commented Jun 18, 2024

neduard commented Jun 27, 2024

neduard commented Jun 15, 2024 •

edited

Loading

aryairani Jun 17, 2024 •

edited

Loading

aryairani Jun 17, 2024 •

edited

Loading