Opened 3 years ago

Last modified 8 months ago

#7277 new bug

Recompilation check fails for TH unless functions are inlined

Reported by: orenbenkiki Owned by:
Priority: normal Milestone: 7.12.1
Component: Template Haskell Version: 7.4.2
Keywords: Cc: hackage.haskell.org@…, nh2
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: Incorrect result at runtime Test Case:
Blocked By: Blocking:
Related Tickets: 481 Differential Revisions:

Description

Even though Issue 481 is marked as closed, the fix is only partial. If a function inside $( ... ) is not inlined, then the dependency check fails. Attached is a full example, built around the following code:

#ifdef NO_INLINE
{-# NOINLINE libraryFunction #-}
#endif

libraryFunction :: Q Exp
libraryFunction = [e| "Return X" |]

And the test:

case_library_function = $( libraryFunction ) @?= "Return A"

Here is are the relevant parts of RUNME.LOG:

+ rm -rf dist
+ cabal configure --enable-tests
Resolving dependencies...
Configuring dependency-bug-0.0.1...
+ sed -e 's/Return ./Return A/' -i src/Dependency/Library.hs
+ cabal build
Building dependency-bug-0.0.1...
Preprocessing library dependency-bug-0.0.1...
[1 of 1] Compiling Dependency.Library ( src/Dependency/Library.hs, dist/build/Dependency/Library.o )
[1 of 1] Compiling Dependency.Library ( src/Dependency/Library.hs, dist/build/Dependency/Library.p_o )
Registering dependency-bug-0.0.1...
Preprocessing test suite 'Test' for dependency-bug-0.0.1...
[1 of 1] Compiling Main             ( test/Main.hs, dist/build/Test/Test-tmp/Main.o )
Linking dist/build/Test/Test ...
+ cabal test
Running 1 test suites...
Test suite Test: RUNNING...
Test suite Test: PASS
Test suite logged to: dist/test/dependency-bug-0.0.1-Test.log
1 of 1 test suites (1 of 1 test cases) passed.
+ sed -e 's/Return ./Return B/' -i src/Dependency/Library.hs
+ cabal build
Building dependency-bug-0.0.1...
Preprocessing library dependency-bug-0.0.1...
[1 of 1] Compiling Dependency.Library ( src/Dependency/Library.hs, dist/build/Dependency/Library.o )
[1 of 1] Compiling Dependency.Library ( src/Dependency/Library.hs, dist/build/Dependency/Library.p_o )
Registering dependency-bug-0.0.1...
Preprocessing test suite 'Test' for dependency-bug-0.0.1...
[1 of 1] Compiling Main             ( test/Main.hs, dist/build/Test/Test-tmp/Main.o )
Linking dist/build/Test/Test ...
+ cabal test
Running 1 test suites...
Test suite Test: RUNNING...
Main:
  library function: [Failed]
expected: "Return A"
 but got: "Return B"

         Test Cases  Total
 Passed  0           0
 Failed  1           1
 Total   1           1
Test suite Test: FAIL
Test suite logged to: dist/test/dependency-bug-0.0.1-Test.log
0 of 1 test suites (0 of 1 test cases) passed.

This is as expected; we changed the source to "Return B" but the test expects the library to "Return A" so it fails. So far, so good. However:

+ rm -rf dist
+ cabal configure --enable-tests -fwithout-inline
Resolving dependencies...
Configuring dependency-bug-0.0.1...
+ sed -e 's/Return ./Return A/' -i src/Dependency/Library.hs
+ cabal build
Building dependency-bug-0.0.1...
Preprocessing library dependency-bug-0.0.1...
[1 of 1] Compiling Dependency.Library ( src/Dependency/Library.hs, dist/build/Dependency/Library.o )
[1 of 1] Compiling Dependency.Library ( src/Dependency/Library.hs, dist/build/Dependency/Library.p_o )
Registering dependency-bug-0.0.1...
Preprocessing test suite 'Test' for dependency-bug-0.0.1...
[1 of 1] Compiling Main             ( test/Main.hs, dist/build/Test/Test-tmp/Main.o )
Loading package ghc-prim ... linking ... done.
Loading package integer-gmp ... linking ... done.
Loading package base ... linking ... done.
Loading package array-0.4.0.0 ... linking ... done.
Loading package deepseq-1.3.0.0 ... linking ... done.
Loading package containers-0.4.2.1 ... linking ... done.
Loading package pretty-1.1.1.0 ... linking ... done.
Loading package template-haskell ... linking ... done.
Loading package dependency-bug-0.0.1 ... linking ... done.
Loading package filepath-1.3.0.0 ... linking ... done.
Loading package old-locale-1.0.0.4 ... linking ... done.
Loading package old-time-1.1.0.0 ... linking ... done.
Loading package bytestring-0.9.2.1 ... linking ... done.
Loading package unix-2.5.1.1 ... linking ... done.
Loading package directory-1.1.0.2 ... linking ... done.
Loading package cpphs-1.14 ... linking ... done.
Loading package haskell-src-exts-1.13.5 ... linking ... done.
Loading package transformers-0.3.0.0 ... linking ... done.
Loading package mtl-2.1.2 ... linking ... done.
Loading package regex-base-0.93.2 ... linking ... done.
Loading package regex-posix-0.95.2 ... linking ... done.
Loading package language-haskell-extract-0.2.1 ... linking ... done.
Loading package ansi-terminal-0.5.5 ... linking ... done.
Loading package ansi-wl-pprint-0.6.4 ... linking ... done.
Loading package extensible-exceptions-0.1.1.4 ... linking ... done.
Loading package hostname-1.0 ... linking ... done.
Loading package time-1.4 ... linking ... done.
Loading package random-1.0.1.1 ... linking ... done.
Loading package text-0.11.2.3 ... linking ... done.
Loading package xml-1.3.12 ... linking ... done.
Loading package test-framework-0.6.1 ... linking ... done.
Loading package test-framework-th-0.2.2 ... linking ... done.
Loading package HUnit-1.2.5.1 ... linking ... done.
Loading package test-framework-hunit-0.2.7 ... linking ... done.
Linking dist/build/Test/Test ...
+ cabal test
Running 1 test suites...
Test suite Test: RUNNING...
Test suite Test: PASS
Test suite logged to: dist/test/dependency-bug-0.0.1-Test.log
1 of 1 test suites (1 of 1 test cases) passed.
+ sed -e 's/Return ./Return B/' -i src/Dependency/Library.hs
+ cabal build
Building dependency-bug-0.0.1...
Preprocessing library dependency-bug-0.0.1...
[1 of 1] Compiling Dependency.Library ( src/Dependency/Library.hs, dist/build/Dependency/Library.o )
[1 of 1] Compiling Dependency.Library ( src/Dependency/Library.hs, dist/build/Dependency/Library.p_o )
Registering dependency-bug-0.0.1...
Preprocessing test suite 'Test' for dependency-bug-0.0.1...
Linking dist/build/Test/Test ...
+ cabal test
Running 1 test suites...
Test suite Test: RUNNING...
Test suite Test: PASS
Test suite logged to: dist/test/dependency-bug-0.0.1-Test.log
1 of 1 test suites (1 of 1 test cases) passed.

As you can see, without-inline the 2nd build did not recompile the test, even though it should have. It only re-linked the test, so it kept seeing "Return A" even though the library actually does "Return B".

In my real code, there are no NO/INLINE pragmas; however, the TH code is complex enough that it is not inlined, which causes the same problem. I had to use the pragma to keep the example short.

Attachments (1)

dependency-bug.tgz (1.9 KB) - added by orenbenkiki 3 years ago.
Sample project gzipped tar file

Download all attachments as: .zip

Change History (10)

Changed 3 years ago by orenbenkiki

Sample project gzipped tar file

comment:1 Changed 3 years ago by simonpj

  • difficulty set to Unknown
  • Milestone set to 7.6.2
  • Priority changed from normal to high

Thanks. We should fix this for 7.8 or even 7.6.2.

comment:2 Changed 3 years ago by simonmar

Yes, good bug. I think this is the result of an unsafe optimisation in the dependency checker: it currently assumes that if none of the modules below the current module were recompiled, then the current module does not need to be recompiled, even if it uses TH. However this ignores the fact that other packages may have changed, and we have no way to detect that (the hash only records ABI changes, not semantic changes).

So I'll have to back off from this optimisation and always recompile a module that uses TH. However I can imagine this is going to result in complaints from users that things get recompiled even when obviously nothing has changed. Perhaps we could record a hash of the object code for a package or something? Ideas anyone?

comment:3 Changed 3 years ago by orenbenkiki

Off the top of my head, each module should have two hashes. One for the interface and one for the implementation. The interface-hash would depend only on the module itself (I think). The implementation-hash would depend on the implementation-hash of all recursively used modules.

Any module would need to be recompiled if the interface-hash of a used module changes. A module that does use TH would also need to be recompiled if the implementation-hash of any module reached from within $( ...) has changed.

Something like (typing off the bat, excuse typos etc.):

moduleInterfaceHash :: Module -> Hash
moduleLocalImplementationHash :: Module -> Hash
moduleUsedModules :: Module -> [ Module ]
moduleUsedModulesTH :: Module -> [ Module ]
combineHashes :: [ Hash ] -> Hash

moduleImplementationHash :: Module -> Hash
moduleImplementationHash module = combineHashes $ localHash : usedHashes
 where
   localHash = moduleLocalImplementationHash module
   usedHahses = map moduleImplementationHash $ moduleUsedModules module

moduleDependencyHash :: Module -> Hash
moduleDependencyHash module = combineHashes $ interfacesHashes ++ implementationHashes
  where
    interfacesHashes = map moduleInterfaceHash $ moduleUsedModules module
    implementationHashes = map moduleImplementationHash $ moduleUsedModulesTH

And the module should be recompiled if the moduleDependencyHash changes.

Or something like that...?

comment:4 Changed 2 years ago by liyang

  • Cc hackage.haskell.org@… added

comment:5 Changed 14 months ago by thoughtpolice

  • Priority changed from high to normal

Lowering priority (these tickets are assigned to older versions, so they're getting bumped as they've been around for a while).

comment:6 Changed 14 months ago by thoughtpolice

  • Milestone changed from 7.6.2 to 7.10.1

Moving to 7.10.1.

comment:7 Changed 9 months ago by thomie

I'm able to reproduce this with 7.8.3, by first doing cabal install test-framework test-framework-hunit test-framework-th and then running the RUNME script inside the tar file from comment:1. The fourth and final test should be failing instead of passing.

comment:8 Changed 9 months ago by nh2

  • Cc nh2 added

comment:9 Changed 8 months ago by thoughtpolice

  • Milestone changed from 7.10.1 to 7.12.1

Moving to 7.12.1 milestone; if you feel this is an error and should be addressed sooner, please move it back to the 7.10.1 milestone.

Note: See TracTickets for help on using tickets.