Parser doesn't support doc comments on type aliases

[waern]

I want to add comments to type synonyms in order to fix the following Haddock bug, but I need some help.

 http://trac.haskell.org/haddock/ticket/9

In the parser, the rule for type synonyms is:

'type' type '=' ctype                                      

Types with comments are already defined as ctypedoc and used for top-level types.

So it is tempting to just change ctype in the above line to ctypedoc. Indeed, I think originally ctypedoc was defined exactly as ctype, modulo comments. However, since then ctype has changed.

The differences are:

• ctype disallows foralls/contexts after a contex implication (=>).
• ctype allows implicit parameters outside contexts. Seems to be disallowed by GHC later (after parsing).
• ctype allows type equalities (~) outside contexts. Seems to be disallowed later also.

I haven't seen any further differences (besides comments).

Can I just change type synonyms to use ctypedoc without any other changes? Or do we need the features of ctype there? If so, then changing ctypedoc into just a commented version of ctype would not work as a solution, since then syntax like this (from base:GHC/Desugar.hs) breaks:

(>>>) :: forall arr. Arrow arr => forall a b c. arr a b -> arr b c -> arr a c

[simonpj]

David,

This looks accidental to me. In general, our story is: if it makes the parser simpler, then it's fine to make the parser more generous (ie willing to parse more stuff), and reject illegal programs later.

So maybe you should replace every use of ctype with ctypedoc (or rather rename the latter to be the former)? That would go further than you need, and would allow 'docs' to be parsed in more places. But if it doesn't give rise to ambiguities, it'd be fine by me.

Make sure you run regression tests of course!

Simon

[waern]

Replying to simonpj:

David, This looks accidental to me. In general, our story is: if it makes the parser simpler, then it's fine to make the parser more generous (ie willing to parse more stuff), and reject illegal programs later.

Thanks! It is very useful to know this principle.

So maybe you should replace every use of ctype with ctypedoc (or rather rename the latter to be the former)? That would go further than you need, and would allow 'docs' to be parsed in more places. But if it doesn't give rise to ambiguities, it'd be fine by me.

I originally tried to replace each ctype with ctypedoc but it gave rise to an ambiguity when parsing record fields. Comments on fields clashes with comments in field type signatures. So we really need two different productions.

So what I want to do now is to just change type synonyms to use ctypedoc.

To do that, I'd like to extend ctypedoc so it allows everything allowed by ctype. That seems to work fine.

But remember that ctypedoc supports syntax not supported by ctype. It allows foralls/contexts after a contex implication (=>). So this is the remaining problem. By changing type synonyms to use ctypedoc we allow more things in type synonyms than we did before.

Here is an example:

{-# LANGUAGE Rank2Types #-}
import Control.Arrow (Arrow(..))

(>>>) :: forall arr. Arrow arr => forall a b c. arr a b -> arr b c -> arr a c
(>>>) = undefined

type Foo = forall arr. Arrow arr => forall a b c. arr a b -> arr b c -> arr a c

The type signature for (>>>) is accepted since it uses ctypedoc which allows this syntax. But we have not accepted the declaration Foo before. With ctypedoc in type synonyms, it will now be accepted. Is that OK?

[simonpj]

I think it'd be fine to accept the examples you give. Doing so is in line with the principle I mentioned above. Indeed, rejecting them is probably accidental!

Please write comments in your patch to explain why ctype and ctypedoc are distinct, notably the ambiguity in records. Give an example of an ambiguous piece of program text!

Just to explain why (x::ty) and (ty1~ty2) are accepted in ctype, when the parser sees

 (C a, D a, a~b, x::Int) => blah

it can't tell that is is parsing a context, rather than a tuple, until very late. So we pretend that it is parsing a tuple, and sort it out later. To make that work, the parser has to recognise (C a, D a, a~b, x::Int) as a tuple type. And that in turn means recognising (a~b) and (x::Int) as a type.

Lastly, you'll see that (a~b) and (x::Int) are treated differently (the former is in context while the latter is only in type). The reason for this is, I think, that we want to allow f and g2 but not g1:

  f  :: a~F b => blah      -- Yes
  g1 :: x::Int => blah     -- No
  g2 :: (x::Int) => blah   -- Yes

I'm not quite sure why!

While I was looking at the code, I noticed the following:

• I don't think it's deliberate that ctype doesn't allow nested foralls, just like ctypedoc. That is, it should probably say

          | context '=>' ctype  { LL $ mkImplicitHsForAllTy   $1 $3 }
  

• We should probably use gentype rather than type in the LHS of type declarations
• That would leave the only use of type in ctype; and only one of its occurrences makes sense there too!
• So it might make sense to inline type there:

  ctype  :: { LHsType RdrName }
          : 'forall' tv_bndrs '.' ctype	{ LL $ mkExplicitHsForAllTy $2 (noLoc []) $4 }
          | context '=>' ctype            { LL $ mkImplicitHsForAllTy   $1 $3 }
          | ipvar '::' gentype
          | gentype				{ $1 }
  

• Which in turn would let us rename gentype to type

I don't know if you feel like doing this, but I thought I'd record it here so I don't forget.

Simon

[waern]

Replying to simonpj:

I think it'd be fine to accept the examples you give. Doing so is in line with the principle I mentioned above. Indeed, rejecting them is probably accidental!

Just to be clear, since you say "them": It's actually only Foo which was rejected before. The type of (>>>) has been allowed before since it uses ctypedoc. (This was not so clear in my last note).

Please write comments in your patch to explain why ctype and ctypedoc are distinct, notably the ambiguity in records. Give an example of an ambiguous piece of program text!

Here is my patch, which I've validated and pushed to HEAD:

Tue Mar 31 23:23:06 CEST 2009  David Waern <david.waern@gmail.com>
  * Allow Haddock comments in type synonyms
  
  We now use `ctypedoc` instead of `ctype` for type synonyms. `ctypedoc` was
  previously only used for top-level type signatures. This change means that type
  synonyms now can contain comments, just like top-level type signatures.
    
  Note:
    
  * I've modified `ctypedoc` so it allows implicit parameters and equational
  constraints, just like ctype.
    
  * Since `ctypedoc` allows nested foralls, we now allow that in type synonyms.
    
  * I have inlined some productions into gentypedoc so that there is now a
  non-doc version of every production with a 'doc' suffix. (Stylistic change
  only, which should make the code easier to follow).
    
  * It would have been nice to simplify the grammar by unifying `ctype` and 
  ctypedoc` into one production, allowing comments on types everywhere (and
  rejecting them after parsing, where necessary).  This is however not possible
  since it leads to ambiguity. The reason is the support for comments on record
  fields:
    
  > data R = R { field :: Int -- ^ comment on the field }
    
  If we allow comments on types here, it's not clear if the comment applies
  to 'field' or to 'Int'. So we must use `ctype` to describe the type.

Just to explain why (x::ty) and (ty1~ty2) are accepted in ctype, when the parser sees {{{ (C a, D a, a~b, x::Int) => blah }}} it can't tell that is is parsing a context, rather than a tuple, until very late. So we pretend that it is parsing a tuple, and sort it out later. To make that work, the parser has to recognise (C a, D a, a~b, x::Int) as a tuple type. And that in turn means recognising (a~b) and (x::Int) as a type.

Thanks for this explanation! It makes sense.

Lastly, you'll see that (a~b) and (x::Int) are treated differently (the former is in context while the latter is only in type). The reason for this is, I think, that we want to allow f and g2 but not g1: {{{ f :: a~F b => blah -- Yes g1 :: x::Int => blah -- No g2 :: (x::Int) => blah -- Yes }}} I'm not quite sure why!

I think I remember wondering why parenthesis was needed around IPs when I used them, long ago. I just thought it was to avoid multiple :: on the same level. I noticed that you seem to require kind signatures to be in parenthesis, also.

While I was looking at the code, I noticed the following: * I don't think it's deliberate that ctype doesn't allow nested foralls, just like ctypedoc. That is, it should probably say {{{ | context '=>' ctype { LL $ mkImplicitHsForAllTy $1 $3 } }}} * We should probably use gentype rather than type in the LHS of type declarations * That would leave the only use of type in ctype; and only one of its occurrences makes sense there too! * So it might make sense to inline type there: {{{ ctype :: { LHsType RdrName? } : 'forall' tv_bndrs '.' ctype { LL $ mkExplicitHsForAllTy $2 (noLoc []) $4 } | context '=>' ctype { LL $ mkImplicitHsForAllTy $1 $3 } | ipvar '::' gentype | gentype { $1 } }}} * Which in turn would let us rename gentype to type

I don't have time to do this right now (already validated). But I'll get to it later, hopefully.

Btw, is there a change my patch could be merged to the stable branch (i.e get into 6.10.2)? It's a pretty important Haddock bug, so it would be nice to get it fixed in 6.10.2, now that it will become part of the first Haskell Platform and all. :-) If you don't dare to merge it, that's fine, of course.

[igloo]

Sorry, it was too late for 6.10.2; the builds were already done. I'll leave the ticket as a merge in case we do a 6.10.3.

[igloo]

We won't be merging non-essential fixes into the 6.10 branch any more.