#11028: Refactor ConDecl -------------------------------------+------------------------------------- Reporter: simonpj | Owner: Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.10.2 Keywords: | Operating System: Unknown/Multiple Architecture: | Type of failure: None/Unknown Unknown/Multiple | Test Case: | Blocked By: Blocking: | Related Tickets: Differential Rev(s): | Wiki Page: -------------------------------------+------------------------------------- The `ConDecl` type in `HsDecls` is an uneasy compromise. For the most part, `HsSyn` directly reflects the syntax written by the programmer; and that gives just the right "pegs" on which to hang Alan's [wiki:ApiAnnotations API annotations]. But `ConDecl` doesn't properly reflect the syntax of Haskell-98 and GADT-style data type declarations. To be concrete, here's a draft new data type {{{ data ConDecl name | ConDeclGADT { con_names :: [Located name] , con_type :: LHsSigType name -- The type after the ‘::’ , con_doc :: Maybe LHsDocString , con_old_rec :: Bool } | ConDeclH98 { con_name :: Located name , con_implict :: HsImplicitBndrs () -- ^ Implicit binders, added by renamer , con_qvars :: Maybe [LHsTyVarBndr name] -- User-written foralls (if any) , con_cxt :: Maybe (LHsContext name) -- ^ User-written context (if any) , con_details :: HsConDeclDetails name -- ^ Arguments , con_doc :: Maybe LHsDocString -- ^ A possible Haddock comment. } deriving (Typeable) }}} Note that * For GADTs, just keep a type. That's what the user writes. (NB: `HsType` can represent records on the LHS of an arrow: `{ x:Int,y:Bool } -> T` * `con_qvars` and `con_cxt` are both `Maybe` because they are both optional (the `forall` and the context of an existential data type) * The `con_old_rec` is a warning thing for GADTs (see current source code) This ticket is just to track the thought and invite feedback. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11028 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#11028: Refactor ConDecl -------------------------------------+------------------------------------- Reporter: simonpj | Owner: Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by goldfire): I have no objections. It would be nice to somehow capture the fact that all constructors are either one or the other. But that might be annoying to code with. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11028#comment:1 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#11028: Refactor ConDecl -------------------------------------+------------------------------------- Reporter: simonpj | Owner: Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by simonpj): Test `ghc-api/annotations/T10399` is expect-broken on this ticket. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11028#comment:2 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#11028: Refactor ConDecl -------------------------------------+------------------------------------- Reporter: simonpj | Owner: Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Changes (by alanz): * cc: alanz, mpickering (added) -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11028#comment:3 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#11028: Refactor ConDecl -------------------------------------+------------------------------------- Reporter: simonpj | Owner: Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by jstolarek): Replying to simonpj:
The `con_old_rec` is a warning thing for GADTs (see current source code)
Incidentally, I removed that field not so long ago - see ea8c116ac9eb916fdb6360a01c285bc8698dfaf9. {{{#!hs data ConDecl name | ConDeclGADT { con_names :: [Located name] , con_type :: LHsSigType name -- The type after the ‘::’ , con_doc :: Maybe LHsDocString , con_old_rec :: Bool } }}} What is `LHsSigType`? A synonym for `LHsType`? If so then, perhaps it would be a good idea to have separate fields for storing foralls and the context? I'm looking at my work on #10828 (Phab:D1465) and I think the implementation in `DsMeta` would get a bit more difficult if foralls and context are folded into one field with the whole type. Of course we can create functions that extract these information from `ConDecl` but wouldn't separate fields be easier? -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11028#comment:4 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#11028: Refactor ConDecl -------------------------------------+------------------------------------- Reporter: simonpj | Owner: Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Changes (by jstolarek): * cc: jstolarek (added) -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11028#comment:5 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#11028: Refactor ConDecl -------------------------------------+------------------------------------- Reporter: simonpj | Owner: Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by simonpj): Jan: see branch `wip/spj-wildcard-refactor`, and https://phabricator.haskell.org/D1428, which will land shortly -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11028#comment:6 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#11028: Refactor ConDecl -------------------------------------+------------------------------------- Reporter: simonpj | Owner: alanz Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Changes (by alanz): * owner: => alanz Comment: I am making a start on this, using `wip/spj-wildcard-refactor` as the base. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11028#comment:7 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#11028: Refactor ConDecl -------------------------------------+------------------------------------- Reporter: simonpj | Owner: alanz Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Old description:
The `ConDecl` type in `HsDecls` is an uneasy compromise. For the most part, `HsSyn` directly reflects the syntax written by the programmer; and that gives just the right "pegs" on which to hang Alan's [wiki:ApiAnnotations API annotations]. But `ConDecl` doesn't properly reflect the syntax of Haskell-98 and GADT-style data type declarations.
To be concrete, here's a draft new data type {{{ data ConDecl name | ConDeclGADT { con_names :: [Located name] , con_type :: LHsSigType name -- The type after the ‘::’ , con_doc :: Maybe LHsDocString , con_old_rec :: Bool }
| ConDeclH98 { con_name :: Located name
, con_implict :: HsImplicitBndrs () -- ^ Implicit binders, added by renamer
, con_qvars :: Maybe [LHsTyVarBndr name] -- User-written foralls (if any)
, con_cxt :: Maybe (LHsContext name) -- ^ User-written context (if any)
, con_details :: HsConDeclDetails name -- ^ Arguments
, con_doc :: Maybe LHsDocString -- ^ A possible Haddock comment. } deriving (Typeable) }}} Note that * For GADTs, just keep a type. That's what the user writes. (NB: `HsType` can represent records on the LHS of an arrow: `{ x:Int,y:Bool } -> T`
* `con_qvars` and `con_cxt` are both `Maybe` because they are both optional (the `forall` and the context of an existential data type)
* The `con_old_rec` is a warning thing for GADTs (see current source code)
This ticket is just to track the thought and invite feedback.
New description: The `ConDecl` type in `HsDecls` is an uneasy compromise. For the most part, `HsSyn` directly reflects the syntax written by the programmer; and that gives just the right "pegs" on which to hang Alan's [wiki:ApiAnnotations API annotations]. But `ConDecl` doesn't properly reflect the syntax of Haskell-98 and GADT-style data type declarations. To be concrete, here's a draft new data type {{{ data ConDecl name | ConDeclGADT { con_names :: [Located name] , con_type :: LHsSigType name -- The type after the ‘::’ , con_doc :: Maybe LHsDocString } | ConDeclH98 { con_name :: Located name , con_implict :: HsImplicitBndrs () -- ^ Implicit binders, added by renamer , con_qvars :: Maybe [LHsTyVarBndr name] -- User-written foralls (if any) , con_cxt :: Maybe (LHsContext name) -- ^ User-written context (if any) , con_details :: HsConDeclDetails name -- ^ Arguments , con_doc :: Maybe LHsDocString -- ^ A possible Haddock comment. } deriving (Typeable) }}} Note that * For GADTs, just keep a type. That's what the user writes. (NB: `HsType` can represent records on the LHS of an arrow: `{ x:Int,y:Bool } -> T` * `con_qvars` and `con_cxt` are both `Maybe` because they are both optional (the `forall` and the context of an existential data type) This ticket is just to track the thought and invite feedback. -- Comment (by simonpj): Terrific. Please make a branch and push to it, so I can join in. Will you use the above data type as the basis? Simon -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11028#comment:8 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#11028: Refactor ConDecl -------------------------------------+------------------------------------- Reporter: simonpj | Owner: alanz Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Old description:
The `ConDecl` type in `HsDecls` is an uneasy compromise. For the most part, `HsSyn` directly reflects the syntax written by the programmer; and that gives just the right "pegs" on which to hang Alan's [wiki:ApiAnnotations API annotations]. But `ConDecl` doesn't properly reflect the syntax of Haskell-98 and GADT-style data type declarations.
To be concrete, here's a draft new data type {{{ data ConDecl name | ConDeclGADT { con_names :: [Located name] , con_type :: LHsSigType name -- The type after the ‘::’ , con_doc :: Maybe LHsDocString }
| ConDeclH98 { con_name :: Located name
, con_implict :: HsImplicitBndrs () -- ^ Implicit binders, added by renamer
, con_qvars :: Maybe [LHsTyVarBndr name] -- User-written foralls (if any)
, con_cxt :: Maybe (LHsContext name) -- ^ User-written context (if any)
, con_details :: HsConDeclDetails name -- ^ Arguments
, con_doc :: Maybe LHsDocString -- ^ A possible Haddock comment. } deriving (Typeable) }}} Note that * For GADTs, just keep a type. That's what the user writes. (NB: `HsType` can represent records on the LHS of an arrow: `{ x:Int,y:Bool } -> T`
* `con_qvars` and `con_cxt` are both `Maybe` because they are both optional (the `forall` and the context of an existential data type)
This ticket is just to track the thought and invite feedback.
New description: The `ConDecl` type in `HsDecls` is an uneasy compromise. For the most part, `HsSyn` directly reflects the syntax written by the programmer; and that gives just the right "pegs" on which to hang Alan's [wiki:ApiAnnotations API annotations]. But `ConDecl` doesn't properly reflect the syntax of Haskell-98 and GADT-style data type declarations. To be concrete, here's a draft new data type {{{ data ConDecl name | ConDeclGADT { con_names :: [Located name] , con_type :: LHsSigType name -- The type after the ‘::’ , con_doc :: Maybe LHsDocString } | ConDeclH98 { con_name :: Located name , con_qvars :: Maybe (LHsQTyVars name) -- User-written forall (if any), and its implicit -- kind variables -- Non-Nothing needs -XExistentialQuantification , con_cxt :: Maybe (LHsContext name) -- ^ User-written context (if any) , con_details :: HsConDeclDetails name -- ^ Arguments , con_doc :: Maybe LHsDocString -- ^ A possible Haddock comment. } deriving (Typeable) }}} Note that * For GADTs, just keep a type. That's what the user writes. (NB: `HsType` can represent records on the LHS of an arrow: `{ x:Int,y:Bool } -> T` * `con_qvars` and `con_cxt` are both `Maybe` because they are both optional (the `forall` and the context of an existential data type * For `ConDeclGADT` the type variables of the data type do ''not'' scope over the `con_type`; whereas for `ConDeclH98` they ''do'' scope over `con_cxt` and `con_details`. This ticket is just to track the thought and invite feedback. -- Comment (by simonpj): PS: I have edited the proposed data type a bit. Do you agree with it? -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11028#comment:9 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#11028: Refactor ConDecl -------------------------------------+------------------------------------- Reporter: simonpj | Owner: alanz Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by alanz): Yes, it maps more clearly on to the original. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11028#comment:10 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#11028: Refactor ConDecl -------------------------------------+------------------------------------- Reporter: simonpj | Owner: alanz Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by alanz): I wonder if we should use `ConDeclSimple` instead of `ConDeclH98`. Alternatively `mkSimpleConDecl` should become `mkCondeclH98`. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11028#comment:11 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#11028: Refactor ConDecl -------------------------------------+------------------------------------- Reporter: simonpj | Owner: alanz Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by alanz): Also, at the moment the `con_doc` field is never set for a GADT. It should either be removed or the parser modified to allow docs. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11028#comment:12 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#11028: Refactor ConDecl -------------------------------------+------------------------------------- Reporter: simonpj | Owner: alanz Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by jstolarek): Replying to [comment:11 alanz]:
I wonder if we should use `ConDeclSimple` instead of `ConDeclH98`.
Alternatively `mkSimpleConDecl` should become `mkCondeclH98`. I vote for the latter - `mkConDeclH98` is more informative than `mkSimpleConDecl`.
Also, at the moment the `con_doc` field is never set for a GADT. It should either be removed or the parser modified to allow docs. Again, my vote for the latter. If we store docs for H98 constructors then we should also store them for GADTs.
-- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11028#comment:13 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#11028: Refactor ConDecl -------------------------------------+------------------------------------- Reporter: simonpj | Owner: alanz Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by simonpj): Yes, I think I prefer `mkConDeclH98` too. As to `con_doc` it depends on the Haddock folk. Maybe file a ticket on the Haddock issue tracker; I don't know if they have a story about documentation GADT data types. But they should! -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11028#comment:14 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#11028: Refactor ConDecl -------------------------------------+------------------------------------- Reporter: simonpj | Owner: alanz Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by alanz): Seems I was wrong about the `con_doc`, it is added after the fact by `addConDoc` -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11028#comment:15 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#11028: Refactor ConDecl -------------------------------------+------------------------------------- Reporter: simonpj | Owner: alanz Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by alanz): At the moment `kcConDecl` is defined as {{{#!hs kcConDecl (ConDecl { con_names = names, con_qvars = ex_tvs , con_cxt = ex_ctxt, con_details = details , con_res = res }) = addErrCtxt (dataConCtxtName names) $ -- the 'False' says that the existentials don't have a CUSK, as the -- concept doesn't really apply here. We just need to bring the variables -- into scope! do { _ <- kcHsTyVarBndrs False ex_tvs $ do { _ <- tcHsContext ex_ctxt ; mapM_ (tcHsOpenType . getBangType) (hsConDeclArgTys details) ; _ <- tcConRes res ; return (panic "kcConDecl", ()) } ; return () } }}} I am working on the `ConDeclGADT` case, and have {{{#!hs kcConDecl (ConDeclGADT { con_name = name , con_type = HsIB { hsib_kvs = ex_kvs , hsib_tvs = ex_tvs , hsib_body = res_ty} }) = addErrCtxt (dataConCtxtName [name]) $ -- the 'False' says that the existentials don't have a CUSK, as the -- concept doesn't really apply here. We just need to bring the variables -- into scope! do { _ <- kcHsTyVarBndrs False ex_tvs $ ..... ; return () } }}} It is not clear to me whether `kcHsTyVarBndrs False ex_tvs` should be checking `ex_kvs`, `ex_tvs`, or both. I suspect `ex_kvs` only. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11028#comment:16 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#11028: Refactor ConDecl -------------------------------------+------------------------------------- Reporter: simonpj | Owner: alanz Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by simonpj): No no! `con_type` is a `LHsSigType`. So use `TcHsType.kcClassSigType` (give it a less specific name!); it does exactly the right thing. Simon -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11028#comment:17 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#11028: Refactor ConDecl -------------------------------------+------------------------------------- Reporter: simonpj | Owner: alanz Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by alanz): `tcConDecl` returns a `DataCon`. For `ConDeclGADT`, should I bring in a function {{{#!hs gadtDeclDetails :: LHsSigType name -> HsConDeclDetails name gadtDeclDetails (HsIB {hsib_body = lbody_ty}) = details where (tvs, cxt, tau) = splitLHsSigmaTy lbody_ty details -- See Note [Sorting out the result type] = case tau of L _ (HsFunTy (L l (HsRecTy flds)) res_ty) -> RecCon (L l flds) _other -> PrefixCon [] }}} and then proceed with the process as it is now? Should there be a call to `tcHsSigType`? -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11028#comment:18 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#11028: Refactor ConDecl -------------------------------------+------------------------------------- Reporter: simonpj | Owner: alanz Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by simonpj): Yes, because of the possibility of `HsRecTy`, you can't use `tcHsSigType`. So something like what you propose looks good. Ca you make a branch and push what you have so that I can see? -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11028#comment:19 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#11028: Refactor ConDecl -------------------------------------+------------------------------------- Reporter: simonpj | Owner: alanz Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by alanz): I just pushed an update to ` wip/T11028` It now compiles for stage1. Only. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11028#comment:20 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#11028: Refactor ConDecl -------------------------------------+------------------------------------- Reporter: simonpj | Owner: alanz Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by alanz): Commit https://phabricator.haskell.org/rGHCff2978cac0cd fails with the res_ty not matching, When type checking {{{#!hs data MaybeO ex t where JustO :: t -> MaybeO O t NothingO :: MaybeO C t }}} This code generates. {{{ libraries/hoopl/src/Compiler/Hoopl/Block.hs:66:3: error: • Data constructor ‘JustO’ returns type ‘t -> MaybeO O t’ instead of an instance of its parent type ‘MaybeO ex t’ • In the definition of data constructor ‘JustO’ In the data type declaration for ‘MaybeO’ }}} I am pretty sure the `res_ty'` variable in [1] should be `MaybeO O t` but it is not clear how this happens in the original code. [1] https://github.com/ghc/ghc/blob/ff2978cac0cd133c2434480e311bed6aea72c6f1/com... -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11028#comment:21 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#11028: Refactor ConDecl -------------------------------------+------------------------------------- Reporter: simonpj | Owner: alanz Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by alanz): Ok, the problem is in `rnConDecl` in the `ConDeclGADT` case, which in this branch simply calls `rnHsSigType`. Instead it should perform as per the original, which splits the result type,leaving it as `MaybeO ex t` in this case. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11028#comment:22 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
Ok, the problem is in `rnConDecl` in the `ConDeclGADT` case, which in
#11028: Refactor ConDecl -------------------------------------+------------------------------------- Reporter: simonpj | Owner: alanz Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by simonpj): Replying to [comment:22 alanz]: this branch simply calls `rnHsSigType`.
Instead it should perform as per the original, which splits the result
type,leaving it as `MaybeO ex t` in this case. Wait! Why? What would the difference be in the result of `rnConDecl`? I'm suspicious; I'd like to understand better. An example perhaps? -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11028#comment:23 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#11028: Refactor ConDecl -------------------------------------+------------------------------------- Reporter: simonpj | Owner: alanz Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by alanz): The key factor is the work currently done in `rnConResult`, {{{#!hs rnConResult doc _con details (ResTyGADT ls ty) = do { (ty', fvs) <- rnLHsType doc ty ; let (arg_tys, res_ty) = splitHsFunType ty' -- We can finally split it up, -- now the renamer has dealt with fixities -- See Note [Sorting out the result type] in RdrHsSyn ; case details of InfixCon {} -> pprPanic "rnConResult" (ppr ty) -- See Note [Sorting out the result type] in RdrHsSyn RecCon {} -> do { unless (null arg_tys) (addErr (badRecResTy doc)) ; return (details, ResTyGADT ls res_ty, fvs) } PrefixCon {} -> return (PrefixCon arg_tys, ResTyGADT ls res_ty, fvs)} }}} `splitHsFunTy` on `t -> MaybeO a b` will return `([t],MaybeO a b)`, and the latter is then returned as the new `res_ty` My understanding is that we are doing this so that the ParsedSource more accurately reflects what was seen, which can simplify the API Annotations work. Perhaps we should add a `PostRn` field to the `ConDeclGADT` to capture the new `details` and `res_ty`. Alternatively, everything except the `rnLHsType` in `rnConResult` can move to the type checking. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11028#comment:24 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#11028: Refactor ConDecl -------------------------------------+------------------------------------- Reporter: simonpj | Owner: alanz Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by simonpj):
Alternatively, everything except the `rnLHsType` in `rnConResult` can move to the type checking.
That sounds exactly right. `Note [Sorting out the result type]` in `RdrHsSyn` seems entirely redundant with the new `ConDeclGADT` form. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11028#comment:25 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#11028: Refactor ConDecl -------------------------------------+------------------------------------- Reporter: simonpj | Owner: alanz Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by alanz): The current error is {{{ compiler/cmm/CmmNode.hs:88:20: error: Record syntax is illegal here: {cml_pred :: CmmExpr, cml_true, cml_false :: {-# UNPACK #-} !Label, cml_likely :: Maybe Bool} }}} which is a record style GADT constructor declaration. I do not understand how {{{#!hs rnHsTyKi _ doc ty@(HsRecTy flds) = do { addErr (hang (ptext (sLit "Record syntax is illegal here:")) 2 (ppr ty)) ; (flds', fvs) <- rnConDeclFields [] doc flds ; return (HsRecTy flds', fvs) } }}} works. The error is always added, I do not see `discardErrors` in `rnConDeclFields`. What am I missing? -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11028#comment:26 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#11028: Refactor ConDecl -------------------------------------+------------------------------------- Reporter: simonpj | Owner: alanz Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by simonpj): I think you should just move the check to the typechecker. It's there to stop you writing {{{ f :: { x::Int } -> Int f = blah }}} in normal code. But the renamer now treats a GADT constructor type uniformly, so we don't want to reject it. The type checker treats them differently, so it can reject `HsRecTy` in ordinary types but not in GADTs -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11028#comment:27 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#11028: Refactor ConDecl -------------------------------------+------------------------------------- Reporter: simonpj | Owner: alanz Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by alanz): Something else I am bumping my head against is that `rnField` gets passed an empty `fl_env` and so `lookupField` generates a panic when the lookup fails. It seems that the new `RdrHsSyn.gadtDeclDetails` needs to be called before this, but unless the results are stored the names may be discarded. I do not know enough about this atm. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11028#comment:28 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#11028: Refactor ConDecl -------------------------------------+------------------------------------- Reporter: simonpj | Owner: alanz Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by simonpj): Ah yes. To rename a record we need to know what data constructor it is from; this call in `rnHsTyKi` {{{ ; (flds', fvs) <- rnConDeclFields [] doc flds }}} should not have the empty list, but rather the result of a lookup, just as in `RnSource.rnConDeclDetails`. (Indeed better to pass the `Name` of the data constructor to `rnConDeclFields`.) Where to get the `Name` of the `DataCon` from in the case of GADTs. It could be in the `HsDocContext` passed down the type renamer. Change `ConDeclCtx` to take a `Name` instead of `RdrName`s. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11028#comment:29 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#11028: Refactor ConDecl -------------------------------------+------------------------------------- Reporter: simonpj | Owner: alanz Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by simonpj): BTW, a refactoring I'd love to see done is this: '''combine `RnEnv.HsDocContext` (used in the renamer) and `TcType.UserTypeCtxt` (used in the type checker)'''. Both are fairly big types; but they are practically the same already, except that the former uses `RdrName`s while the latter uses `Name`s. But that's accidental; in fact the renamer could perfectly well construct contexts with `Name`s in them. I think that everything else is just accidental differences. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11028#comment:30 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#11028: Refactor ConDecl -------------------------------------+------------------------------------- Reporter: simonpj | Owner: alanz Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by alanz): So `kcConDecl` can't just call `kcHsSigType`, the strict marks on arguments need to be managed. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11028#comment:31 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#11028: Refactor ConDecl -------------------------------------+------------------------------------- Reporter: simonpj | Owner: alanz Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by simonpj): Bother. I guess that to typecheck a GADT decl we'll need a function with a type like this: {{{ tcGadtSigType :: LHsSigType Name -> TcM ([StrictnessMark], [FieldLabel], [Type], Type) }}} or something like that. It takes a GADT data constructor type signature, like {{{ C :: { x :: !Int, y:: Bool } -> T Int }}} and returns the typechecked pieces. Once you've got that, you can use the very same function during kind checking. Just call it and discard the result. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11028#comment:32 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#11028: Refactor ConDecl -------------------------------------+------------------------------------- Reporter: simonpj | Owner: alanz Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by alanz): I am now using `rnHsSigType` to rename the RHS of a `ConDeclGADT`, passing in a `ConDeclCtx`. This ends up calling {{{#!hs rnHsTyKi _ doc ty@(HsForAllTy { hst_bndrs = tyvars, hst_body = tau }) = bindLHsTyVarBndrs doc Nothing tyvars $ \ tyvars' -> do { (tau', fvs) <- rnLHsType doc tau ; warnUnusedForAlls (inTypeDoc ty) tyvars' fvs ; return ( HsForAllTy { hst_bndrs = tyvars', hst_body = tau' } , fvs) } }}} `warnUnusedForAlls` does not use the passed in context, but `inTypeDoc ty` instead, causing tests such as T5331 to fail. Possible solutions seem to be 1. use the passed in context for the `warnUnusedForalls`, which has the disadvantage of giving a less precise location. 2. Combine the `doc` and the `inTypeDoc ty` contexts for the warning. 3. Update the expected warning in the test, the forall context is good enough. Which is the best option, or is there another solution? -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11028#comment:33 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#11028: Refactor ConDecl -------------------------------------+------------------------------------- Reporter: simonpj | Owner: alanz Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by simonpj): What is the regression? I think it's that for {{{ data W where W1 :: forall a. W }}} we get {{{ T5331.hs:11:16: Warning: Unused quantified type variable ‘a’ In the type `forall a. W’ }}} instead of {{{ T5331.hs:11:16: Warning: Unused quantified type variable ‘a’ In the definition of data constructor ‘W1’ }}} If so, I think that's fine -- accept it. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11028#comment:34 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#11028: Refactor ConDecl -------------------------------------+------------------------------------- Reporter: simonpj | Owner: alanz Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by alanz): Ok, will do. Option 3. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11028#comment:35 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#11028: Refactor ConDecl -------------------------------------+------------------------------------- Reporter: simonpj | Owner: alanz Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by alanz): I will add this to phab for review once `wip/spj-wildcard-refactor` lands -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11028#comment:36 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#11028: Refactor ConDecl -------------------------------------+------------------------------------- Reporter: simonpj | Owner: alanz Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D1558 Wiki Page: | -------------------------------------+------------------------------------- Changes (by alanz): * differential: => Phab:D1558 -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11028#comment:37 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#11028: Refactor ConDecl -------------------------------------+------------------------------------- Reporter: simonpj | Owner: alanz Type: bug | Status: new Priority: normal | Milestone: 8.0.1 Component: Compiler | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D1558 Wiki Page: | -------------------------------------+------------------------------------- Changes (by alanz): * milestone: => 8.0.1 -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11028#comment:38 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#11028: Refactor ConDecl -------------------------------------+------------------------------------- Reporter: simonpj | Owner: alanz Type: bug | Status: new Priority: normal | Milestone: 8.0.1 Component: Compiler | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: #11155 | Differential Rev(s): Phab:D1558 Wiki Page: | -------------------------------------+------------------------------------- Changes (by alanz): * related: => #11155 Comment: The haddock linker failure in Phab:D1558 is due to #11155 -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11028#comment:39 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#11028: Refactor ConDecl -------------------------------------+------------------------------------- Reporter: simonpj | Owner: alanz Type: bug | Status: new Priority: normal | Milestone: 8.0.1 Component: Compiler | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: #11155 | Differential Rev(s): Phab:D1558 Wiki Page: | -------------------------------------+------------------------------------- Changes (by alanz): * Attachment "old-condecl.png" added. CmmNode documents for 7.10.2 -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11028 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#11028: Refactor ConDecl -------------------------------------+------------------------------------- Reporter: simonpj | Owner: alanz Type: bug | Status: new Priority: normal | Milestone: 8.0.1 Component: Compiler | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: #11155 | Differential Rev(s): Phab:D1558 Wiki Page: | -------------------------------------+------------------------------------- Changes (by alanz): * Attachment "new-condecl.png" added. CmmNode with #11028 patch -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11028 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#11028: Refactor ConDecl -------------------------------------+------------------------------------- Reporter: simonpj | Owner: alanz Type: bug | Status: new Priority: normal | Milestone: 8.0.1 Component: Compiler | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: #11155 | Differential Rev(s): Phab:D1558 Wiki Page: | -------------------------------------+------------------------------------- Comment (by alanz): I am finishing off the haddock changes for this. The original documentation generated for `CmmNode` was [[Image(https://ghc.haskell.org/trac/ghc/attachment/ticket/11028/old- condecl.png)]] With the update from the patch this becomes [[Image(https://ghc.haskell.org/trac/ghc/attachment/ticket/11028/new- condecl.png)]] See in particular `CmmCondBranch`. In the original it gives the (supposed) prefix constructor version, followed by the fields. But because `cml_true` and `cml_false` are part of the same field declaration, only one instance of `!Label` appears. In the new one, only the record fields are listed at the moment. Question: Should a prefix type signature be given at all, for a record GADT ConDecl? Or should it be left as per the new image? -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11028#comment:40 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#11028: Refactor ConDecl -------------------------------------+------------------------------------- Reporter: simonpj | Owner: alanz Type: bug | Status: patch Priority: normal | Milestone: 8.0.1 Component: Compiler | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: #11155 | Differential Rev(s): Phab:D1558 Wiki Page: | -------------------------------------+------------------------------------- Changes (by alanz): * status: new => patch -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11028#comment:41 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#11028: Refactor ConDecl -------------------------------------+------------------------------------- Reporter: simonpj | Owner: alanz Type: bug | Status: patch Priority: normal | Milestone: 8.0.1 Component: Compiler | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: #11155 | Differential Rev(s): Phab:D1558 Wiki Page: | -------------------------------------+------------------------------------- Comment (by simonpj): The old one is clearly wrong when we have {{{ cml_true, cml_false :: !Label }}} We could fix it to provide ''both'' a prefix signature ''and'' list the fields; and that is presumably what the old version was supposed to do. And the new form which looks like {{{ CmmCondBranch :: -> CmmNode O C cml_true, cml_false :: !Label ...etc... }}} does look very weird. `CmmCondBranch :: -> CmmNode O C` just isn't Haskell. Why not just print it out in the source syntax form: {{{ CmmCondBranch :: { cml_true, cml_false :: ! Label , ...etc... } -> CmmNode O C }}} Admittedly that's more work, because it's not what Haddock does right now. But it would make more sense to users wouldn't it? -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11028#comment:42 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#11028: Refactor ConDecl -------------------------------------+------------------------------------- Reporter: simonpj | Owner: alanz Type: bug | Status: patch Priority: normal | Milestone: 8.0.1 Component: Compiler | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: #11155 | Differential Rev(s): Phab:D1558 Wiki Page: | -------------------------------------+------------------------------------- Comment (by alanz): After a brief chat on IRC we changed it in the current patch to {{{#!hs CmmCondBranch :: {..} -> CmmNode O C }}} followed by the field definition. The other option is to surround the field definition with braces and put the return type at the and, as per the original source. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11028#comment:43 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#11028: Refactor ConDecl -------------------------------------+------------------------------------- Reporter: simonpj | Owner: alanz Type: bug | Status: patch Priority: normal | Milestone: 8.0.1 Component: Compiler | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: #11155 | Differential Rev(s): Phab:D1558 Wiki Page: | -------------------------------------+------------------------------------- Comment (by alanz): Clarification: the chat took place shortly after the issue was identified. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11028#comment:44 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#11028: Refactor ConDecl
-------------------------------------+-------------------------------------
Reporter: simonpj | Owner: alanz
Type: bug | Status: patch
Priority: normal | Milestone: 8.0.1
Component: Compiler | Version: 7.10.2
Resolution: | Keywords:
Operating System: Unknown/Multiple | Architecture:
| Unknown/Multiple
Type of failure: None/Unknown | Test Case:
Blocked By: | Blocking:
Related Tickets: #11155 | Differential Rev(s): Phab:D1558
Wiki Page: |
-------------------------------------+-------------------------------------
Comment (by Ben Gamari
#11028: Refactor ConDecl -------------------------------------+------------------------------------- Reporter: simonpj | Owner: alanz Type: bug | Status: closed Priority: normal | Milestone: 8.0.1 Component: Compiler | Version: 7.10.2 Resolution: fixed | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: #11155 | Differential Rev(s): Phab:D1558 Wiki Page: | -------------------------------------+------------------------------------- Changes (by bgamari): * status: patch => closed * resolution: => fixed Comment: The commit above performed the refactoring described here. There are still a few open questions regarding Haddock's handling of this. These will be addressed in due course. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11028#comment:46 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
participants (1)
-
GHC