AMP (#8004) almost finished, review would be nice

[Simon Peyton-Jones]

David

Thanks for doing this.

Some comments on the code.

* Use warnTc or addWarnTc, rather than fiddling with mkPlainWarnMsg etc.

* Ditto in checkShouldInst, instead of returning a (Maybe WarnMsg), 
  just use warnTc inside checkShouldInst.

* Don't use tryTc in tcLookupClassMaybe -- it's a sledgehammer
  to crack a nut. The only classes you are looking up are monad, 
  applicative, alternative, and they will always be found -- or at 
  least if not we have a problem, and tcLookupClass will rightly
  report an error.  So I see no need for this tryTc and matching
  on maybes.

* Why not just getInstEnvs and lookupInstEnv in checkShouldInst,
  much as in TcInteract.matchClassInst?  Instead you are passing 
  parameters around that are readily available in the Tc monad.

* I don't see any documentation.

I don't know about this validation stuff; maybe others can help.  Why not *not* add -fno-warn-amp to GcStage1HcOpts?

Simon

| 1. Validation does not work due to the warnings issued. Since "sh
| validate" uses -Werror, an AMP warning will stop the compilation before
| tests can even be run. Unfortunately, the build system seems to use the
| variables 'GhcStage1HcOpts' in the build process of phase 1, which is of
| course done with the current (7.6.3 or whatever is installed) compiler.
| When adding "-fno-warn-amp" to that variable phase 1 won't build because
| the parameter is unknown to the old compiler.


 Is there some sort of
| -"ignore next parameter if unknown" hack, or is there a smart solution?
| 
| 2. Temporarily removing the -Werror constraint from validate-settings.mk
| (or by using custom-settings.mk) makes the validation build go through.
| However, the testsuite defines a couple of violating modules, therefore
| there is unexpected STDERR output, hence a handful of tests fail. Should
| a fix for this be included in the AMP patch, or be done as a separate
| instance?
| 
| 3. Similarly, GHC defines around 50 offending modules, creating warnings
| in the standard build process. Again, should this be included, or can we
| push that to after the feature freeze and regard it as bugfixing?
| 
| Greetings,
| David/quchen
| 
| _______________________________________________
| ghc-devs mailing list
| ghc-devs at haskell.org
| http://www.haskell.org/mailman/listinfo/ghc-devs