Addresses Trac #9766
Details
Diff Detail
- Repository
- rGHC Glasgow Haskell Compiler
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
@dreixel I just ran validate w/ D493 on top of a7c29721535d636fb16ab756b3f44224e04a5113, and got the following compile error:
$ make ===--- building phase 0 make -r --no-print-directory -f ghc.mk phase=0 phase_0_builds make[1]: Nothing to be done for 'phase_0_builds'. ===--- building phase 1 make -r --no-print-directory -f ghc.mk phase=1 phase_1_builds make[1]: Nothing to be done for 'phase_1_builds'. ===--- building final phase make -r --no-print-directory -f ghc.mk phase=final all "inplace/bin/ghc-cabal" hscolour libraries/template-haskell dist-install Running hscolour for template-haskell-2.10.0.0... Preprocessing library template-haskell-2.10.0.0... "/home/hvr/Haskell/GHC/ghc/inplace/bin/haddock" --odir="libraries/template-haskell/dist-install/doc/html/template-haskell" --no-tmp-comp-dir --dump-interface=libraries/template-haskell/dist-install/doc/html/template-haskell/template-haskell.haddock --html --hoogle --title="template-haskell-2.10.0.0: Support library for Template Haskell" --prologue="libraries/template-haskell/dist-install/haddock-prologue.txt" --hide=Language.Haskell.TH.Lib.Map --read-interface=../base-4.8.0.0,../base-4.8.0.0/src/%{MODULE/./-}.html\#%{NAME},libraries/base/dist-install/doc/html/base/base.haddock --read-interface=../pretty-1.1.1.1,../pretty-1.1.1.1/src/%{MODULE/./-}.html\#%{NAME},libraries/pretty/dist-install/doc/html/pretty/pretty.haddock --optghc=-hisuf --optghc=dyn_hi --optghc=-osuf --optghc=dyn_o --optghc=-hcsuf --optghc=dyn_hc --optghc=-fPIC --optghc=-dynamic --optghc=-H32m --optghc=-O --optghc=-Werror --optghc=-Wall --optghc=-H64m --optghc=-O0 --optghc=-this-package-key --optghc=templ_C42vZOlutVC8TW7eaTG1Aw --optghc=-hide-all-packages --optghc=-i --optghc=-ilibraries/template-haskell/. --optghc=-ilibraries/template-haskell/dist-install/build --optghc=-ilibraries/template-haskell/dist-install/build/autogen --optghc=-Ilibraries/template-haskell/dist-install/build --optghc=-Ilibraries/template-haskell/dist-install/build/autogen --optghc=-Ilibraries/template-haskell/. --optghc=-optP-include --optghc=-optPlibraries/template-haskell/dist-install/build/autogen/cabal_macros.h --optghc=-package-key --optghc=base_Fk2bxXXwDEyIsDbL6nDNHy --optghc=-package-key --optghc=prett_Gs6aZ8cu8OQ4HFwF3s4B6l --optghc=-Wall --optghc=-this-package-key --optghc=template-haskell --optghc=-XHaskell2010 --optghc=-O2 --optghc=-O --optghc=-dcore-lint --optghc=-fno-warn-deprecated-flags --optghc=-fno-warn-tabs --optghc=-no-user-package-db --optghc=-rtsopts --optghc=-fno-warn-inline-rule-shadowing --optghc=-odir --optghc=libraries/template-haskell/dist-install/build --optghc=-hidir --optghc=libraries/template-haskell/dist-install/build --optghc=-stubdir --optghc=libraries/template-haskell/dist-install/build --source-module=src/%{MODULE/./-}.html --source-entity=src/%{MODULE/./-}.html#%{NAME} libraries/template-haskell/./Language/Haskell/TH.hs libraries/template-haskell/./Language/Haskell/TH/Lib.hs libraries/template-haskell/./Language/Haskell/TH/Ppr.hs libraries/template-haskell/./Language/Haskell/TH/PprLib.hs libraries/template-haskell/./Language/Haskell/TH/Quote.hs libraries/template-haskell/./Language/Haskell/TH/Syntax.hs libraries/template-haskell/./Language/Haskell/TH/Lib/Map.hs +RTS -tlibraries/template-haskell/dist-install/doc/html/template-haskell/template-haskell.haddock.t --machine-readable Haddock coverage: 0% ( 0 / 5) in 'Language.Haskell.TH.Lib.Map' haddock: panic! (the 'impossible' happened) (GHC version 7.9.20141124 for x86_64-unknown-linux): kindFunResult Please report this as a GHC bug: http://www.haskell.org/ghc/reportabug libraries/template-haskell/ghc.mk:4: recipe for target 'libraries/template-haskell/dist-install/doc/html/template-haskell/template-haskell.haddock' failed make[1]: *** [libraries/template-haskell/dist-install/doc/html/template-haskell/template-haskell.haddock] Error 1 Makefile:71: recipe for target 'all' failed make: *** [all] Error 2
Currently this diff panics when haddock runs on Language.Haskell.TH.Syntax. Removing all derive Generic clauses from that file makes it go through. Surprisingly, removing the NoInline constructor on line 1268 also makes the panic go away (though things fail later on due to a missing constructor).
I have no idea what could be causing this, as this diff does not touch haddock, and everything else but this passes successfully. I also don't exactly know how to compile haddock in debug mode so that I can obtain a stack trace, for example. So I could really use some help with further debugging.
The diff does not need to touch Haddock for something to crap out while it runs: the error comes from GHC, *not* Haddock. What this indicates is that Haddock's use of GHC API causes a GHC panic with your changes.
There are two ways to debug this: either start entering traces &c inside GHC or Haddock. I highly recommend you do it inside Haddock as that is far quicker to recompile than all of GHC. Of course we would still need to run Haddock on the offending module. To do that you could run the command similar to what hvr's log shows:
"/home/hvr/Haskell/GHC/ghc/inplace/bin/haddock" --odir="libraries/template-haskell/dist-install/doc/html/template-haskell" --no-tmp-comp-dir --dump-interface=libraries/template-haskell/dist-install/doc/html/template-haskell/template-haskell.haddock --html --hoogle --title="template-haskell-2.10.0.0: Support library for Template Haskell" --prologue="libraries/template-haskell/dist-install/haddock-prologue.txt" --hide=Language.Haskell.TH.Lib.Map --read-interface=../base-4.8.0.0,../base-4.8.0.0/src/%{MODULE/./-}.html\#%{NAME},libraries/base/dist-install/doc/html/base/base.haddock --read-interface=../pretty-1.1.1.1,../pretty-1.1.1.1/src/%{MODULE/./-}.html\#%{NAME},libraries/pretty/dist-install/doc/html/pretty/pretty.haddock --optghc=-hisuf --optghc=dyn_hi --optghc=-osuf --optghc=dyn_o --optghc=-hcsuf --optghc=dyn_hc --optghc=-fPIC --optghc=-dynamic --optghc=-H32m --optghc=-O --optghc=-Werror --optghc=-Wall --optghc=-H64m --optghc=-O0 --optghc=-this-package-key --optghc=templ_C42vZOlutVC8TW7eaTG1Aw --optghc=-hide-all-packages --optghc=-i --optghc=-ilibraries/template-haskell/. --optghc=-ilibraries/template-haskell/dist-install/build --optghc=-ilibraries/template-haskell/dist-install/build/autogen --optghc=-Ilibraries/template-haskell/dist-install/build --optghc=-Ilibraries/template-haskell/dist-install/build/autogen --optghc=-Ilibraries/template-haskell/. --optghc=-optP-include --optghc=-optPlibraries/template-haskell/dist-install/build/autogen/cabal_macros.h --optghc=-package-key --optghc=base_Fk2bxXXwDEyIsDbL6nDNHy --optghc=-package-key --optghc=prett_Gs6aZ8cu8OQ4HFwF3s4B6l --optghc=-Wall --optghc=-this-package-key --optghc=template-haskell --optghc=-XHaskell2010 --optghc=-O2 --optghc=-O --optghc=-dcore-lint --optghc=-fno-warn-deprecated-flags --optghc=-fno-warn-tabs --optghc=-no-user-package-db --optghc=-rtsopts --optghc=-fno-warn-inline-rule-shadowing --optghc=-odir --optghc=libraries/template-haskell/dist-install/build --optghc=-hidir --optghc=libraries/template-haskell/dist-install/build --optghc=-stubdir --optghc=libraries/template-haskell/dist-install/build --source-module=src/%{MODULE/./-}.html --source-entity=src/%{MODULE/./-}.html#%{NAME} libraries/template-haskell/./Language/Haskell/TH.hs libraries/template-haskell/./Language/Haskell/TH/Lib.hs libraries/template-haskell/./Language/Haskell/TH/Ppr.hs libraries/template-haskell/./Language/Haskell/TH/PprLib.hs libraries/template-haskell/./Language/Haskell/TH/Quote.hs libraries/template-haskell/./Language/Haskell/TH/Syntax.hs libraries/template-haskell/./Language/Haskell/TH/Lib/Map.hs +RTS -tlibraries/template-haskell/dist-install/doc/html/template-haskell/template-haskell.haddock.t --machine-readable
Personally I would start by inserting prints (traces) in various places in Haddock to first establish how far it gets before GHC panics. Once that's narrowed down, I would trace what function with what values it calls GHC with. From there it should be much easier to come up with smaller test case and chase it up and fix it in GHC itself.
(Requesting changes pending ./validate failure, to move this out of the blocking queue)
@dreixel, this looks like a nice use of promotion. Is there any chance you will be continuing this work?
I'm afraid not, at least not in the foreseeable future. I'd love it if someone were to pick it up, though!
I rebased the changes on top of master and, interestingly, could not reproduce the Haddock errors related to template-haskell. I hope that's a good sign. I'm going to commandeer this revision and try to apply my updates to see what happens.
I'm really happy to see someone pick this up. It looks like the testsuite needs to be updated, however.
Yay, it passes validate!
By the way, a side-effect of this Diff is that the ill-behaved Generic instances for Char, Double, Float, and Int were removed. As a result, Trac #9526 and Trac #10512 are probably moot now.
Thanks for picking this up! I'm not surprised that the TH error is gone; this patch didn't change anything TH-related, I think it was just tripping on some unrelated bug which has been fixed in the meantime.
This looks pretty good to me. I've left an idea for refinement inline.
libraries/base/GHC/Generics.hs | ||
---|---|---|
859 | This type needs at least a brief Haddock comment. | |
861 | Why not split the no-selector case from MetaSel (e.g. introduce a MetaNoSel constructor or make the field Maybe Symbol)? Seems a bit cleaner than just using an empy string. | |
testsuite/tests/perf/compiler/all.T | ||
524 | Nice improvement. All of those dummy types, instances, etc. really added up I guess. |
Okay, after refreshing myself over Trac #9766 and reading the main meat of the patch, I think this is good to go in. Ben, do you have any other concerns?
I agree; this looks good.
@RyanGlScott, once this lands do you think you could update the wiki page to reflect the fact that the proposal is now reality?
All in all, looks very nice. Thanks for this. Added two really minor comments.
I've also briefly wondered whether it'd be better to use a dedicated enumeration type as opposed to Bool to encode the isNewtype status, because the False / True in the encoding is not particularly descriptive. But I think it's ok to leave it like this for now.
compiler/prelude/PrelNames.hs | ||
---|---|---|
885–886 | This is a minor point, but by removing the Par1 declaration from between U1 and Rec1 and changing Par0 to Par1 here, the effect is that the order of these declarations does not longer match the order of assigning the numeric ids elsewhere. | |
1633 | See my other comment about Par1. |
@bgamari: Sure, I can pretty up the wiki.
@kosmikus: The isNewtype function has been around for a bit, so I'd be reluctant to change its type signature. I wouldn't be opposed to changing the type-level representation to something like data IsNewtype = Newtype | NotNewtype (name suggestions welcome) and changing the Datatype 'MetaData instance to something like
instance (KnownSymbol n, KnownSymbol m, KnownSymbol p, SingI nt) => Datatype ('MetaData n m p nt) where ... isNewtype _ = fromSing (sing :: Sing nt) == Newtype
Would it be appropriate to introduce data ConIsRecord = Record | NoRecord for conIsRecord as well?
@RyanGlScott, it appears that there is a spurious change here that needs to be fixed. See inline comment.
compiler/prelude/PrelNames.hs | ||
---|---|---|
412 | It looks like you accidentally deleted dATA_MAYBE here which is causing validation to fail. |
compiler/prelude/PrelNames.hs | ||
---|---|---|
412 | I'm confused. The history doesn't show that a dATA_MAYBE ever existed in PrelNames AFAIK. Am I missing something? |
Looks good. I'll merge once the validation has finished.
compiler/prelude/PrelNames.hs | ||
---|---|---|
412 | That is incredibly odd. Indeed whatever oddness arc thought up seems to have dissipated. |