ghc cyclic import error confusing

[Evan Laforge]

Excellent, I look forward to seeing it in the next version... well, I
never look forward to seeing that particular error, but I look forward
to being able to read it better when it does come up :)

You beat me to it, but I was being slow...

On Mon, Jun 13, 2011 at 11:58 PM, Simon Peyton-Jones
<simonpj at microsoft.com> wrote:
> Following Bryan's suggestion I've improved GHC's error message when there's a module cycle:
>
>      Module imports form a cycle:
>        module `Foo4' imports `Foo'
>                which imports `Foo2'
>                which imports `Foo3'
>                which imports `Foo4'
>
> Simon
>
> | -----Original Message-----
> | From: glasgow-haskell-users-bounces at haskell.org [mailto:glasgow-haskell-users-
> | bounces at haskell.org] On Behalf Of Bryan Richter
> | Sent: 14 April 2011 18:24
> | To: glasgow-haskell-users at haskell.org
> | Subject: Re: ghc cyclic import error confusing
> |
> | On Wed, Apr 13, 2011 at 11:44:33AM +0100, Simon Marlow wrote:
> | > On 09/04/2011 04:32, Evan Laforge wrote:
> | > >I've found ghc's cyclic import error to be rather confusing, and I
> | > >finally took the time to understand why.  Here's an example:
> | > >
> | > >Module imports form a cycle for modules:
> | > >  Cmd.Cmd (./Cmd/Cmd.hs)
> | > >    imports: Perform.Midi.Instrument Instrument.MidiDb Instrument.Db
> | > >  Perform.Midi.Instrument (./Perform/Midi/Instrument.hs)
> | > >    imports: Cmd.Cmd
> | > >  Instrument.MidiDb (./Instrument/MidiDb.hs)
> | > >    imports: Perform.Midi.Instrument
> | > >  Instrument.Db (./Instrument/Db.hs)
> | > >    imports: Instrument.Search Instrument.MidiDb
> | > >             Perform.Midi.Instrument
> | > >  Instrument.Search (./Instrument/Search.hs)
> | > >    imports: Instrument.MidiDb Perform.Midi.Instrument
> | > >
> | > >It seems to be in a strange order and mentions extraneous modules.  I
> | > >would find this much easier to read:
> | > >
> | > >Perform.Midi.Instrument ->  Cmd.Cmd ->  Instrument.MidiDb ->
> | > >Perform.Midi.Instrument
> | >
> | > So the algorithm that GHC uses is this:
> | >
> | >   - do a strongly connected component analysis
> | >   - build until we hit a cycle
> | >   - then, report the error for the cycle
> | >
> | > Now, the modules in the cycle are a strongly connected component:
> | > every module is reachable from every other module by following
> | > imports.  We report all the modules in the strongly connected
> | > component and their imports, but omit imports of modules outside the
> | > cycle.
> | >
> | > >Instead, the order goes Cmd.Cmd ->  Instrument.MidiDb, and then goes
> | > >backwards to Perform.Midi.Instrument ->  Cmd.Cmd.  Then it goes
> | > >forward again to Instrument.MidiDb ->  Perform.Midi.Instrument.  So
> | > >the order makes you jump around if you want to trace the import
> | > >chain.  The duplicated module that joins the cycle is not visually highlighted.
> | > >Whats more, it further confuses the eye by merging in multiple loops.
> | > >I suppose it could be useful to include additional loops, but I would
> | > >find it easier to read if they were included on their own line, such
> | > >as:
> | > >
> | > >Cmd.Cmd ->  Instrument.Db ->  Instrument.Search ->
> | > >Perform.Midi.Instrument ->  Cmd.Cmd
> | > >
> | > >However, I think probably the shortest loop is the most interesting
> | > >one, and if there are multiple shortest loops, simply picking one
> | > >randomly is likely to be sufficient.
> | >
> | > Picking the shortest one sounds reasonable.  However, there could be a
> | > single edge which if removed will break all the loops, and they might
> | > want to know which it is.
> | >
> | > If you want to play with this, the code is very localised: it is a few
> | > lines in GhcMake.cyclicModuleError
> | >
> | > http://hackage.haskell.org/trac/ghc/browser/compiler/main/GhcMake.hs#L
> | > 1446
> |
> | Hi Simon and Evan,
> |
> | Thanks for bringing up a problem, and providing useful information about it, in a way
> | that is understandable enough for a newcomer to have an opinion about it!
> |
> | So, here is my newcomer's opinion: The error displays a strongly connected graph,
> | with one or more cycles, but labels it a "cycle". As you both point out, having all
> | the information about the strongly connected modules is very useful. Labeling it a
> | cycle, however, gives the reader an expectation that is bound to be confounded.
> |
> | Perhaps the following change would be sufficient?
> |
> |
> |
> | --- tmp/GhcMake.hs~   2011-04-14 09:46:02.177298318 -0700
> | +++ tmp/GhcMake.hs    2011-04-14 09:52:25.121290827 -0700
> | @@ -1460,7 +1460,8 @@
> |
> |  cyclicModuleErr :: [ModSummary] -> SDoc  cyclicModuleErr ms
> | -  = hang (ptext (sLit "Module imports form a cycle for modules:"))
> | +  = hang (ptext (sLit "Module imports form a strongly connected graph,
> | + with one or more cycles, for these modules:"))
> |         2 (vcat (map show_one ms))
> |    where
> |      mods_in_cycle = map ms_mod_name ms
> |
> |
> |
> |
> |
> | -Bryan
> |
> | P.S. I'm not sure what the accepted method for formatting/wrapping string literals
> | is, so I left it as an exercise. :)
>
>
> _______________________________________________
> Glasgow-haskell-users mailing list
> Glasgow-haskell-users at haskell.org
> http://www.haskell.org/mailman/listinfo/glasgow-haskell-users
>