Fix recompilation checking of pure plugins
authorDaniel Gröber <dxld@darkboxed.org>
Tue, 11 Dec 2018 23:36:00 +0000 (18:36 -0500)
committerBen Gamari <ben@smart-cactus.org>
Thu, 13 Dec 2018 04:25:02 +0000 (23:25 -0500)
Previously when switching from using a Plugin with
`RecompMaybe`/`ForceRecompile` in `pluginRecompile` to a Plugin with
`NoForceRecompile` GHC would never even consider recompiling.

However the previously active plugin could have modified the
compilation output so we should recompile.

Test Plan: validate

Reviewers: bgamari, mpickering

Subscribers: mpickering, rwbarton, carter

GHC Trac Issues: #15858

Differential Revision: https://phabricator.haskell.org/D5299

compiler/iface/MkIface.hs
compiler/main/DynFlags.hs
docs/users_guide/extending_ghc.rst
testsuite/tests/plugins/T15858.script [new file with mode: 0644]
testsuite/tests/plugins/T15858.stderr [new file with mode: 0644]
testsuite/tests/plugins/all.T

index c23f577..1ea608e 100644 (file)
@@ -1325,14 +1325,15 @@ checkVersions hsc_env mod_summary iface
 -- | Check if any plugins are requesting recompilation
 checkPlugins :: HscEnv -> ModIface -> IfG RecompileRequired
 checkPlugins hsc iface = liftIO $ do
-  -- [(ModuleName, Plugin, [Opts])]
+  new_fingerprint <- fingerprintPlugins hsc
   let old_fingerprint = mi_plugin_hash iface
-  res <- mconcat <$> mapM pluginRecompile' (plugins (hsc_dflags hsc))
-  return (pluginRecompileToRecompileRequired old_fingerprint res)
+  pr <- mconcat <$> mapM pluginRecompile' (plugins (hsc_dflags hsc))
+  return $
+    pluginRecompileToRecompileRequired old_fingerprint new_fingerprint pr
 
 fingerprintPlugins :: HscEnv -> IO Fingerprint
 fingerprintPlugins hsc_env = do
-  fingerprintPlugins' $ plugins(hsc_dflags hsc_env)
+  fingerprintPlugins' $ plugins (hsc_dflags hsc_env)
 
 fingerprintPlugins' :: [PluginWithArgs] -> IO Fingerprint
 fingerprintPlugins' plugins = do
@@ -1346,13 +1347,49 @@ fingerprintPlugins' plugins = do
       (MaybeRecompile fp) -> fp
 
 
-pluginRecompileToRecompileRequired :: Fingerprint -> PluginRecompile -> RecompileRequired
-pluginRecompileToRecompileRequired old_fp pr =
-  case pr of
-    NoForceRecompile -> UpToDate
-    ForceRecompile   -> RecompBecause "Plugin forced recompilation"
-    MaybeRecompile fp  -> if fp == old_fp then UpToDate
-                                          else RecompBecause "Plugin fingerprint changed"
+pluginRecompileToRecompileRequired
+    :: Fingerprint -> Fingerprint -> PluginRecompile -> RecompileRequired
+pluginRecompileToRecompileRequired old_fp new_fp pr
+  | old_fp == new_fp =
+    case pr of
+      NoForceRecompile  -> UpToDate
+
+      -- we already checked the fingerprint above so a mismatch is not possible
+      -- here, remember that: `fingerprint (MaybeRecomp x) == x`.
+      MaybeRecompile _  -> UpToDate
+
+      -- when we have an impure plugin in the stack we have to unconditionally
+      -- recompile since it might integrate all sorts of crazy IO results into
+      -- its compilation output.
+      ForceRecompile    -> RecompBecause "Impure plugin forced recompilation"
+
+  | old_fp `elem` magic_fingerprints ||
+    new_fp `elem` magic_fingerprints
+    -- The fingerprints do not match either the old or new one is a magic
+    -- fingerprint. This happens when non-pure plugins are added for the first
+    -- time or when we go from one recompilation strategy to another: (force ->
+    -- no-force, maybe-recomp -> no-force, no-force -> maybe-recomp etc.)
+    --
+    -- For example when we go from from ForceRecomp to NoForceRecomp
+    -- recompilation is triggered since the old impure plugins could have
+    -- changed the build output which is now back to normal.
+    = RecompBecause "Plugins changed"
+
+  | otherwise =
+    let reason = "Plugin fingerprint changed" in
+    case pr of
+      -- even though a plugin is forcing recompilation the fingerprint changed
+      -- which would cause recompilation anyways so we report the fingerprint
+      -- change instead.
+      ForceRecompile   -> RecompBecause reason
+
+      _                -> RecompBecause reason
+
+ where
+   magic_fingerprints =
+       [ fingerprintString "NoForceRecompile"
+       , fingerprintString "ForceRecompile"
+       ]
 
 
 -- | Check if an hsig file needs recompilation because its
index be347d9..02ad366 100644 (file)
@@ -2590,6 +2590,12 @@ setComponentId s d =
 addPluginModuleName :: String -> DynFlags -> DynFlags
 addPluginModuleName name d = d { pluginModNames = (mkModuleName name) : (pluginModNames d) }
 
+clearPluginModuleNames :: DynFlags -> DynFlags
+clearPluginModuleNames d =
+    d { pluginModNames = []
+      , pluginModNameOpts = []
+      , cachedPlugins = [] }
+
 addPluginModuleNameOption :: String -> DynFlags -> DynFlags
 addPluginModuleNameOption optflag d = d { pluginModNameOpts = (mkModuleName m, option) : (pluginModNameOpts d) }
   where (m, rest) = break (== ':') optflag
@@ -3488,6 +3494,7 @@ dynamic_flags_deps = [
         ------ Plugin flags ------------------------------------------------
   , make_ord_flag defGhcFlag "fplugin-opt" (hasArg addPluginModuleNameOption)
   , make_ord_flag defGhcFlag "fplugin"     (hasArg addPluginModuleName)
+  , make_ord_flag defGhcFlag "fclear-plugins" (noArg clearPluginModuleNames)
   , make_ord_flag defGhcFlag "ffrontend-opt" (hasArg addFrontendPluginOption)
 
         ------ Optimisation flags ------------------------------------------
index 20d9674..a913684 100644 (file)
@@ -193,10 +193,11 @@ with ``-fexternal-interpreter`` let GHC developers know in :ghc-ticket:`14335`.
 Using compiler plugins
 ~~~~~~~~~~~~~~~~~~~~~~
 
-Plugins can be specified on the command line with the
-:ghc-flag:`-fplugin=⟨module⟩` option where ⟨module⟩ is a
-module in a registered package that exports a plugin. Arguments can be given to
-plugins with the :ghc-flag:`-fplugin-opt=⟨module⟩:⟨args⟩` option.
+Plugins can be added on the command line with the :ghc-flag:`-fplugin=⟨module⟩`
+option where ⟨module⟩ is a module in a registered package that exports the
+plugin. Arguments can be passed to the plugins with the
+:ghc-flag:`-fplugin-opt=⟨module⟩:⟨args⟩` option. The list of enabled plugins can
+be reset with the :ghc-flag:`-fclear-plugins` option.
 
 .. ghc-flag:: -fplugin=⟨module⟩
     :shortdesc: Load a plugin exported by a given module
@@ -215,6 +216,16 @@ plugins with the :ghc-flag:`-fplugin-opt=⟨module⟩:⟨args⟩` option.
     Give arguments to a plugin module; module must be specified with
     :ghc-flag:`-fplugin=⟨module⟩`.
 
+.. ghc-flag:: -fclear-plugins
+    :shortdesc: Clear the list of active plugins
+    :type: dynamic
+    :category: plugins
+
+    Clear the list of plugins previously specified with
+    :ghc-flag:`-fplugin`. This is useful in GHCi where simply removing the
+    :ghc-flag:`-fplugin` options from the command line is not possible. Instead
+    `:set -fclear-plugins` can be used.
+
 
 As an example, in order to load the plugin exported by ``Foo.Plugin`` in
 the package ``foo-ghc-plugin``, and give it the parameter "baz", we
diff --git a/testsuite/tests/plugins/T15858.script b/testsuite/tests/plugins/T15858.script
new file mode 100644 (file)
index 0000000..77a1b53
--- /dev/null
@@ -0,0 +1,33 @@
+:set -fobject-code
+-- ^ Without this no recompilation happens at all in ghci, but that's a bug for
+-- another ticket.
+
+:l plugin-recomp-test.hs
+
+-- switching to an impure plugin triggers recomp.
+:! echo ==ImpurePlugin.0 >&2
+:set -fclear-plugins -fplugin ImpurePlugin
+:l plugin-recomp-test.hs
+
+-- ..forever, this also triggers recomp.
+:! echo ==ImpurePlugin.1 >&2
+:l plugin-recomp-test.hs
+
+-- switching from impure to pure plugin triggers recomp.
+:! echo ==PurePlugin.0 >&2
+:set -fclear-plugins -fplugin PurePlugin
+:l plugin-recomp-test.hs
+
+-- switching to a fingerprint plugin triggers recomp.
+:! echo ==FingerprintPlugin.0 >&2
+:set -fclear-plugins -fplugin FingerprintPlugin
+:l plugin-recomp-test.hs
+
+-- same fingerprint plugin doesn't trigger recomp.
+:! echo ==FingerprintPlugin.1 >&2
+:l plugin-recomp-test.hs
+
+-- switching from fingerprint to pure plugin triggers recomp.
+:! echo ==PurePlugin.1 >&2
+:set -fclear-plugins -fplugin PurePlugin
+:l plugin-recomp-test.hs
diff --git a/testsuite/tests/plugins/T15858.stderr b/testsuite/tests/plugins/T15858.stderr
new file mode 100644 (file)
index 0000000..db0610f
--- /dev/null
@@ -0,0 +1,21 @@
+==ImpurePlugin.0
+Simple Plugin Passes Queried
+Got options:
+Simple Plugin Pass Run
+==ImpurePlugin.1
+Simple Plugin Passes Queried
+Got options:
+Simple Plugin Pass Run
+==PurePlugin.0
+Simple Plugin Passes Queried
+Got options:
+Simple Plugin Pass Run
+==FingerprintPlugin.0
+Simple Plugin Passes Queried
+Got options:
+Simple Plugin Pass Run
+==FingerprintPlugin.1
+==PurePlugin.1
+Simple Plugin Passes Queried
+Got options:
+Simple Plugin Pass Run
\ No newline at end of file
index 72a42da..3b03166 100644 (file)
@@ -199,3 +199,11 @@ test('static-plugins',
       extra_run_opts('"' + config.libdir + '"')],
      compile_and_run,
      ['-package ghc -isimple-plugin/'])
+
+test('T15858',
+     [extra_files(['plugin-recomp/', 'plugin-recomp-test.hs']),
+#      only_ways([config.ghc_plugin_way]),
+      pre_cmd('$MAKE -s --no-print-directory -C plugin-recomp package.plugins01 TOP={top}'),
+      extra_hc_opts("-package-db plugin-recomp/pkg.plugins01/local.package.conf ")
+      ],
+     ghci_script, ['T15858.script'])