Twan,

In general I'd be delighted for you to clean up GHC's code -- thank you!  There 
is a slight down-side which is that because you end up touching a lot of code, 
you get a big patch that means you can't selectively pick patches from before 
and after the change -- but I'm not too bothered about that, and its balanced 
by the clean-up.

Let's also see what Simon and Ian (or others) have to say.

Best to do one change per patch.

A very useful thing to do would be to improve the Commentary:
        http://hackage.haskell.org/trac/ghc/wiki/Commentary
In fiddling with GHC's codebase you will surely learn stuff that you wish you'd 
known to start with, and that is the Ideal Moment to capture it in writing on 
the Wiki.  A real service to others.


|   1a. Use do notation where possible instead of `thenXX`.

Yes.

|   1b. Even better, use Applicative where possible. For example
|
|          ds_type (HsFunTy ty1 ty2)
|            = dsHsType ty1                       `thenM` \ tau_ty1 ->
|              dsHsType ty2                       `thenM` \ tau_ty2 ->
|              returnM (mkFunTy tau_ty1 tau_ty2)
|
|       Could be rewritten as
|
|          ds_type (HsFunTy ty1 ty2)
|            = mkFunTy <$> dsHsType ty1 <*> dsHsType ty2

This works great when you have the pattern above -- but often it's a bit more 
complicated and then it does not work so well.  So it's hard to employ the 
pattern consistently.  Then you have to ask whether a mixture of styles is good.

My gut feel is that it's better to use do-notation consistently.

|   2. Investigate the need for all these mappM functions. To quote the
| source: "Standard combinators, but specialised for this monad (for
| efficiency)". Wouldn't a SPECIALIZE pragma work just as well?

The trouble is that currently you can only specialise a function at its 
definition site -- and at the definition of mapM the relevant monad isn't in 
scope because mapM is in a library.

I have no clue whether this has any effect on performance.  You could try a 
small experiment, by defining mappM=mapM and seeing if there was any change.

|   3. In general, use standard functions when they are available. Currently
| GHC has its own sort function and its own versions of FiniteMap/Data.Map and
| Text.PrettyPrint. Using standard functions/data structures will a) make GHC
| smaller and therefore easier to maintain, and b) will make the code easier
| to understand for someone who knows the standard library.

In general yes.

But GHC is meant to be compilable with any GHC back to 6.2.  So if the 
interface to a library changes between (say) 6.2 and 6.4, you end up with 
ifdefs.  That can be a royal pain.  Having the library code in the compiler 
source eliminates that problem.

The other thing is that it's possible that GHC's version of the library has 
more functions.  Or uses some non-standard feature like unboxed types.

So proceed with caution here.  Prettyprinting would be a strong candidate.  
FiniteMap too. But be careful with UniqFM -- it's a very very heavily-used 
library.

|   4. A more radical change would be introducing hierarchical modules. This
| could be just a matter of renaming the directories to start with an upper
| case character and changing the import declarations. This gives module names
| like "Typecheck.TcGadt". The tc is redundant here, so perhaps it should be
| renamed to "Typecheck.Gadt" or "Typecheck.GADT". Perhaps even better would
| be "GHC.Typecheck.GADT", this way some modules can be exposed as part of the
| GHC api.

I don't think I'd object, but I'm not sure that much is gained here.  We don't 
spend much time on mondule-name clashes.

Simon
_______________________________________________
Glasgow-haskell-users mailing list
Glasgow-haskell-users@haskell.org
http://www.haskell.org/mailman/listinfo/glasgow-haskell-users

Reply via email to