Skip to content

Commit 8a1f3d4

Browse files
committed
fix: remove the unsafePerformIO and head usage
The cache is now stored inside the `ShakeExtra`. I'm unsure that's the right location for it. This should fix hlint. Also, not having a cache stored as top level entry may makes it not survive session reinit, which seams sane (for example, during tests). Still 3 `-boot` related tests are failing. 2 are however flaky.
1 parent 949bbd1 commit 8a1f3d4

File tree

2 files changed

+18
-10
lines changed

2 files changed

+18
-10
lines changed

ghcide/src/Development/IDE/Core/Rules.hs

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -178,10 +178,9 @@ import System.Info.Extra (isWindows)
178178
import qualified Data.IntMap as IM
179179
import GHC.Fingerprint
180180
import qualified Data.Map.Strict as Map
181-
import System.FilePath (takeExtension, takeFileName, normalise, dropTrailingPathSeparator, dropExtension, splitDirectories)
181+
import System.FilePath (takeExtension, takeFileName, normalise, dropExtension, splitDirectories)
182182
import Data.Char (isUpper)
183183
import System.Directory.Extra (listFilesInside)
184-
import System.IO.Unsafe
185184

186185
data Log
187186
= LogShake Shake.Log
@@ -625,15 +624,13 @@ getModuleGraphRule recorder = defineEarlyCutOffNoFile (cmapWithPrio LogShake rec
625624
fs <- toKnownFiles <$> useNoFile_ GetKnownTargets
626625
dependencyInfoForFiles (HashSet.toList fs)
627626

628-
{-# NOINLINE cacheVar #-}
629-
-- TODO: this should not use unsaferPerformIO
630-
cacheVar = unsafePerformIO (newTVarIO mempty)
631-
632627
getModulesPathsRule :: Recorder (WithPriority Log) -> Rules ()
633628
getModulesPathsRule recorder = defineEarlyCutoff (cmapWithPrio LogShake recorder) $ Rule $ \GetModulesPaths file -> do
634629
env_eq <- use_ GhcSession file
635630

636-
cache <- liftIO (readTVarIO cacheVar)
631+
ShakeExtras{moduleToPathCache} <- getShakeExtras
632+
633+
cache <- liftIO (readTVarIO moduleToPathCache)
637634
case Map.lookup (envUnique env_eq) cache of
638635
Just res -> pure (mempty, ([], Just res))
639636
Nothing -> do
@@ -646,7 +643,9 @@ getModulesPathsRule recorder = defineEarlyCutoff (cmapWithPrio LogShake recorder
646643
(unzip -> (a, b)) <- flip mapM import_dirs $ \(u, dyn) -> do
647644
(unzip -> (a, b)) <- flip mapM (importPaths dyn) $ \dir' -> do
648645
let dir = normalise dir'
649-
let predicate path = pure (normalise path == dir || isUpper (head (takeFileName path)))
646+
let predicate path = pure (normalise path == dir || case takeFileName path of
647+
[] -> False
648+
(x:_) -> isUpper x)
650649
let dir_number_directories = length (splitDirectories dir)
651650
let toModule file = mkModuleName (intercalate "." $ drop dir_number_directories (splitDirectories (dropExtension file)))
652651

@@ -662,7 +661,7 @@ getModulesPathsRule recorder = defineEarlyCutoff (cmapWithPrio LogShake recorder
662661
pure (fmap (u,) $ mconcat a, fmap (u, ) $ mconcat b)
663662

664663
let res = (mconcat a, mconcat b)
665-
liftIO $ atomically $ modifyTVar' cacheVar (Map.insert (envUnique env_eq) res)
664+
liftIO $ atomically $ modifyTVar' moduleToPathCache (Map.insert (envUnique env_eq) res)
666665

667666
pure (mempty, ([], Just res))
668667

ghcide/src/Development/IDE/Core/Shake.hs

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ import Development.IDE.Core.WorkerThread
135135
import Development.IDE.GHC.Compat (NameCache,
136136
NameCacheUpdater,
137137
initNameCache,
138-
knownKeyNames)
138+
knownKeyNames, ModuleName, UnitId)
139139
import Development.IDE.GHC.Orphans ()
140140
import Development.IDE.Graph hiding (ShakeValue,
141141
action)
@@ -178,6 +178,7 @@ import System.FilePath hiding (makeRelative)
178178
import System.IO.Unsafe (unsafePerformIO)
179179
import System.Time.Extra
180180
import UnliftIO (MonadUnliftIO (withRunInIO))
181+
import Data.Map.Strict (Map)
181182

182183

183184
data Log
@@ -310,6 +311,10 @@ data ShakeExtras = ShakeExtras
310311
,ideNc :: NameCache
311312
-- | A mapping of module name to known target (or candidate targets, if missing)
312313
,knownTargetsVar :: TVar (Hashed KnownTargets)
314+
,moduleToPathCache :: TVar (Map
315+
Unique
316+
(Map ModuleName (UnitId, NormalizedFilePath),
317+
Map ModuleName (UnitId, NormalizedFilePath)))
313318
-- | A mapping of exported identifiers for local modules. Updated on kick
314319
,exportsMap :: TVar ExportsMap
315320
-- | A work queue for actions added via 'runInShakeSession'
@@ -704,6 +709,8 @@ shakeOpen recorder lspEnv defaultConfig idePlugins debouncer
704709
dirtyKeys <- newTVarIO mempty
705710
-- Take one VFS snapshot at the start
706711
vfsVar <- newTVarIO =<< vfsSnapshot lspEnv
712+
713+
moduleToPathCache <- newTVarIO mempty
707714
pure ShakeExtras{shakeRecorder = recorder, ..}
708715
shakeDb <-
709716
shakeNewDatabase
@@ -1481,3 +1488,5 @@ runWithSignal msgStart msgEnd files rule = do
14811488
kickSignal testing lspEnv files msgStart
14821489
void $ uses rule files
14831490
kickSignal testing lspEnv files msgEnd
1491+
1492+

0 commit comments

Comments
 (0)