| I think we should not build the non-core libs with -Werror. It makes | perfect sense for the core libs where the ghc team effectively maintains | them, but not for non-core ones. But time *is* a core lib. Similarly containers, pretty, filepath, directory... The full list is below. GHC is simply a client for these libraries. Should they have -Werror or not? I'm not sure. Simon | | It is for exactly this reason that hackage rejects packages that specify | "ghc-options: -Werror"; new compiler warnings make old packages fail to | compile. So there should not be many libraries affected (there are only | one or two on hackage that use -Werror before we added the check to | reject it). So the packages themselves don't specify -Werror. I assume | it's just ghc's build system adds it for all libs, core and other. | | Duncan utils/hsc2hs hsc2hs libraries/array packages/array libraries/base packages/base libraries/bytestring packages/bytestring libraries/Cabal packages/Cabal libraries/containers packages/containers libraries/directory packages/directory libraries/editline packages/editline libraries/filepath packages/filepath libraries/ghc-prim packages/ghc-prim libraries/haskell98 packages/haskell98 libraries/hpc packages/hpc libraries/integer-gmp packages/integer-gmp libraries/old-locale packages/old-locale libraries/old-time packages/old-time libraries/packedstring packages/packedstring libraries/pretty packages/pretty libraries/process packages/process libraries/random packages/random libraries/template-haskell packages/template-haskell libraries/unix packages/unix libraries/Win32 packages/Win32

| I think we should not build the non-core libs with -Werror. It makes | perfect sense for the core libs where the ghc team effectively maintains | them, but not for non-core ones.

But time *is* a core lib. Similarly containers, pretty, filepath, directory... The full list is below. GHC is simply a client for these libraries. Should they have -Werror or not? I'm not sure.

The first time I joined a project with rcs (and in those days, that meant RCS), I was given a few firm rules, the most important of which was (emphasis added): 1. whatever I check in, the _whole_ thing has to build ok An immediate corollary was: 2. if _my_ changes break someone else's code, _I_ have to fix that In these days of distributed rcs with one build using multiple repos or even multiple rcss, those rules aren't as clear, but I'd suggest to interpret GHC+corelibs as a unit, and to apply rules 1 and 2. In fact, until the Haskell Platform is ready to take over from extralibs, it would be helpful to apply the same rules to extralibs as well. Otherwise, we'll end up here ? It highlights an important advantage of having a nontrivial set ? of extralibs in the ghc buildbot: early warnings about when and ? how ghc changes are going to break user-/library-code. ? ? Once the extralibs go, that feedback will be less immediate, the ? spread of breakage will be wider, and the current "why should ? Ghc Hq have to worry about network package maintenance?" ? could easily turn into a major source of friction, with both Ghc ? Hq and library maintainers insisting that the breakage isn't in their ? boat ("but HEAD fast builds just fine","but I didn't change a bit ? in my library code") and users likely to pay the price. much faster than even I feared just a week ago.. Claus ps. so that you don't have to iterate over failures: http://www.haskell.org/pipermail/cvs-ghc/2008-August/044021.html

On Tue, 2008-08-12 at 00:33 +0100, Ian Lynagh wrote:

On Mon, Aug 11, 2008 at 12:16:08PM +0100, Duncan Coutts wrote:

...

...

Who is responsible for the time/ library?

I think we should not build the non-core libs with -Werror.

We don't, but:

$ head -1 time/Data/Time/Calendar/Gregorian.hs {-# OPTIONS -Wall -Werror #-}

(in actual fact, we don't even build GHC/bootlibs with -Werror except when validating).

Ah, so that's the culprit. Duncan

Ashley | Duncan Coutts wrote: | >> $ head -1 time/Data/Time/Calendar/Gregorian.hs | >> {-# OPTIONS -Wall -Werror #-} | >> | >> (in actual fact, we don't even build GHC/bootlibs with -Werror except | >> when validating). | > | > Ah, so that's the culprit. | | I prefer this, actually, as we get to discover issues sooner rather than | later. Though putting it in the .cabal file or wherever might be better. That's fine. But then you can choose a) add -fno-warn-orphans or b) move the (Show Day) instance to the module declaring Day. I'm not sure which is best for you, but you're the package author so you get to decide! Regardless, it'd help if you felt able to do one or the other, because currently the package won't compile at all. Simon

| Patch pushed. I've plumped for option a. It's better structuring, as b | would involve moving more code from Gregorian.hs to Days.hs. | | The concern Henning raised shouldn't apply, as both modules are hidden | and re-exported by Data.Time.Calendar. I've also fixed two other modules | with the same issue (again, also hidden). Thanks. It doesn't matter whether they are hidden or not --- their instances are visible regardless in Haskell. So those interface files will be read any time you compile a module that depends on a module in the time package. But that's not a terribly big deal. Simon

On Tue, Aug 12, 2008 at 12:54:04AM -0700, Ashley Yakeley wrote:

What I meant was, the module in which the instance is defined, and the module in which the type is defined are both hidden, and only re-exported by another module. Thus it is not possible to import the type without importing the instance.

Not quite: * Data.Time.Calendar exports Day without its Read instance. * Data.Time.Clock exports UTCTime without Read or Show instances. * Data.Time.LocalTime exports TimeOfDay, LocalTime, TimeZone, UTCTime and ZonedTime without Read instances.

On Tue, 2008-08-12 at 10:30 +0100, Claus Reinke wrote:

...

...

I'm not sure which is best for you, but you're the package author so you get to decide!

While you're looking at the code for time, Ashley, I've got a question about its relation to old-time: shouldn't System.Time have a DEPRECATED pragma, pointing to time? The comments and package name say old-time is deprecated in favour of time.

Probably. I've never touched old-time. -- Ashley Yakeley

...

While you're looking at the code for time, Ashley, I've got a question about its relation to old-time: shouldn't System.Time have a DEPRECATED pragma, pointing to time? The comments and package name say old-time is deprecated in favour of time.

I looked into this; it's not quite that simple. System.Time exports ClockTime, which is still used in System.Directory.getModificationTime. So in order to properly deprecate System.Time, we have to supply an alternative to System.Directory.getModificationTime, which would introduce a dependency on the time package, and directory is currently a core package.

Thanks for checking, Simon. But wouldn't that simply mean replacing old-time with time in the corelibs, keeping old-time around for one or two releases only to get the deprecation message out? Perhaps time could even provide a compat module for the transition period, so that old-time could be dropped immediately, while current old-time clients transition from the compat module to proper time modules. Claus

Henning Thielemann wrote:

On Tue, 12 Aug 2008 kahl@cas.mcmaster.ca wrote:

...

I just would like to point out that there is nothing inherently bad about what GHC calls ``orphan instances''.

From a code structuring point of view, I frequently ``consider orphan'' instances useful for separation of concerns.

The problem is, that if you have a main instance of a class for a type and this one is not bundled with either the type or the class, then you are able to import the type and the class without the main instance (that is, you can accidentally miss that instance)

Or on purpose --- this is in fact another use of ``orphan instances'' I forgot to mention.

, and thus you are able to define another instance.

Indeed --- this is the only way to have different instances for the same class, as long as we do not have something like the ``named instances'' of our Haskell-2001 paper (shameless plug ;-).

This will likely cause clash with the main instance sooner or later, if other modules import your custom instance and the main one.

If there are several instances, there is very likely no ``main instance''. Wolfram

Wolfram wrote:

...

Or on purpose --- this is in fact another use of ``orphan instances'' I forgot to mention... Indeed --- this is the only way to have different instances for the same class, as long as we do not have something like the ``named instances'' of our Haskell-2001 paper (shameless plug ;-).

Henning Thielemann wrote:

...

...

This will likely cause clash with the main instance sooner or later, if other modules import your custom instance and the main one.

...

If there are several instances, there is very likely no ``main instance''.

Jonathan Cast wrote:

If there is no main instance, there should very likely be no instance at all. We already have named instances... Confusing this with type classes seems mostly redundant to me.

This argument, or something like it, is raised whenever someone mentions the need to define multiple instances of a class for the same type. And it is correct, theoretically. But in real life, you often need to write code against existing modules that you can't change. When an existing module exports an instance that is inconvenient, you can be in deep trouble. We are desperately in need of a solution to this problem. If not Wolfram's "named instances", then at least there must be some way to control the import and export of instances, just as we can now control the import and export of every other kind of symbol binding. Thanks, Yitz

On 2008-08-12, Yitzchak Gale wrote:

Wolfram wrote:

...

...

Or on purpose --- this is in fact another use of ``orphan instances'' I forgot to mention... Indeed --- this is the only way to have different instances for the same class, as long as we do not have something like the ``named instances'' of our Haskell-2001 paper (shameless plug ;-).

Henning Thielemann wrote:

...

...

...

This will likely cause clash with the main instance sooner or later, if other modules import your custom instance and the main one.

...

...

If there are several instances, there is very likely no ``main instance''.

Jonathan Cast wrote:

...

If there is no main instance, there should very likely be no instance at all. We already have named instances... Confusing this with type classes seems mostly redundant to me.

This argument, or something like it, is raised whenever someone mentions the need to define multiple instances of a class for the same type. And it is correct, theoretically.

But in real life, you often need to write code against existing modules that you can't change. When an existing module exports an instance that is inconvenient, you can be in deep trouble.

Or, you can just use newtype. -- Aaron Denney -><-

On Wed, 2008-08-13 at 09:29 +0100, Malcolm Wallace wrote:

...

We already have named instances:

data ShowDict alpha = ShowDict alpha { namedShows :: alpha -> String -> String }

This is not valid Haskell.

I must have been distracted. I meant: data ShowDict alpha = ShowDict { namedShows :: alpha -> String -> String } obviously.

Either all components of the data structure are named fields, or none. It is not possible to have a mixture of the two, as here.

In addition, there are many classes it is not currently possible to simulate using this technique, because the types go significantly beyond Haskell'98.

My proposal already required implicit parameters... jcc

| Ashley: | What is an orphan instance, and why do we care about them? They are documented in the GHC manual http://www.haskell.org/ghc/docs/latest/html/users_guide/separate-compilation... | Wolfram: | I just would like to point out that there is nothing inherently bad about | what GHC calls ``orphan instances''. ... that there is no *implementation-independent* | reason to avoid ``orphan instances''. | | From a code structuring point of view, | I frequently ``consider orphan'' instances | useful for separation of concerns. I agree. The warning just warns you that compilation of any module that depends on this module, or on the package of which this module becomes a part, will become a little slower, for reasons explained above. | (On the implementation side, a completely different solution would be | to add (automatically) re-exported instances (and rewrite rules) | to the export lists stored inside .hi files --- | then ``orphan instances'' would be no worse than other instances.) Indeed, you could certainly accumulate in every M.hi file a list of all orphan instances anywhere below M. What GHC does instead is to accumulate a list of all the *modules that contain* orphan instances, which amounts to much the same thing. Either way it's tiresome because all these instances must be brought into scope for every compilation, even though most of them are useless. As you say, though, it's just an implementation matter. That's why it's only a warning. Simon

| I still don't understand why ghc will become "slower" with orphaned | modules. Where is "wasted work" or which "instances are useless"? | Doesn't ghc just read all interface files of modules in the import chain | (i.e. all modules "below")? Or is that the "disaster in practice, so GHC | tries to be clever"? In what way is GHC clever? Are only interface files | of directly imported modules (plus orphaned modules mentioned in there) | read in? In the *absence* of orphan modules, GHC reads as few interface files as possible. It must read the interface of every *directly-imported* module. After that, it's by-need only. For example module Foo where import Prelude x = () GHC must read Prelude.hi, but needs read nothing else to compile the module. Now suppose it's like this instead module Foo where import Prelude x = map module Prelude( map, filter ) where import GHC.Map( map ) import GHC.Filter( filter ) Now when compiling Foo, GHC reads Prelude.hi, and sees that GHC.Map.map is brought into scope. Since that function is *used* in Foo, GHC also reads GHC.Map.hi to find GHC.Map.map's type, unfolding, arity, strictness etc etc. But it doesn't read GHC.Filter. In the *presence* of orphan modules, perhaps somewhere in the transitive closure of modules imported by Prelude, GHC must read those interface files too. We store a list of all orphan modules transitively below Prelude inside Prelude.hi, precisely so GHC knows which ones to read. Does that help? (If so, and you find it helpful, would you like to add some advice or information to the GHC wiki? So that those not reading this thread right now might be illuminated later.) Simon

Simon Peyton-Jones wrote:

Now when compiling Foo, GHC reads Prelude.hi, and sees that GHC.Map.map is brought into scope. Since that function is *used* in Foo, GHC also reads GHC.Map.hi to find GHC.Map.map's type, unfolding, arity, strictness etc etc. But it doesn't read GHC.Filter.

In the *presence* of orphan modules, perhaps somewhere in the transitive closure of modules imported by Prelude, GHC must read those interface files too. We store a list of all orphan modules transitively below Prelude inside Prelude.hi, precisely so GHC knows which ones to read.

Perhaps Prelude.hi could, instead of storing a *list* of orphan modules, store all the orphan instances themselves? Obviously that would waste a bit more space (less now that instances don't contain big function bodies in the HEAD; more because RULES can be orphan too, and numerous). Would it be any faster loading or is it just a bad idea? :-) -Isaac