Skip to content

Commit

Permalink
Fix positionMapping in stale data
Browse files Browse the repository at this point in the history
  • Loading branch information
soulomoon committed Dec 25, 2023
1 parent 74466a9 commit b1f1feb
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 4 deletions.
5 changes: 5 additions & 0 deletions ghcide/src/Development/IDE/Core/PositionMapping.hs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ module Development.IDE.Core.PositionMapping
, toCurrentPosition
, PositionDelta(..)
, addDelta
, addOldDelta
, idDelta
, composeDelta
, mkDelta
Expand Down Expand Up @@ -123,6 +124,10 @@ mkDelta cs = foldl' applyChange idDelta cs
addDelta :: PositionDelta -> PositionMapping -> PositionMapping
addDelta delta (PositionMapping pm) = PositionMapping (composeDelta delta pm)

-- | Add a old delta onto a Mapping k n to make a Mapping (k - 1) n
addOldDelta :: PositionDelta -> PositionMapping -> PositionMapping
addOldDelta delta (PositionMapping pm) = PositionMapping (composeDelta pm delta)

-- TODO: We currently ignore the right hand side (if there is only text), as
-- that was what was done with lsp* 1.6 packages
applyChange :: PositionDelta -> TextDocumentContentChangeEvent -> PositionDelta
Expand Down
8 changes: 4 additions & 4 deletions ghcide/src/Development/IDE/Core/Shake.hs
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ data ShakeExtras = ShakeExtras
-- ^ Map from a text document version to a PositionMapping that describes how to map
-- positions in a version of that document to positions in the latest version
-- First mapping is delta from previous version and second one is an
-- accumulation of all previous mappings.
-- accumulation to the current version.
,progress :: ProgressReporting
,ideTesting :: IdeTesting
-- ^ Whether to enable additional lsp messages used by the test suite for checking invariants
Expand Down Expand Up @@ -443,7 +443,7 @@ lastValueIO s@ShakeExtras{positionMapping,persistentKeys,state} k file = do
`catch` (\(_ :: IOException) -> pure Nothing)
atomicallyNamed "lastValueIO 2" $ do
STM.focus (Focus.alter (alterValue $ Stale (Just del) actual_version (toDyn v))) (toKey k file) state
Just . (v,) . addDelta del <$> mappingForVersion positionMapping file actual_version
Just . (v,) . addOldDelta del <$> mappingForVersion positionMapping file actual_version

-- We got a new stale value from the persistent rule, insert it in the map without affecting diagnostics
alterValue new Nothing = Just (ValueWithDiagnostics new mempty) -- If it wasn't in the map, give it empty diagnostics
Expand All @@ -459,7 +459,7 @@ lastValueIO s@ShakeExtras{positionMapping,persistentKeys,state} k file = do
Succeeded ver (fromDynamic -> Just v) ->
atomicallyNamed "lastValueIO 5" $ Just . (v,) <$> mappingForVersion positionMapping file ver
Stale del ver (fromDynamic -> Just v) ->
atomicallyNamed "lastValueIO 6" $ Just . (v,) . maybe id addDelta del <$> mappingForVersion positionMapping file ver
atomicallyNamed "lastValueIO 6" $ Just . (v,) . maybe id addOldDelta del <$> mappingForVersion positionMapping file ver
Failed p | not p -> readPersistent
_ -> pure Nothing

Expand Down Expand Up @@ -1357,7 +1357,7 @@ updatePositionMapping IdeState{shakeExtras = ShakeExtras{positionMapping}} Versi
-- Very important to use mapAccum here so that the tails of
-- each mapping can be shared, otherwise quadratic space is
-- used which is evident in long running sessions.
EM.mapAccumRWithKey (\acc _k (delta, _) -> let new = addDelta delta acc in (new, (delta, acc)))
EM.mapAccumRWithKey (\acc _k (delta, _) -> let new = addOldDelta delta acc in (new, (delta, acc)))
zeroMapping
(EM.insert _version (shared_change, zeroMapping) mappingForUri)
shared_change = mkDelta changes

0 comments on commit b1f1feb

Please sign in to comment.