Re: [commit: ghc] master: Refactor tuple constraints (ffc2150)
Hi, This patch broke the ‘Binary’ instance for ‘IfaceType’ in ‘iface/IfaceType.hs’ Specifically, it changed: ``` put_ bh (IfaceLitTy n) = do { putByte bh 30; put_ bh n } ``` to: ``` put_ bh (IfaceLitTy n) = do { putByte bh 7; put_ bh n } ``` However, the ‘get’ definition was left as: ``` 30 -> do n <- get bh return (IfaceLitTy n) ``` I don’t know what’s preferable: - revert the definition of `put_`, - or update the `get` definition to `7` Regards, Christiaan
On 18 May 2015, at 14:45, git@git.haskell.org wrote:
Repository : ssh://git@git.haskell.org/ghc
On branch : master Link : http://ghc.haskell.org/trac/ghc/changeset/ffc21506894c7887d3620423aaf86bc611...
---------------------------------------------------------------
commit ffc21506894c7887d3620423aaf86bc6113a1071 Author: Simon Peyton Jones <simonpj@microsoft.com> Date: Mon May 11 23:19:14 2015 +0100
Refactor tuple constraints
Make tuple constraints be handled by a perfectly ordinary type class, with the component constraints being the superclasses: class (c1, c2) => (c2, c2)
This change was provoked by
#10359 inability to re-use a given tuple constraint as a whole
#9858 confusion between term tuples and constraint tuples
but it's generally a very nice simplification. We get rid of - In Type, the TuplePred constructor of PredTree, and all the code that dealt with TuplePreds - In TcEvidence, the constructors EvTupleMk, EvTupleSel
See Note [How tuples work] in TysWiredIn.
Of course, nothing is ever entirely simple. This one proved quite fiddly.
- I did quite a bit of renaming, which makes this patch touch a lot of modules. In partiuclar tupleCon -> tupleDataCon.
- I made constraint tuples known-key rather than wired-in. This is different to boxed/unboxed tuples, but it proved awkward to have all the superclass selectors wired-in. Easier just to use the standard mechanims.
- While I was fiddling with known-key names, I split the TH Name definitions out of DsMeta into a new module THNames. That meant that the known-key names can all be gathered in PrelInfo, without causing module loops.
- I found that the parser was parsing an import item like T( .. ) as a *data constructor* T, and then using setRdrNameSpace to fix it. Stupid! So I changed the parser to parse a *type constructor* T, which means less use of setRdrNameSpace.
I also improved setRdrNameSpace to behave better on Exact Names. Largely on priciple; I don't think it matters a lot.
- When compiling a data type declaration for a wired-in thing like tuples (,), or lists, we don't really need to look at the declaration. We have the wired-in thing! And not doing so avoids having to line up the uniques for data constructor workers etc. See Note [Declarations for wired-in things]
- I found that FunDeps.oclose wasn't taking superclasses into account; easily fixed.
- Some error message refactoring for invalid constraints in TcValidity
- Haddock needs to absorb the change too; so there is a submodule update
---------------------------------------------------------------
ffc21506894c7887d3620423aaf86bc6113a1071 compiler/basicTypes/BasicTypes.hs | 21 +- compiler/basicTypes/DataCon.hs | 1 - compiler/basicTypes/RdrName.hs | 28 +- compiler/basicTypes/Unique.hs | 28 +- compiler/basicTypes/VarSet.hs | 23 +- compiler/coreSyn/CoreLint.hs | 2 +- compiler/coreSyn/MkCore.hs | 7 +- compiler/coreSyn/PprCore.hs | 4 +- compiler/deSugar/Check.hs | 2 +- compiler/deSugar/DsArrows.hs | 2 +- compiler/deSugar/DsBinds.hs | 25 +- compiler/deSugar/DsCCall.hs | 6 +- compiler/deSugar/DsExpr.hs | 5 +- compiler/deSugar/DsMeta.hs | 840 +-------------------- compiler/deSugar/Match.hs | 4 +- compiler/ghc.cabal.in | 3 +- compiler/ghci/RtClosureInspect.hs | 7 +- compiler/hsSyn/Convert.hs | 4 +- compiler/hsSyn/HsExpr.hs | 3 +- compiler/hsSyn/HsPat.hs | 35 +- compiler/hsSyn/HsTypes.hs | 2 +- compiler/iface/BinIface.hs | 14 +- compiler/iface/BuildTyCl.hs | 4 + compiler/iface/IfaceSyn.hs | 9 +- compiler/iface/IfaceType.hs | 154 ++-- compiler/iface/TcIface.hs | 84 ++- compiler/main/Constants.hs | 3 + compiler/main/HscMain.hs | 11 +- compiler/parser/Parser.y | 20 +- compiler/parser/RdrHsSyn.hs | 164 +++- compiler/prelude/PrelInfo.hs | 28 +- compiler/prelude/PrelNames.hs | 17 - compiler/prelude/PrelRules.hs | 6 +- compiler/prelude/PrimOp.hs | 2 +- compiler/prelude/THNames.hs | 836 ++++++++++++++++++++ compiler/prelude/TysWiredIn.hs | 269 ++++--- compiler/rename/RnEnv.hs | 1 + compiler/rename/RnNames.hs | 42 +- compiler/rename/RnSplice.hs | 6 +- compiler/simplStg/UnariseStg.hs | 10 +- compiler/specialise/Specialise.hs | 3 +- compiler/stranal/WwLib.hs | 6 +- compiler/typecheck/FunDeps.hs | 32 +- compiler/typecheck/TcBinds.hs | 52 +- compiler/typecheck/TcCanonical.hs | 32 - compiler/typecheck/TcErrors.hs | 2 - compiler/typecheck/TcEvidence.hs | 15 +- compiler/typecheck/TcExpr.hs | 10 +- compiler/typecheck/TcGenDeriv.hs | 15 +- compiler/typecheck/TcHsSyn.hs | 5 +- compiler/typecheck/TcHsType.hs | 15 +- compiler/typecheck/TcInstDcls.hs | 4 +- compiler/typecheck/TcInteract.hs | 11 +- compiler/typecheck/TcMType.hs | 1 - compiler/typecheck/TcPat.hs | 2 +- compiler/typecheck/TcRnMonad.hs | 4 + compiler/typecheck/TcSimplify.hs | 1 - compiler/typecheck/TcSplice.hs | 2 +- compiler/typecheck/TcTyClsDecls.hs | 20 +- compiler/typecheck/TcType.hs | 3 - compiler/typecheck/TcValidity.hs | 186 +++-- compiler/types/TyCon.hs | 34 +- compiler/types/Type.hs | 7 +- compiler/types/TypeRep.hs | 11 +- compiler/vectorise/Vectorise/Builtins/Base.hs | 2 +- .../vectorise/Vectorise/Builtins/Initialise.hs | 2 +- compiler/vectorise/Vectorise/Utils/Closure.hs | 4 +- libraries/ghc-prim/GHC/Classes.hs | 44 +- libraries/ghc-prim/GHC/Tuple.hs | 242 +++--- libraries/ghc-prim/GHC/Types.hs | 2 +- .../should_fail/NotRelaxedExamples.stderr | 17 +- .../indexed-types/should_fail/TyFamUndec.stderr | 17 +- testsuite/tests/module/all.T | 2 +- testsuite/tests/module/mod89.hs | 2 + testsuite/tests/module/mod89.stderr | 10 +- .../tests/typecheck/should_fail/T9858a.stderr | 2 +- .../tests/typecheck/should_fail/fd-loop.stderr | 12 +- .../tests/typecheck/should_fail/tcfail108.stderr | 4 +- .../tests/typecheck/should_fail/tcfail154.stderr | 6 +- .../tests/typecheck/should_fail/tcfail157.stderr | 12 +- .../tests/typecheck/should_fail/tcfail213.stderr | 4 +- .../tests/typecheck/should_fail/tcfail214.stderr | 8 +- .../tests/typecheck/should_fail/tcfail220.hsig | 1 - .../tests/typecheck/should_fail/tcfail220.stderr | 8 - utils/genprimopcode/Main.hs | 49 +- utils/haddock | 2 +- 86 files changed, 1985 insertions(+), 1672 deletions(-)
Diff suppressed because of size. To see it, use:
git diff-tree --root --patch-with-stat --no-color --find-copies-harder --ignore-space-at-eol --cc ffc21506894c7887d3620423aaf86bc6113a1071 _______________________________________________ ghc-commits mailing list ghc-commits@haskell.org http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-commits
Eek. Well spotted. The "30" is really odd compared to all the other instances, so best to use "7". I'm validating a patch now. Simon | -----Original Message----- | From: ghc-devs [mailto:ghc-devs-bounces@haskell.org] On Behalf Of | Christiaan Baaij | Sent: 20 May 2015 14:02 | To: ghc-devs@haskell.org | Subject: Re: [commit: ghc] master: Refactor tuple constraints | (ffc2150) | | Hi, | | This patch broke the ‘Binary’ instance for ‘IfaceType’ in | ‘iface/IfaceType.hs’ | Specifically, it changed: | | ``` | put_ bh (IfaceLitTy n) | = do { putByte bh 30; put_ bh n } | ``` | | to: | | ``` | put_ bh (IfaceLitTy n) | = do { putByte bh 7; put_ bh n } | ``` | | However, the ‘get’ definition was left as: | | ``` | 30 -> do n <- get bh | return (IfaceLitTy n) | ``` | | I don’t know what’s preferable: | - revert the definition of `put_`, | - or update the `get` definition to `7` | | Regards, | | Christiaan | | | > On 18 May 2015, at 14:45, git@git.haskell.org wrote: | > | > Repository : ssh://git@git.haskell.org/ghc | > | > On branch : master | > Link : | http://ghc.haskell.org/trac/ghc/changeset/ffc21506894c7887d3620423aaf8 | 6bc6113a1071/ghc | > | >> --------------------------------------------------------------- | > | > commit ffc21506894c7887d3620423aaf86bc6113a1071 | > Author: Simon Peyton Jones <simonpj@microsoft.com> | > Date: Mon May 11 23:19:14 2015 +0100 | > | > Refactor tuple constraints | > | > Make tuple constraints be handled by a perfectly ordinary | > type class, with the component constraints being the | > superclasses: | > class (c1, c2) => (c2, c2) | > | > This change was provoked by | > | > #10359 inability to re-use a given tuple | > constraint as a whole | > | > #9858 confusion between term tuples | > and constraint tuples | > | > but it's generally a very nice simplification. We get rid of | > - In Type, the TuplePred constructor of PredTree, | > and all the code that dealt with TuplePreds | > - In TcEvidence, the constructors EvTupleMk, EvTupleSel | > | > See Note [How tuples work] in TysWiredIn. | > | > Of course, nothing is ever entirely simple. This one | > proved quite fiddly. | > | > - I did quite a bit of renaming, which makes this patch | > touch a lot of modules. In partiuclar tupleCon -> tupleDataCon. | > | > - I made constraint tuples known-key rather than wired-in. | > This is different to boxed/unboxed tuples, but it proved | > awkward to have all the superclass selectors wired-in. | > Easier just to use the standard mechanims. | > | > - While I was fiddling with known-key names, I split the TH Name | > definitions out of DsMeta into a new module THNames. That | meant | > that the known-key names can all be gathered in PrelInfo, | without | > causing module loops. | > | > - I found that the parser was parsing an import item like | > T( .. ) | > as a *data constructor* T, and then using setRdrNameSpace to | > fix it. Stupid! So I changed the parser to parse a *type | > constructor* T, which means less use of setRdrNameSpace. | > | > I also improved setRdrNameSpace to behave better on Exact | Names. | > Largely on priciple; I don't think it matters a lot. | > | > - When compiling a data type declaration for a wired-in thing | like | > tuples (,), or lists, we don't really need to look at the | > declaration. We have the wired-in thing! And not doing so | avoids | > having to line up the uniques for data constructor workers etc. | > See Note [Declarations for wired-in things] | > | > - I found that FunDeps.oclose wasn't taking superclasses into | > account; easily fixed. | > | > - Some error message refactoring for invalid constraints in | TcValidity | > | > - Haddock needs to absorb the change too; so there is a submodule | update | > | > | >> --------------------------------------------------------------- | > | > ffc21506894c7887d3620423aaf86bc6113a1071 | > compiler/basicTypes/BasicTypes.hs | 21 +- | > compiler/basicTypes/DataCon.hs | 1 - | > compiler/basicTypes/RdrName.hs | 28 +- | > compiler/basicTypes/Unique.hs | 28 +- | > compiler/basicTypes/VarSet.hs | 23 +- | > compiler/coreSyn/CoreLint.hs | 2 +- | > compiler/coreSyn/MkCore.hs | 7 +- | > compiler/coreSyn/PprCore.hs | 4 +- | > compiler/deSugar/Check.hs | 2 +- | > compiler/deSugar/DsArrows.hs | 2 +- | > compiler/deSugar/DsBinds.hs | 25 +- | > compiler/deSugar/DsCCall.hs | 6 +- | > compiler/deSugar/DsExpr.hs | 5 +- | > compiler/deSugar/DsMeta.hs | 840 +---------- | ---------- | > compiler/deSugar/Match.hs | 4 +- | > compiler/ghc.cabal.in | 3 +- | > compiler/ghci/RtClosureInspect.hs | 7 +- | > compiler/hsSyn/Convert.hs | 4 +- | > compiler/hsSyn/HsExpr.hs | 3 +- | > compiler/hsSyn/HsPat.hs | 35 +- | > compiler/hsSyn/HsTypes.hs | 2 +- | > compiler/iface/BinIface.hs | 14 +- | > compiler/iface/BuildTyCl.hs | 4 + | > compiler/iface/IfaceSyn.hs | 9 +- | > compiler/iface/IfaceType.hs | 154 ++-- | > compiler/iface/TcIface.hs | 84 ++- | > compiler/main/Constants.hs | 3 + | > compiler/main/HscMain.hs | 11 +- | > compiler/parser/Parser.y | 20 +- | > compiler/parser/RdrHsSyn.hs | 164 +++- | > compiler/prelude/PrelInfo.hs | 28 +- | > compiler/prelude/PrelNames.hs | 17 - | > compiler/prelude/PrelRules.hs | 6 +- | > compiler/prelude/PrimOp.hs | 2 +- | > compiler/prelude/THNames.hs | 836 | ++++++++++++++++++++ | > compiler/prelude/TysWiredIn.hs | 269 ++++--- | > compiler/rename/RnEnv.hs | 1 + | > compiler/rename/RnNames.hs | 42 +- | > compiler/rename/RnSplice.hs | 6 +- | > compiler/simplStg/UnariseStg.hs | 10 +- | > compiler/specialise/Specialise.hs | 3 +- | > compiler/stranal/WwLib.hs | 6 +- | > compiler/typecheck/FunDeps.hs | 32 +- | > compiler/typecheck/TcBinds.hs | 52 +- | > compiler/typecheck/TcCanonical.hs | 32 - | > compiler/typecheck/TcErrors.hs | 2 - | > compiler/typecheck/TcEvidence.hs | 15 +- | > compiler/typecheck/TcExpr.hs | 10 +- | > compiler/typecheck/TcGenDeriv.hs | 15 +- | > compiler/typecheck/TcHsSyn.hs | 5 +- | > compiler/typecheck/TcHsType.hs | 15 +- | > compiler/typecheck/TcInstDcls.hs | 4 +- | > compiler/typecheck/TcInteract.hs | 11 +- | > compiler/typecheck/TcMType.hs | 1 - | > compiler/typecheck/TcPat.hs | 2 +- | > compiler/typecheck/TcRnMonad.hs | 4 + | > compiler/typecheck/TcSimplify.hs | 1 - | > compiler/typecheck/TcSplice.hs | 2 +- | > compiler/typecheck/TcTyClsDecls.hs | 20 +- | > compiler/typecheck/TcType.hs | 3 - | > compiler/typecheck/TcValidity.hs | 186 +++-- | > compiler/types/TyCon.hs | 34 +- | > compiler/types/Type.hs | 7 +- | > compiler/types/TypeRep.hs | 11 +- | > compiler/vectorise/Vectorise/Builtins/Base.hs | 2 +- | > .../vectorise/Vectorise/Builtins/Initialise.hs | 2 +- | > compiler/vectorise/Vectorise/Utils/Closure.hs | 4 +- | > libraries/ghc-prim/GHC/Classes.hs | 44 +- | > libraries/ghc-prim/GHC/Tuple.hs | 242 +++--- | > libraries/ghc-prim/GHC/Types.hs | 2 +- | > .../should_fail/NotRelaxedExamples.stderr | 17 +- | > .../indexed-types/should_fail/TyFamUndec.stderr | 17 +- | > testsuite/tests/module/all.T | 2 +- | > testsuite/tests/module/mod89.hs | 2 + | > testsuite/tests/module/mod89.stderr | 10 +- | > .../tests/typecheck/should_fail/T9858a.stderr | 2 +- | > .../tests/typecheck/should_fail/fd-loop.stderr | 12 +- | > .../tests/typecheck/should_fail/tcfail108.stderr | 4 +- | > .../tests/typecheck/should_fail/tcfail154.stderr | 6 +- | > .../tests/typecheck/should_fail/tcfail157.stderr | 12 +- | > .../tests/typecheck/should_fail/tcfail213.stderr | 4 +- | > .../tests/typecheck/should_fail/tcfail214.stderr | 8 +- | > .../tests/typecheck/should_fail/tcfail220.hsig | 1 - | > .../tests/typecheck/should_fail/tcfail220.stderr | 8 - | > utils/genprimopcode/Main.hs | 49 +- | > utils/haddock | 2 +- | > 86 files changed, 1985 insertions(+), 1672 deletions(-) | > | > Diff suppressed because of size. To see it, use: | > | > git diff-tree --root --patch-with-stat --no-color --find-copies- | harder --ignore-space-at-eol --cc | ffc21506894c7887d3620423aaf86bc6113a1071 | > _______________________________________________ | > ghc-commits mailing list | > ghc-commits@haskell.org | > http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-commits | | _______________________________________________ | ghc-devs mailing list | ghc-devs@haskell.org | http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs
participants (2)
-
Christiaan Baaij -
Simon Peyton Jones