Skip to content

Commit

Permalink
separate partitioning from asserting no builtins
Browse files Browse the repository at this point in the history
  • Loading branch information
mitchellwrosen committed Aug 5, 2024
1 parent b946980 commit 325e4ee
Show file tree
Hide file tree
Showing 5 changed files with 45 additions and 39 deletions.
9 changes: 6 additions & 3 deletions unison-cli/src/Unison/Codebase/Editor/HandleInput/Merge2.hs
Original file line number Diff line number Diff line change
Expand Up @@ -336,9 +336,12 @@ doMerge info = do
liftIO (debugFunctions.debugCombinedDiff diff)

-- Partition the combined diff into the conflicted things and the unconflicted things
(conflicts, unconflicts) <-
Merge.partitionCombinedDiffs (ThreeWay.forgetLca defns3) declNameLookups diff & onLeft \name ->
done (Output.MergeConflictInvolvingBuiltin name)
(conflicts, unconflicts) <- do
let (conflicts0, unconflicts) = Merge.partitionCombinedDiffs (ThreeWay.forgetLca defns3) declNameLookups diff
conflicts <-
Merge.narrowConflictsToNonBuiltins conflicts0 & onLeft \name ->
done (Output.MergeConflictInvolvingBuiltin name)
pure (conflicts, unconflicts)

liftIO (debugFunctions.debugPartitionedDiff conflicts unconflicts)

Expand Down
2 changes: 1 addition & 1 deletion unison-cli/src/Unison/Codebase/Editor/Output.hs
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,7 @@ data Output
| MergeSuccess !MergeSourceAndTarget
| MergeSuccessFastForward !MergeSourceAndTarget
| MergeConflictedAliases !MergeSourceOrTarget !(Defn (Name, Name) (Name, Name))
| MergeConflictInvolvingBuiltin !Name
| MergeConflictInvolvingBuiltin !(Defn Name Name)
| MergeDefnsInLib !MergeSourceOrTarget
| InstalledLibdep !(ProjectAndBranch ProjectName ProjectBranchName) !NameSegment
| NoUpgradeInProgress
Expand Down
37 changes: 21 additions & 16 deletions unison-cli/src/Unison/CommandLine/OutputMessages.hs
Original file line number Diff line number Diff line change
Expand Up @@ -1368,22 +1368,27 @@ notifyUser dir = \case
<> P.newline
<> P.newline
<> P.wrap "and then try merging again."
MergeConflictInvolvingBuiltin name ->
pure . P.lines $
[ P.wrap "Sorry, I wasn't able to perform the merge:",
"",
P.wrap
( "There's a merge conflict on"
<> P.group (prettyName name <> ",")
<> "but it's a builtin on one or both branches. I can't yet handle merge conflicts involving builtins."
),
"",
P.wrap
( "Please eliminate this conflict by updating one branch or the other, making"
<> prettyName name
<> "the same on both branches, or making neither of them a builtin, and then try the merge again."
)
]
MergeConflictInvolvingBuiltin defn ->
let (isTerm, name) =
case defn of
TermDefn n -> (True, n)
TypeDefn n -> (False, n)
in pure . P.lines $
[ P.wrap "Sorry, I wasn't able to perform the merge:",
"",
P.wrap
( "There's a merge conflict on"
<> (if isTerm then "term" else "type")
<> P.group (prettyName name <> ",")
<> "but it's a builtin on one or both branches. I can't yet handle merge conflicts involving builtins."
),
"",
P.wrap
( "Please eliminate this conflict by updating one branch or the other, making"
<> prettyName name
<> "the same on both branches, or making neither of them a builtin, and then try the merge again."
)
]
-- Note [DefnsInLibMessage] If you change this, also change the other similar one
MergeDefnsInLib aliceOrBob ->
pure . P.lines $
Expand Down
3 changes: 2 additions & 1 deletion unison-merge/src/Unison/Merge.hs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ module Unison.Merge
-- * Partitioning combined namespace diffs
Unconflicts (..),
partitionCombinedDiffs,
narrowConflictsToNonBuiltins,

-- * Merging libdeps
LibdepDiffOp (..),
Expand Down Expand Up @@ -56,7 +57,7 @@ import Unison.Merge.EitherWayI (EitherWayI (..))
import Unison.Merge.FindConflictedAlias (findConflictedAlias)
import Unison.Merge.Libdeps (LibdepDiffOp (..), applyLibdepsDiff, diffLibdeps)
import Unison.Merge.PartialDeclNameLookup (PartialDeclNameLookup (..))
import Unison.Merge.PartitionCombinedDiffs (partitionCombinedDiffs)
import Unison.Merge.PartitionCombinedDiffs (narrowConflictsToNonBuiltins, partitionCombinedDiffs)
import Unison.Merge.Synhashed (Synhashed (..))
import Unison.Merge.ThreeWay (ThreeWay (..))
import Unison.Merge.TwoOrThreeWay (TwoOrThreeWay (..))
Expand Down
33 changes: 15 additions & 18 deletions unison-merge/src/Unison/Merge/PartitionCombinedDiffs.hs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
module Unison.Merge.PartitionCombinedDiffs
( partitionCombinedDiffs,
narrowConflictsToNonBuiltins,
)
where

Expand Down Expand Up @@ -27,6 +28,7 @@ import Unison.Referent (Referent)
import Unison.Referent qualified as Referent
import Unison.Util.BiMultimap (BiMultimap)
import Unison.Util.BiMultimap qualified as BiMultimap
import Unison.Util.Defn (Defn (..))
import Unison.Util.Defns (Defns (..), DefnsF, DefnsF2, defnsAreEmpty)
import Unison.Util.Map qualified as Map

Expand All @@ -35,16 +37,12 @@ partitionCombinedDiffs ::
TwoWay (Defns (BiMultimap Referent Name) (BiMultimap TypeReference Name)) ->
TwoWay DeclNameLookup ->
DefnsF2 (Map Name) CombinedDiffOp Referent TypeReference ->
Either
Name
( TwoWay (DefnsF (Map Name) TermReferenceId TypeReferenceId),
DefnsF Unconflicts Referent TypeReference
)
partitionCombinedDiffs defns declNameLookups diffs = do
let conflicts0 = identifyConflicts declNameLookups defns diffs
let unconflicts = identifyUnconflicts declNameLookups conflicts0 diffs
conflicts <- assertThereAreNoBuiltins conflicts0
Right (conflicts, unconflicts)
( TwoWay (DefnsF (Map Name) TermReference TypeReference),
DefnsF Unconflicts Referent TypeReference
)
partitionCombinedDiffs defns declNameLookups diffs =
let conflicts = identifyConflicts declNameLookups defns diffs
in (conflicts, identifyUnconflicts declNameLookups conflicts diffs)

data S = S
{ me :: !(EitherWay ()),
Expand Down Expand Up @@ -247,21 +245,20 @@ justTheConflictedNames =
CombinedDiffOp'Delete _ -> names
CombinedDiffOp'Update _ -> names

assertThereAreNoBuiltins ::
narrowConflictsToNonBuiltins ::
TwoWay (DefnsF (Map Name) TermReference TypeReference) ->
Either Name (TwoWay (DefnsF (Map Name) TermReferenceId TypeReferenceId))
assertThereAreNoBuiltins =
Either (Defn Name Name) (TwoWay (DefnsF (Map Name) TermReferenceId TypeReferenceId))
narrowConflictsToNonBuiltins =
traverse (bitraverse (Map.traverseWithKey assertTermIsntBuiltin) (Map.traverseWithKey assertTypeIsntBuiltin))
where
assertTermIsntBuiltin :: Name -> TermReference -> Either Name TermReferenceId
assertTermIsntBuiltin :: Name -> TermReference -> Either (Defn Name Name) TermReferenceId
assertTermIsntBuiltin name ref =
case Reference.toId ref of
Nothing -> Left name
Nothing -> Left (TermDefn name)
Just refId -> Right refId

-- Same body as above, but could be different some day (e.g. return value tells you what namespace)
assertTypeIsntBuiltin :: Name -> TypeReference -> Either Name TypeReferenceId
assertTypeIsntBuiltin :: Name -> TypeReference -> Either (Defn Name Name) TypeReferenceId
assertTypeIsntBuiltin name ref =
case Reference.toId ref of
Nothing -> Left name
Nothing -> Left (TypeDefn name)
Just refId -> Right refId

0 comments on commit 325e4ee

Please sign in to comment.