Refactor validity checking for constraints

Authored by simonpj on Jul 4 2018, 9:17 AM.

Description

Refactor validity checking for constraints

There are several changes here.

  • TcInteract has gotten too big, so I moved all the class-instance matching out of TcInteract into a new module ClsInst. It parallels the FamInst module.

    The main export of ClsInst is matchGlobalInst. This now works in TcM not TcS.
  • A big reason to make matchGlobalInst work in TcM is that we can then use it from TcValidity.checkSimplifiableClassConstraint. That extends checkSimplifiableClassConstraint to work uniformly for built-in instances, which means that we now get a warning if we have givens (Typeable x, KnownNat n); see Trac Trac #15322.
  • This change also made me refactor LookupInstResult, in particular by adding the InstanceWhat field. I also changed the name of the type to ClsInstResult.

    Then instead of matchGlobalInst reporting a staging error (which is inappropriate for the call from TcValidity), we can do so in TcInteract.checkInstanceOK.
  • In TcValidity, we now check quantified constraints for termination. For example, this signature should be rejected: f :: (forall a. Eq (m a) => Eq (m a)) => blah as discussed in Trac Trac #15316. The main change here is that TcValidity.check_pred_help now uses classifyPredType, and has a case for ForAllPred which it didn't before.

    This had knock-on refactoring effects in TcValidity.
RyanGlScott added a subscriber: RyanGlScott.EditedJul 5 2018, 8:52 AM

This commit appears to have broken the tcrun045 test:

=====> tcrun045(normal) 1 of 1 [0, 0, 0]
cd "./typecheck/should_run/tcrun045.run" &&  "/home/rgscott/Software/ghc/inplace/test   spaces/ghc-stage2" -c tcrun045.hs -dcore-lint -dcmm-lint -no-user-package-db -rtsopts -fno-warn-missed-specialisations -fshow-warning-groups -fdiagnostics-color=never -fno-diagnostics-show-caret -dno-debug-output  
Actual stderr output differs from expected:
diff -uw "./typecheck/should_run/tcrun045.run/tcrun045.stderr.normalised" "./typecheck/should_run/tcrun045.run/tcrun045.comp.stderr.normalised"
--- ./typecheck/should_run/tcrun045.run/tcrun045.stderr.normalised      2018-07-05 09:37:19.945418084 -0400
+++ ./typecheck/should_run/tcrun045.run/tcrun045.comp.stderr.normalised 2018-07-05 09:37:19.945418084 -0400
@@ -1,8 +1,6 @@
 
 tcrun045.hs:11:10:
      Illegal implicit parameter ‘?imp::Int’
-     In the context: ?imp::Int
-      While checking an instance declaration
       In the instance declaration for ‘C Int’
 
 tcrun045.hs:24:1:
@@ -13,6 +11,4 @@
 
 tcrun045.hs:27:10:
      Illegal implicit parameter ‘?imp::Int’
-     In the context: ?imp::Int
-      While checking an instance declaration
       In the instance declaration for ‘D Int’
*** unexpected failure for tcrun045(normal)

Unexpected results from:
TEST="tcrun045"

I've decided to accept the new output in commit 45f0026818402aa08398131507bc587ea4a2b387.

/compiler/typecheck/TcValidity.hs
857

This ends up polluting stderr with lots of check_class ~ messages, so I commented this line out in commit 048f72ed7dfe8264cbd95006f8206bad764aeede.

RyanGlScott added inline comments.Jul 5 2018, 8:54 AM
/compiler/typecheck/TcValidity.hs
857

Sorry, that should be commit 9b26aa09036e6cd5ceb92b01350ba9d33fadb933.