Avoid unnecessary recompilation due to -haddock

Due to unprincipled adding and removing the `-haddock` flag during
compilation and recompilation checking, we were performing more work
than necessary.
We avoid this by compiling everything with `-haddock` by default. This
is safe nowadays, we have essentially been doing this for many releases,
and know this is fine.
For the occasion where we actually want to parse without the `-haddock`
flag, we keep explicitly disabling it.

We enable `-haddock` flag during session loading, since we already
perform a number of DynFlags tweaks.
This behaviour is dependent on the `OptHaddockParse` opton, which can,
currently, only be modified at compile-time.

Author: fendor

ghcide/session-loader/Development/IDE/Session.hs

@@ -451,6 +451,7 @@ loadSessionWithOptions recorder SessionLoadingOptions{..} rootDir que = do
     IdeOptions{ optTesting = IdeTesting optTesting
               , optCheckProject = getCheckProject
               , optExtensions
+              , optHaddockParse
               } <- getIdeOptions
 
         -- populate the knownTargetsVar with all the
@@ -495,7 +496,7 @@ loadSessionWithOptions recorder SessionLoadingOptions{..} rootDir que = do
         packageSetup (hieYaml, cfp, opts, libDir) = do
           -- Parse DynFlags for the newly discovered component
           hscEnv <- emptyHscEnv ideNc libDir
-          newTargetDfs <- evalGhcEnv hscEnv $ setOptions cfp opts (hsc_dflags hscEnv) rootDir
+          newTargetDfs <- evalGhcEnv hscEnv $ setOptions optHaddockParse cfp opts (hsc_dflags hscEnv) rootDir
           let deps = componentDependencies opts ++ maybeToList hieYaml
           dep_info <- getDependencyInfo deps
           -- Now lookup to see whether we are combining with an existing HscEnv
@@ -1107,12 +1108,13 @@ addUnit unit_str = liftEwM $ do
 
 -- | Throws if package flags are unsatisfiable
 setOptions :: GhcMonad m
-    => NormalizedFilePath
+    => OptHaddockParse
+    -> NormalizedFilePath
     -> ComponentOptions
     -> DynFlags
     -> FilePath -- ^ root dir, see Note [Root Directory]
     -> m (NonEmpty (DynFlags, [GHC.Target]))
-setOptions cfp (ComponentOptions theOpts compRoot _) dflags rootDir = do
+setOptions haddockOpt cfp (ComponentOptions theOpts compRoot _) dflags rootDir = do
     ((theOpts',_errs,_warns),units) <- processCmdLineP unit_flags [] (map noLoc theOpts)
     case NE.nonEmpty units of
       Just us -> initMulti us
@@ -1173,6 +1175,7 @@ setOptions cfp (ComponentOptions theOpts compRoot _) dflags rootDir = do
               dontWriteHieFiles $
               setIgnoreInterfacePragmas $
               setBytecodeLinkerOptions $
+              enableOptHaddock haddockOpt $
               disableOptimisation $
               Compat.setUpTypedHoles $
               makeDynFlagsAbsolute compRoot -- makeDynFlagsAbsolute already accounts for workingDirectory
@@ -1186,6 +1189,14 @@ setIgnoreInterfacePragmas df =
 disableOptimisation :: DynFlags -> DynFlags
 disableOptimisation df = updOptLevel 0 df
 
+-- | We always compile with '-haddock' unless explicitly disabled.
+--
+-- This avoids inconsistencies when doing recompilation checking which was
+-- observed in https://github.com/haskell/haskell-language-server/issues/4511
+enableOptHaddock :: OptHaddockParse -> DynFlags -> DynFlags
+enableOptHaddock HaddockParse d = gopt_set d Opt_Haddock
+enableOptHaddock NoHaddockParse d = d
+
 setHiDir :: FilePath -> DynFlags -> DynFlags
 setHiDir f d =
     -- override user settings to avoid conflicts leading to recompilation

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

@@ -260,12 +260,10 @@ getParsedModuleRule recorder =
     let ms = ms' { ms_hspp_opts = modify_dflags $ ms_hspp_opts ms' }
         reset_ms pm = pm { pm_mod_summary = ms' }
 
-    -- We still parse with Haddocks whether Opt_Haddock is True or False to collect information
-    -- but we no longer need to parse with and without Haddocks separately for above GHC90.
-    liftIO $ (fmap.fmap.fmap) reset_ms $ getParsedModuleDefinition hsc opt file (withOptHaddock ms)
+    liftIO $ (fmap.fmap.fmap) reset_ms $ getParsedModuleDefinition hsc opt file ms
 
-withOptHaddock :: ModSummary -> ModSummary
-withOptHaddock = withOption Opt_Haddock
+withoutOptHaddock :: ModSummary -> ModSummary
+withoutOptHaddock = withoutOption Opt_Haddock
 
 withOption :: GeneralFlag -> ModSummary -> ModSummary
 withOption opt ms = ms{ms_hspp_opts= gopt_set (ms_hspp_opts ms) opt}
@@ -284,7 +282,7 @@ getParsedModuleWithCommentsRule recorder =
     ModSummaryResult{msrModSummary = ms, msrHscEnv = hsc} <- use_ GetModSummary file
     opt <- getIdeOptions
 
-    let ms' = withoutOption Opt_Haddock $ withOption Opt_KeepRawTokenStream ms
+    let ms' = withoutOptHaddock $ withOption Opt_KeepRawTokenStream ms
     modify_dflags <- getModifyDynFlags dynFlagsModifyParser
     let ms'' = ms' { ms_hspp_opts = modify_dflags $ ms_hspp_opts ms' }
         reset_ms pm = pm { pm_mod_summary = ms' }
@@ -973,7 +971,7 @@ regenerateHiFile sess f ms compNeeded = do
     opt <- getIdeOptions
 
     -- Embed haddocks in the interface file
-    (diags, mb_pm) <- liftIO $ getParsedModuleDefinition hsc opt f (withOptHaddock ms)
+    (diags, mb_pm) <- liftIO $ getParsedModuleDefinition hsc opt f ms
     case mb_pm of
         Nothing -> return (diags, Nothing)
         Just pm -> do

ghcide/src/Development/IDE/Types/Options.hs

@@ -69,9 +69,7 @@ data IdeOptions = IdeOptions
     -- ^ When to typecheck reverse dependencies of a file
   , optHaddockParse       :: OptHaddockParse
     -- ^ Whether to return result of parsing module with Opt_Haddock.
-    --   Otherwise, return the result of parsing without Opt_Haddock, so
-    --   that the parsed module contains the result of Opt_KeepRawTokenStream,
-    --   which might be necessary for hlint.
+    --   Otherwise, return the result of parsing without Opt_Haddock.
   , optModifyDynFlags     :: Config -> DynFlagsModifications
     -- ^ Will be called right after setting up a new cradle,
     --   allowing to customize the Ghc options used