Skip to content

New design for Errors internals (DiagnosticReason, MessageClass & co)

Alfredo Di Napoli requested to merge wip/adinapoli-message-class-part-2 into master

This is part of #18516.

This is the latest sublimation of the design me and @rae have been discussing (with always-valuable feedback from @simonpj).

As a recap, the current Severity in HEAD is not ideal, so Richard proposed a split, which resulted into a MessageClass and a Severity type, to keep things clean and separated. I have implemented that (see !4959 (closed)) but this wasn't fully satisfactory in the context of #18516, see !4798 (d9057446, comment 329250) .

This MR implements Richard's design, with minor changes. Severity is now a plain enum, a new type DiagnosticReason is added that supersedes both WarnReason and the old Severity. A function reasonSeverity :: DiagnosticReason -> Severity allows us to compute the severity out of a DiagnosticReason. Dynflags are not required as input to the function, because we classify a diagnostic with the right reason at birth.

Lo and behold, when applied to MR !4798 (closed), it will allow us to write something like:

mkTcRnDiagnostic :: DynFlags -> SrcSpan -> PrintUnqualified -> TcRnDiagnostic -> MsgEnvelope TcRnMessage
mkTcRnDiagnostic dflags loc printer msg =
  mkMsgEnvelope loc printer (TcRnMessage $ DiagnosticMessage msg tcRnReason)
  where
    tcRnReason :: DiagnosticReason
    tcRnReason = case msg of
      TcRnUnknownMessage decMsg
        -> diagnosticReason decMsg
      TcRnMessageWithUnitState _ decMsg
        -> diagnosticReason decMsg
      TcRnBadTelescope{}
        -> ErrReason
      TcRnOutOfScope{}
        -> ErrReason
      TcRnOutOfScopeHole{}
        -> let defer_holes           = gopt Opt_DeferTypedHoles dflags
               warn_holes            = wopt Opt_WarnTypedHoles dflags
               rea | not defer_holes = ErrReason
                   | warn_holes      = WarnReasonWithFlag Opt_WarnTypedHoles
                   | otherwise       = ErrReason
           in rea

This now gets rid of the duplication Richard complained about, and now mkTcRnDiagnostic forbids the creation of impossible states, because the DiagnosticReason is calculated in the body of the function and is derived from the input TcRnDiagnostic, which is what in !4798 (closed) we called TcRnMessage.

This is a big MR and it's "bold" in spirit, as it replaces the WarnReason from GHC.Session.Flags, but I think it's the right design.

Merge request reports