Ticket #4809 (closed bug: fixed)

Opened 2 years ago

Last modified 2 years ago

MonoLocalBinds and type classes cause infinite loop

Reported by: JeremyShaw Owned by:
Priority: normal Milestone:
Component: Compiler Version: 7.0.1
Keywords: Cc: dsf@…, niklas.broberg@…, clifford.beshers@…
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: Incorrect result at runtime Difficulty:
Test Case: typecheck/should_run/T4809 Blocked By:
Blocking: Related Tickets:

Description

The following program gets stuck in a loop and prints no output when run:

{-# LANGUAGE MonoLocalBinds #-}
module Main where

import IdentityT (IdentityT(..), XML, runIdentityT)
import XMLGenerator (XMLGenT(..), XMLGen(genElement), Child, EmbedAsChild(..), unXMLGenT)
import System.IO (BufferMode(..), hSetBuffering, stdout)

page :: XMLGenT (IdentityT IO) XML
page = genElement (Nothing, "ul") [] [ asChild (asChild "foo")]
    where
--       item :: XMLGenT (IdentityT IO) [Child (IdentityT IO)]                                                                                                                          
       item = (asChild $ asChild (return "bar" :: XMLGenT (IdentityT IO) String))

main :: IO ()
main =
    do hSetBuffering stdout LineBuffering
       r <- runIdentityT (unXMLGenT page)
       print r

I believe this is due to a compiler bug. There are five things you can do to make it run successfully (i.e., not loop) -- none of which ought to have an effect. Doing any one of the five works though.

Note that 'item' in the 'page' where clause is not actually used anywhere, but three of the five things involve changes to that clause.

1. remove the 'where' clause in 'page' entirely. After all it is not needed.

2. uncomment the type signature for 'item'

3. remove one of the calls to 'asChild' in item.

4. remove one of the calls to 'asChild' in page.

5. remove the MonoLocalBinds? pragma.

I considered the possibility that the loop might be cause by asChild calls forming a loop depending on how the types are inferred. However, each call to asChild does a putStrLn. Since there is no output when the loop occurs, I believe that the execution is not even getting that far.

Doing --dump-ds, does show the loop. But I could figure out anything useful from seeing the loop in the desugared code.

The looping seems to occur whether compiled or using GHCi and at all optimization levels.

This bug prevents HSP, and therefore Happstack and SeeReason? from moving to GHC 7.

I have attached 3 files. The above file, IdentityT.hs and XMLGenerator.hs. The latter two come from HSP/Happstack, but have been stripped down to the bare essentials.

Attachments

Main.hs Download (0.6 KB) - added by JeremyShaw 2 years ago.
Main
XMLGenerator.hs Download (2.8 KB) - added by JeremyShaw 2 years ago.
stripped down version of HSX required by Main
IdentityT.hs Download (1.5 KB) - added by JeremyShaw 2 years ago.
instance of XMLGen for IdentityT required by Main

Change History

Changed 2 years ago by JeremyShaw

Main

Changed 2 years ago by JeremyShaw

stripped down version of HSX required by Main

Changed 2 years ago by JeremyShaw

instance of XMLGen for IdentityT required by Main

Changed 2 years ago by dsf

  • cc dsf@… added

Changed 2 years ago by nibro

  • cc niklas.broberg@… added

Changed 2 years ago by cliffordbeshers

  • cc clifford.beshers@… added

Changed 2 years ago by dsf

Does anyone have any thoughts about this bug?

Changed 2 years ago by simonpj

I've been meaning to add a note. THANK YOU to Jeremy for boiling out a very tricky example for us. Dimitrios and I sat down last week and figured out exactly what is going on. We're working on a patch; will be in 7.0.2. This will be fixed!

Simon

Changed 2 years ago by simonpj

  • status changed from new to merge
  • testcase set to typecheck/should_run/T4809

I believe we have now, finally, fixed this.

Mon Dec 13 09:15:11 PST 2010  simonpj@microsoft.com
  * Fix recursive superclasses (again).  Fixes Trac #4809.
  
  This patch finally deals with the super-delicate question of
  superclases in possibly-recursive dictionaries.  The key idea
  is the DFun Superclass Invariant (see TcInstDcls):
  
       In the body of a DFun, every superclass argument to the
       returned dictionary is
         either   * one of the arguments of the DFun,
         or       * constant, bound at top level
  
  To establish the invariant, we add new "silent" superclass
  argument(s) to each dfun, so that the dfun does not do superclass
  selection internally.  There's a bit of hoo-ha to make sure that
  we don't print those silent arguments in error messages; a knock
  on effect was a change in interface-file format.
  
  A second change is that instead of the complex and fragile
  "self dictionary binding" in TcInstDcls and TcClassDcl,
  using the same mechanism for existential pattern bindings.
  See Note [Subtle interaction of recursion and overlap] in TcInstDcls
  and Note [Binding when looking up instances] in InstEnv.
  
  Main notes are here:
  
    * Note [Silent Superclass Arguments] in TcInstDcls,
      including the DFun Superclass Invariant
  
  Main code changes are:
  
    * The code for MkId.mkDictFunId and mkDictFunTy
  
    * DFunUnfoldings get a little more complicated;
      their arguments are a new type DFunArg (in CoreSyn)
  
    * No "self" argument in tcInstanceMethod
    * No special tcSimplifySuperClasss
    * No "dependents" argument to EvDFunApp
  
  IMPORTANT
     It turns out that it's quite tricky to generate the right
     DFunUnfolding for a specialised dfun, when you use SPECIALISE
     INSTANCE.  For now I've just commented it out (in DsBinds) but
     that'll lose some optimisation, and I need to get back to
     this.

    M ./compiler/basicTypes/Id.lhs -3 +8
    M ./compiler/basicTypes/IdInfo.lhs -7 +14
    M ./compiler/basicTypes/MkId.lhs -12 +24
    M ./compiler/coreSyn/CoreFVs.lhs -1 +1
    M ./compiler/coreSyn/CoreSubst.lhs -1 +3
    M ./compiler/coreSyn/CoreSyn.lhs -9 +23
    M ./compiler/coreSyn/CoreTidy.lhs -1 +1
    M ./compiler/coreSyn/CoreUnfold.lhs -4 +6
    M ./compiler/coreSyn/CoreUtils.lhs -1 +1
    M ./compiler/coreSyn/PprCore.lhs -2 +6
    M ./compiler/deSugar/DsBinds.lhs -29 +15
    M ./compiler/hsSyn/HsBinds.lhs -4 +1
    M ./compiler/hsSyn/HsExpr.lhs-boot +4
    M ./compiler/hsSyn/HsPat.lhs-boot +1
    M ./compiler/iface/BinIface.hs -2 +13
    M ./compiler/iface/IfaceSyn.lhs -8 +7
    M ./compiler/iface/MkIface.lhs -2 +2
    M ./compiler/iface/TcIface.lhs -3 +6
    M ./compiler/main/TidyPgm.lhs -1 +1
    M ./compiler/simplCore/Simplify.lhs -1 +1
    M ./compiler/typecheck/Inst.lhs -3 +11
    M ./compiler/typecheck/TcClassDcl.lhs -17 +7
    M ./compiler/typecheck/TcDeriv.lhs -21 +22
    M ./compiler/typecheck/TcEnv.lhs -2 +2
    M ./compiler/typecheck/TcErrors.lhs -9 +10
    M ./compiler/typecheck/TcHsSyn.lhs -2 +2
    M ./compiler/typecheck/TcHsType.lhs -28 +30
    M ./compiler/typecheck/TcInstDcls.lhs -143 +178
    M ./compiler/typecheck/TcInteract.lhs -3 +2
    M ./compiler/typecheck/TcMType.lhs -49 +34
    M ./compiler/typecheck/TcSMonad.lhs -5 +3
    M ./compiler/typecheck/TcSimplify.lhs -115 +5
    M ./compiler/typecheck/TcType.lhs -5 +8
    M ./compiler/typecheck/TcUnify.lhs -7 +5
    M ./compiler/types/InstEnv.lhs -8 +18
    M ./compiler/vectorise/Vectorise/Type/PADict.hs -1 +2

This needs to merge to stable.

Simon

Changed 2 years ago by igloo

  • status changed from merge to closed
  • resolution set to fixed

Merged

Changed 2 years ago by dsf

I have now built a compiler with this patch and built and successfully run one of our HSP/Happstack applications. Sweet!

Changed 2 years ago by simonpj

Happy days!

Dimitrios and I are still not happy with our solution so we plan to re-jig it again. Testing again after we've done that (a week or so) would be helpful. Thanks.

Changed 2 years ago by simonpj

See also #4370, #3731, which are other examples of the same loopy behaviour.

Note: See TracTickets for help on using tickets.