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 <simo...@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-boun...@haskell.org > [mailto:glasgow-haskell-users- > | boun...@haskell.org] On Behalf Of Bryan Richter > | Sent: 14 April 2011 18:24 > | To: glasgow-haskell-users@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@haskell.org > http://www.haskell.org/mailman/listinfo/glasgow-haskell-users > _______________________________________________ Glasgow-haskell-users mailing list Glasgow-haskell-users@haskell.org http://www.haskell.org/mailman/listinfo/glasgow-haskell-users
