Opened 4 years ago

Closed 4 years ago

Last modified 5 months ago

#10078 closed bug (fixed)

tcPluginStop of a type checker plugin is not called if an error occurs

Reported by: jbracker Owned by: jbracker
Priority: normal Milestone: 7.10.1
Component: Compiler (Type checker) Version: 7.11
Keywords: TypeCheckerPlugins Cc: adamgundry, gridaphobe, diatchki
Operating System: Linux Architecture: x86_64 (amd64)
Type of failure: Incorrect result at runtime Test Case:
Blocked By: Blocking:
Related Tickets: Differential Rev(s):
Wiki Page:

Description

When a module using a type checker plugin produces a compiler error the clean up function tcPluginStop of the plugin is not called.

I am not sure if this is intended, but according to the description of the wiki page (Plugins/TypeChecker) this should always be called.

Test plugin

MyPlugin.hs:

module MyPlugin
  ( plugin ) where

import Plugins
import TcRnTypes
import TcPluginM

plugin :: Plugin
plugin = defaultPlugin 
  { tcPlugin = \clos -> Just $ TcPlugin 
    { tcPluginInit  = tcPluginIO $ putStrLn ">>> Plugin Init"
    , tcPluginSolve = \_ _ _ _ -> do
        tcPluginIO $ putStrLn ">>> Plugin Solve"
        return $ TcPluginOk [] []
    , tcPluginStop  = \_ -> tcPluginIO $ putStrLn ">>> Plugin Stop"
    }
  }

Minimal example (with type error)

Main.hs:

{-# OPTIONS_GHC -fplugin MyPlugin #-}
module Main where

main :: (Monad m) => m ()
main = do
  return 1

Compiling this will lead to the following output:

$ ~/ghc/inplace/bin/ghc-stage2 --make -package ghc-7.11.20150209 -dynamic Main.hs
[2 of 2] Compiling Main             ( Main.hs, Main.o )
>>> Plugin Init
>>> Plugin Solve
>>> Plugin Solve
>>> Plugin Solve

Main.hs:6:10:
    Could not deduce (Num ()) arising from the literal ‘1’
    from the context: Monad m
      bound by the type signature for: main :: Monad m => m ()
      at Main.hs:4:9-25
    In the first argument of ‘return’, namely ‘1’
    In a stmt of a 'do' block: return 1
    In the expression: do { return 1 }

Which means tcPluginStop was _not_ called.

Minimal example (without type error)

Main.hs:

{-# OPTIONS_GHC -fplugin MyPlugin #-}
module Main where

main :: (Monad m) => m ()
main = do
  return ()

Compiling this will lead to the following output:

$ ~/ghc/inplace/bin/ghc-stage2 --make -package ghc-7.11.20150209 -dynamic Main.hs
[2 of 2] Compiling Main             ( Main.hs, Main.o ) [MyPlugin changed]
>>> Plugin Init
>>> Plugin Solve
>>> Plugin Solve
>>> Plugin Stop
Linking Main ...

Which means tcPluginStop _was_ called.

Possible solution

As far as I can see, the solution to this should be to change the plugin code at the bottom of typechecker/TcRnDriver.hs from

withTcPlugins :: HscEnv -> TcM a -> TcM a
withTcPlugins hsc_env m =
  do plugins <- liftIO (loadTcPlugins hsc_env)
     case plugins of
       [] -> m  -- Common fast case
       _  -> do (solvers,stops) <- unzip `fmap` mapM startPlugin plugins
                res <- updGblEnv (\e -> e { tcg_tc_plugins = solvers }) m
                mapM_ runTcPluginM stops
                return res
  where
  startPlugin (TcPlugin start solve stop) =
    do s <- runTcPluginM start
       return (solve s, stop s)

to

withTcPlugins :: HscEnv -> TcM a -> TcM a
withTcPlugins hsc_env m =
  do plugins <- liftIO (loadTcPlugins hsc_env)
     case plugins of
       [] -> m  -- Common fast case
       _  -> do (solvers,stops) <- unzip `fmap` mapM startPlugin plugins
                eitherRes <- tryM $ do updGblEnv (\e -> e { tcg_tc_plugins = solvers }) m
                mapM_ runTcPluginM stops
                case eitherRes of
                  Left e -> failM
                  Right res -> return res
  where
  startPlugin (TcPlugin start solve stop) =
    do s <- runTcPluginM start
       return (solve s, stop s)

.

I have tried this. It compiles and my minimal example delivers the correct result.

Are there any arguments against this change? If not, I would try to commit a patch for this problem sometime this weekend.

Change History (7)

comment:1 Changed 4 years ago by adamgundry

Cc: gridaphobe diatchki added

Sounds fine to me. Thanks for the detailed report and for proposing a fix!

comment:2 Changed 4 years ago by gridaphobe

Good catch, thanks!

comment:3 Changed 4 years ago by jbracker

Owner: set to jbracker

comment:4 Changed 4 years ago by Austin Seipp <austin@…>

In fd581a7300abede9a070cb6e9b835b2e18f68b0b/ghc:

Fix for ticket #10078: ensure that tcPluginStop is called even in case of type errors

Summary:
Remove unused variable that appeared through the fix for ticket #10078

Merge branch 'master' of git://git.haskell.org/ghc

Added comment with bug ID.

Reviewers: adamgundry, gridaphobe, austin

Reviewed By: austin

Subscribers: thomie

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

GHC Trac Issues: #10078

comment:5 Changed 4 years ago by thoughtpolice

Milestone: 7.10.1
Status: newmerge

comment:6 Changed 4 years ago by thoughtpolice

Resolution: fixed
Status: mergeclosed

Merged to ghc-7.10 (via f163b15ce15cbe6ce19e168efde400a630cbf8b1).

comment:7 Changed 5 months ago by adamgundry

Keywords: TypeCheckerPlugins added
Note: See TracTickets for help on using tickets.