Skip to content

Fix a wingman bug caused by mismanaged stale data #1657

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

Merged
merged 23 commits into from
Apr 6, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
bd33819
Start tracking provenance of stale data
isovector Apr 3, 2021
489f216
Add some machinery for automagically updating the age
isovector Apr 3, 2021
4398610
Add an applicative instance
isovector Apr 3, 2021
8fb6be7
Tracked ages makes everything much easier to reason about
isovector Apr 3, 2021
a648a25
Formatting
isovector Apr 3, 2021
f1b1c49
Haddock and small changes
isovector Apr 4, 2021
4b77f7f
Update haddock on IdeAction
isovector Apr 4, 2021
2285f68
Update to lsp-1.2 (#1631)
wz1000 Apr 1, 2021
d43a087
Avoid reordering plugins (#1629)
pepeiborra Apr 2, 2021
f22718f
Civilized indexing progress reporting (#1633)
pepeiborra Apr 2, 2021
00240fa
Do not override custom user commands (#1650)
pepeiborra Apr 3, 2021
f687a27
Shut the Shake session on exit, instead of restarting it (#1655)
pepeiborra Apr 3, 2021
80662f2
Fix importing type operators (#1644)
berberman Apr 3, 2021
5217a04
log exceptions before killing the server (#1651)
pepeiborra Apr 3, 2021
594e31b
additional .gitignore entries (#1659)
pepeiborra Apr 3, 2021
de03ac2
Fix ignore paths (#1656)
jneira Apr 4, 2021
f6da637
Add bounds for Diff (#1665)
berberman Apr 4, 2021
80122ec
Replace Barrier with MVar in lsp main (#1668)
berberman Apr 4, 2021
3028692
Port UseStale to ghcide
isovector Apr 5, 2021
9a191f3
Use the new ghcide UseStale machinery
isovector Apr 5, 2021
f877af0
Merge branch 'master' into no-stale-ranges
isovector Apr 5, 2021
d54cccf
Fix hlint complaints
isovector Apr 5, 2021
abfd7f8
Merge branch 'master' into no-stale-ranges
mergify[bot] Apr 6, 2021
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
3 changes: 3 additions & 0 deletions ghcide/ghcide.cabal
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,8 @@ library
BangPatterns
DeriveFunctor
DeriveGeneric
DeriveFoldable
DeriveTraversable
FlexibleContexts
GeneralizedNewtypeDeriving
LambdaCase
Expand Down Expand Up @@ -149,6 +151,7 @@ library
Development.IDE.Core.Service
Development.IDE.Core.Shake
Development.IDE.Core.Tracing
Development.IDE.Core.UseStale
Development.IDE.GHC.Compat
Development.IDE.Core.Compile
Development.IDE.GHC.Error
Expand Down
1 change: 1 addition & 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
, PositionDelta(..)
, addDelta
, idDelta
, composeDelta
, mkDelta
, toCurrentRange
, fromCurrentRange
Expand Down
8 changes: 5 additions & 3 deletions ghcide/src/Development/IDE/Core/Shake.hs
Original file line number Diff line number Diff line change
Expand Up @@ -838,12 +838,14 @@ usesWithStale_ key files = do
Nothing -> liftIO $ throwIO $ BadDependency (show key)
Just v -> return v

newtype IdeAction a = IdeAction { runIdeActionT :: (ReaderT ShakeExtras IO) a }
deriving newtype (MonadReader ShakeExtras, MonadIO, Functor, Applicative, Monad)

-- | IdeActions are used when we want to return a result immediately, even if it
-- is stale Useful for UI actions like hover, completion where we don't want to
-- block.
--
-- Run via 'runIdeAction'.
newtype IdeAction a = IdeAction { runIdeActionT :: (ReaderT ShakeExtras IO) a }
deriving newtype (MonadReader ShakeExtras, MonadIO, Functor, Applicative, Monad)

runIdeAction :: String -> ShakeExtras -> IdeAction a -> IO a
runIdeAction _herald s i = runReaderT (runIdeActionT i) s

Expand Down
153 changes: 153 additions & 0 deletions ghcide/src/Development/IDE/Core/UseStale.hs
Original file line number Diff line number Diff line change
@@ -0,0 +1,153 @@
{-# LANGUAGE DerivingVia #-}
{-# LANGUAGE GADTs #-}
{-# LANGUAGE KindSignatures #-}
{-# LANGUAGE RankNTypes #-}

module Development.IDE.Core.UseStale
( Age(..)
, Tracked
, unTrack
, PositionMap
, TrackedStale (..)
, unsafeMkStale
, unsafeMkCurrent
, unsafeCopyAge
, MapAge (..)
, dualPositionMap
, useWithStale
, useWithStale_
) where

import Control.Arrow
import Control.Category (Category)
import qualified Control.Category as C
import Control.DeepSeq (NFData)
import Data.Aeson
import Data.Coerce (coerce)
import Data.Functor ((<&>))
import Data.Functor.Identity (Identity(Identity))
import Data.Kind (Type)
import Data.String (fromString)
import Development.IDE (NormalizedFilePath, IdeRule, Action, Range, rangeToRealSrcSpan, realSrcSpanToRange)
import qualified Development.IDE.Core.PositionMapping as P
import qualified Development.IDE.Core.Shake as IDE
import qualified FastString as FS
import SrcLoc


------------------------------------------------------------------------------
-- | A data kind for 'Tracked'.
data Age = Current | Stale Type


------------------------------------------------------------------------------
-- | Some value, tagged with its age. All 'Current' ages are considered to be
-- the same thing, but 'Stale' values are protected by an untouchable variable
-- to ensure they can't be unified.
newtype Tracked (age :: Age) a = UnsafeTracked
{ unTrack :: a
}
deriving stock (Functor, Foldable, Traversable)
deriving newtype (Eq, Ord, Show, Read, ToJSON, FromJSON, NFData)
deriving (Applicative, Monad) via Identity


------------------------------------------------------------------------------
-- | Like 'P.PositionMapping', but with annotated ages for how 'Tracked' values
-- change. Use the 'Category' instance to compose 'PositionMapping's in order
-- to transform between values of different stale ages.
newtype PositionMap (from :: Age) (to :: Age) = PositionMap
{ getPositionMapping :: P.PositionMapping
}

instance Category PositionMap where
id = coerce P.zeroMapping
(.) = coerce P.composeDelta


------------------------------------------------------------------------------
-- | Get a 'PositionMap' that runs in the opposite direction.
dualPositionMap :: PositionMap from to -> PositionMap to from
dualPositionMap (PositionMap (P.PositionMapping (P.PositionDelta from to))) =
PositionMap $ P.PositionMapping $ P.PositionDelta to from


------------------------------------------------------------------------------
-- | A pair containing a @'Tracked' 'Stale'@ value, as well as
-- a 'PositionMapping' that will fast-forward values to the current age.
data TrackedStale a where
TrackedStale
:: Tracked (Stale s) a
-> PositionMap (Stale s) Current
-> TrackedStale a

instance Functor TrackedStale where
fmap f (TrackedStale t pm) = TrackedStale (fmap f t) pm


------------------------------------------------------------------------------
-- | A class for which 'Tracked' values can be run across a 'PositionMapping'
-- to change their ages.
class MapAge a where
{-# MINIMAL mapAgeFrom | mapAgeTo #-}
mapAgeFrom :: PositionMap from to -> Tracked to a -> Maybe (Tracked from a)
mapAgeFrom = mapAgeTo . dualPositionMap

mapAgeTo :: PositionMap from to -> Tracked from a -> Maybe (Tracked to a)
mapAgeTo = mapAgeFrom . dualPositionMap


instance MapAge Range where
mapAgeFrom = coerce P.fromCurrentRange
mapAgeTo = coerce P.toCurrentRange


instance MapAge RealSrcSpan where
mapAgeFrom =
invMapAge (\fs -> rangeToRealSrcSpan (fromString $ FS.unpackFS fs))
(srcSpanFile &&& realSrcSpanToRange)
. mapAgeFrom


------------------------------------------------------------------------------
-- | Helper function for deriving 'MapAge' for values in terms of other
-- instances.
invMapAge
:: (c -> a -> b)
-> (b -> (c, a))
-> (Tracked from a -> Maybe (Tracked to a))
-> Tracked from b
-> Maybe (Tracked to b)
invMapAge to from f t =
let (c, t') = unTrack $ fmap from t
in fmap (fmap $ to c) $ f $ UnsafeTracked t'


unsafeMkCurrent :: age -> Tracked 'Current age
unsafeMkCurrent = coerce


unsafeMkStale :: age -> Tracked (Stale s) age
unsafeMkStale = coerce


unsafeCopyAge :: Tracked age a -> b -> Tracked age b
unsafeCopyAge _ = coerce


-- | Request a Rule result, it not available return the last computed result, if any, which may be stale
useWithStale :: IdeRule k v
=> k -> NormalizedFilePath -> Action (Maybe (TrackedStale v))
useWithStale key file = do
x <- IDE.useWithStale key file
pure $ x <&> \(v, pm) ->
TrackedStale (coerce v) (coerce pm)

-- | Request a Rule result, it not available return the last computed result which may be stale.
-- Errors out if none available.
useWithStale_ :: IdeRule k v
=> k -> NormalizedFilePath -> Action (TrackedStale v)
useWithStale_ key file = do
(v, pm) <- IDE.useWithStale_ key file
pure $ TrackedStale (coerce v) (coerce pm)

5 changes: 3 additions & 2 deletions plugins/hls-tactics-plugin/src/Wingman/Judgements.hs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import qualified Data.Map as M
import Data.Maybe
import Data.Set (Set)
import qualified Data.Set as S
import Development.IDE.Core.UseStale (Tracked, unTrack)
import Development.IDE.Spans.LocalBindings
import OccName
import SrcLoc
Expand All @@ -22,8 +23,8 @@ import Wingman.Types

------------------------------------------------------------------------------
-- | Given a 'SrcSpan' and a 'Bindings', create a hypothesis.
hypothesisFromBindings :: RealSrcSpan -> Bindings -> Hypothesis CType
hypothesisFromBindings span bs = buildHypothesis $ getLocalScope bs span
hypothesisFromBindings :: Tracked age RealSrcSpan -> Tracked age Bindings -> Hypothesis CType
hypothesisFromBindings (unTrack -> span) (unTrack -> bs) = buildHypothesis $ getLocalScope bs span


------------------------------------------------------------------------------
Expand Down
6 changes: 4 additions & 2 deletions plugins/hls-tactics-plugin/src/Wingman/Judgements/Theta.hs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ module Wingman.Judgements.Theta
import Data.Maybe (fromMaybe, mapMaybe)
import Data.Set (Set)
import qualified Data.Set as S
import Development.IDE.Core.UseStale
import Development.IDE.GHC.Compat
import Generics.SYB hiding (tyConName)
import GhcPlugins (mkVarOcc, splitTyConApp_maybe, getTyVar_maybe)
Expand Down Expand Up @@ -50,11 +51,12 @@ mkEvidence _ = Nothing

------------------------------------------------------------------------------
-- | Compute all the 'Evidence' implicitly bound at the given 'SrcSpan'.
getEvidenceAtHole :: SrcSpan -> LHsBinds GhcTc -> [Evidence]
getEvidenceAtHole dst
getEvidenceAtHole :: Tracked age SrcSpan -> Tracked age (LHsBinds GhcTc) -> [Evidence]
getEvidenceAtHole (unTrack -> dst)
= mapMaybe mkEvidence
. (everything (<>) $
mkQ mempty (absBinds dst) `extQ` wrapperBinds dst `extQ` matchBinds dst)
. unTrack


------------------------------------------------------------------------------
Expand Down
Loading