driver: Consider dyn_o files when checking recompilation in -cwip/dynamic-too-recomp-c

When -dynamic-too is enabled, there are two result files, .o and .dyn_o,
therefore we should check both to decide whether to set SourceModified
or not.

The whole recompilation logic is very messy, a more thorough refactor
would be beneficial in this area but this is the minimal patch to fix
this more high priority problem.

Fixes #17968 and hopefully #17534

5 files changed, 59 insertions, 8 deletions

diff --git a/compiler/GHC/Driver/Pipeline.hs b/compiler/GHC/Driver/Pipeline.hs
index e6b7be62ef..8589b81ee5 100644
--- a/compiler/GHC/Driver/Pipeline.hs
+++ b/compiler/GHC/Driver/Pipeline.hs
@@ -1269,8 +1269,16 @@ runPhase (RealPhase (Hsc src_flavour)) input_fn
   -- the object file for one module.)
   -- Note the nasty duplication with the same computation in compileFile above
         location <- getLocation src_flavour mod_name
-
+        dt_state <- dynamicTooState dflags
         let o_file = ml_obj_file location -- The real object file
+            -- dynamic-too *also* produces the dyn_o_file, so have to check
+            -- that's there, and if it's not, regenerate both .o and
+            -- .dyn_o
+            dyn_o_file = case dt_state of
+                           DT_OK
+                            | not (writeInterfaceOnlyMode dflags)
+                              -> Just (dynamicOutputFile dflags o_file)
+                           _ -> Nothing
             hi_file = ml_hi_file location
             hie_file = ml_hie_file location
             dest_file | writeInterfaceOnlyMode dflags
@@ -1280,11 +1288,11 @@ runPhase (RealPhase (Hsc src_flavour)) input_fn
 
   -- Figure out if the source has changed, for recompilation avoidance.
   --
-  -- Setting source_unchanged to True means that M.o (or M.hie) seems
+  -- Setting source_unchanged to True means that M.o, M.dyn_o (or M.hie) seems
   -- to be up to date wrt M.hs; so no need to recompile unless imports have
   -- changed (which the compiler itself figures out).
-  -- Setting source_unchanged to False tells the compiler that M.o is out of
-  -- date wrt M.hs (or M.o doesn't exist) so we must recompile regardless.
+  -- Setting source_unchanged to False tells the compiler that M.o or M.dyn_o is out of
+  -- date wrt M.hs (or M.o/dyn_o doesn't exist) so we must recompile regardless.
         src_timestamp <- liftIO $ getModificationUTCTime (basename <.> suff)
 
         source_unchanged <- liftIO $
@@ -1295,11 +1303,12 @@ runPhase (RealPhase (Hsc src_flavour)) input_fn
              then return SourceModified
                 -- Otherwise look at file modification dates
              else do dest_file_mod <- sourceModified dest_file src_timestamp
+                     dyn_file_mod  <- traverse (flip sourceModified src_timestamp) dyn_o_file
                      hie_file_mod <- if gopt Opt_WriteHie dflags
                                         then sourceModified hie_file
                                                             src_timestamp
                                         else pure False
-                     if dest_file_mod || hie_file_mod
+                     if dest_file_mod || hie_file_mod || fromMaybe False dyn_file_mod
                         then return SourceModified
                         else return SourceUnmodified
 
diff --git a/compiler/GHC/Driver/Session.hs b/compiler/GHC/Driver/Session.hs
index 969d63a54b..9b6ee1b626 100644
--- a/compiler/GHC/Driver/Session.hs
+++ b/compiler/GHC/Driver/Session.hs
@@ -985,9 +985,8 @@ positionIndependent dflags = gopt Opt_PIC dflags || gopt Opt_PIE dflags
 -- Core optimisation, then the backend (from Core to object code) is executed
 -- twice.
 --
--- The implementation is currently rather hacky: recompilation avoidance is
--- broken (#17968), we don't clearly separate non-dynamic and dynamic loaded
--- interfaces (#9176), etc.
+-- The implementation is currently rather hacky, for example, we don't clearly separate non-dynamic
+-- and dynamic loaded interfaces (#9176).
 --
 -- To make matters worse, we automatically enable -dynamic-too when some modules
 -- need Template-Haskell and GHC is dynamically linked (cf
diff --git a/testsuite/tests/driver/dynamicToo/ARecomp.hs b/testsuite/tests/driver/dynamicToo/ARecomp.hs
new file mode 100644
index 0000000000..c760d18753
--- /dev/null
+++ b/testsuite/tests/driver/dynamicToo/ARecomp.hs
@@ -0,0 +1,3 @@
+module ARecomp where
+
+main = print ()
diff --git a/testsuite/tests/driver/dynamicToo/Makefile b/testsuite/tests/driver/dynamicToo/Makefile
index b1eab7ef30..33b6a5a9cb 100644
--- a/testsuite/tests/driver/dynamicToo/Makefile
+++ b/testsuite/tests/driver/dynamicToo/Makefile
@@ -3,6 +3,7 @@ include $(TOP)/mk/boilerplate.mk
 include $(TOP)/mk/test.mk
 
 checkExists = [ -f $1 ] || echo $1 missing
+checkMissing = [ ! -f $1 ] || echo $1 exists
 
 .PHONY: dynamicToo003
 # Check that "-c -dynamic-too" works
@@ -13,3 +14,32 @@ dynamicToo003:
 	$(call checkExists,A003.dyn_o)
 	$(call checkExists,A003.dyn_hi)
 
+.PHONY: dynamicTooRecomp
+# Check that recompilation with "-c -dynamic-too" works
+dynamicTooRecomp:
+	"$(TEST_HC)" $(TEST_HC_OPTS) -dynamic-too -v0 -c ARecomp.hs
+	# Remove just dynamic objects
+	$(RM) ARecomp.dyn*
+	# Recompile
+	"$(TEST_HC)" $(TEST_HC_OPTS) -dynamic-too -v0 -c ARecomp.hs
+	# Check everything is there
+	$(call checkExists,ARecomp.o)
+	$(call checkExists,ARecomp.hi)
+	$(call checkExists,ARecomp.dyn_o)
+	$(call checkExists,ARecomp.dyn_hi)
+
+.PHONY: dynamicTooOnlyInterface
+# Check that a missing .dyn_o does not cause recompilation when `-fno-code` `-fwrite-interface`
+# is combined
+dynamicTooOnlyInterface:
+	"$(TEST_HC)" $(TEST_HC_OPTS) -dynamic-too -v0 -c ARecomp.hs
+	# Remove just dynamic objects
+	$(RM) ARecomp.dyn*
+	# Recompile, using -fno-code -fwrite-interface
+	"$(TEST_HC)" $(TEST_HC_OPTS) -fno-code -fwrite-interface -v0 -c ARecomp.hs
+	# Check the right things are there
+	$(call checkExists,ARecomp.o)
+	$(call checkExists,ARecomp.hi)
+	$(call checkMissing,ARecomp.dyn_o)
+	$(call checkMissing,ARecomp.dyn_hi)
+
diff --git a/testsuite/tests/driver/dynamicToo/all.T b/testsuite/tests/driver/dynamicToo/all.T
index 0c890efd70..28fedc7863 100644
--- a/testsuite/tests/driver/dynamicToo/all.T
+++ b/testsuite/tests/driver/dynamicToo/all.T
@@ -3,3 +3,13 @@ test('dynamicToo003',
      [extra_files(['A003.hs']),
       unless(have_vanilla(), skip), unless(have_dynamic(), skip)],
      makefile_test, [])
+
+test('dynamicTooRecomp',
+     [extra_files(['ARecomp.hs']),
+      unless(have_vanilla(), skip), unless(have_dynamic(), skip)],
+     makefile_test, [])
+
+test('dynamicTooOnlyInterface',
+     [extra_files(['ARecomp.hs']),
+      unless(have_vanilla(), skip), unless(have_dynamic(), skip)],
+     makefile_test, [])