GHC's new diagnostic infrastructure: remaining work
NOTE: This ticket is not actionable, but I am using it as a dumping ground to collect everything which needs to be done to consider #18516 morally "finished". Me and @rae will eventually break this down into smaller tasks.
The first round of #18516 is approaching its natural end, but the work in this area in not done. This is an unordered collection of things which are missing and would be nice to do:
-
Port the remaining TcRnmessages to use the newTcRnMessagediagnostic type. Currently, for a lot of GHC typecheck- rename errors and warnings we "cheat", i.e we rewrap the oldSDoc-style messages into anTcRnUnknownMessage SDoctype constructor (the type is slightly more complicated than this, but that gives you the idea).
Action: We need togrepfor every occurrence ofTcRnUnknownMessageand convert that into a proper type constructor.
Ticket(s): #19926 (closed), #19927 (closed), #19930 (closed), #20114 (closed), #20115 (closed), #20116 (closed), #20117 (closed), #20118 (closed), #20119 (closed)
Example:GHC.Tc.Module.tcRnModulecurrently emits:[...] err_msg = mkPlainErrorMsgEnvelope loc $ TcRnUnknownMessage $ mkPlainError noHints $ text "Module does not have a RealSrcSpan:" <+> ppr this_mod [...]We need to add a new type constructor for
TcRnMessagecalled, for example,TcRnModuleHasNoRealSrcSpan !Module, write a proper comment for it, add this constructor to theDiagnosticinstance insideGHC.Tc.Errors.Pprand eventually replace thaterr_msgwith something like:[...] err_msg = TcRnModuleHasNoRealSrcSpan this_mod [...]
-
Port the remaining Dsmessages to use the newDsMessagediagnostic type. This is largely what I am already doing as part of !5719 (closed), but I am currently trying to fix the humongous increase in parser deps
Action: Me (Alfredo) needs to make this eventually mergeable, or pass the baton to someone which can make it mergeable.
Example: Same idea as per above.
-
Port the remaining Drivermessages to use the newDriverMessagediagnostic type.
Action:grepforDriverUnknownMessageand follow the same recipe as described for theTcRnMessagecase.
Example: Same idea as per theTcRnMessage.
-
Document all the type constructors for all the diagnostic types and all the hints. This is an heroic (and admittedly a bit tedious) task, but also an important one. Currently we are trying our best to document each type constructor
to give information about what this message is, which flag (if any) controls it, and which are the tests that triggers it.
Action: Pick one or more undocumented type constructors. Choose from the pool ofGhcMessage,TcRnMessage,DsMessage,PsMessageandGhcHint. Follow the blueprint for the other (documented) constructors. If a tests is missing, and is not too complicated to add, consider adding it.
Example: Currently thePsWarnUnrecognisedPragmatype constructor ofGHC.Parser.Errors.Typeshas a single-line, bland comment. Replace it with something like:{-| PsWarnUnrecognisedPragma is a warning (controlled by the -Wunrecognised-pragmas flag) that occurs when trying to use a pragma that GHC doesn't know. Example(s): module Foo where {-# THISISATYPO foo #-} foo :: () foo = () Test case(s): parser/should_compile/read064 -} | PsWarnUnrecognisedPragma
-
Improve existing type constructors. To go back to the previous example, currently PsWarnUnrecognisedPragmadoesn't take the offending pragma as a type argument, which means that GHC can only produce a warning message like "Unrecognised pragma", but in principle it would be nice to also report which pragma that is.
Action: Work backward: pick a diagnostic message in GHC you don't like or you find incomplete, trace back which type constructor that correspond, and improve it.
Example: ThePsWarnUnrecognisedPragmais a good candidate. Eventually it might not be feasible to improve it if this is emitted in theLexer.yand it's cumbersome to get the parsed pragma. If that's the case, pick another one.
-
Improve the diagnostic messages and hints. Currently GHC is not disciplined when it comes to emitting diagnostic messages and hints, in the sense that during #18516 we largely moved the existing SDocmessages into thePprmodule, without changing too much the same and form of these diagnostics to not break too many tests. However, in an ideal world, we should be consistent.
Action: Pick a diagnostic message (or a hint) you are not happy about, and try to make it either more precise, or more fluent, or more consistent with the rest.
Example: There are some hints which are posed as a question, with a question mark at the end, but in this new "brave new world(TM)" they should really be sentences, as they appears in a list of "Suggested fixes".
-
Move hints in the Parser.Errors.Pprdiagnostics into proper types. SomePsMessageconstructors are still emitting hints bundled with diagnostic messages, which means that callingdiagnosticHintson these constructors will yieldnoHints, which is not very useful.
Action: Pick a diagnostic message where the hint is bundled with the diagnostic message (look at thediagnosticMessageimplementation) and move it into a separate hint.
Tickets: #20055 (closed)
-
Review usages of MessageClass, and make sure those are appropriate. During #18516, we disentangled theSeverityused for diagnostics with the severity of messages emitted by GHC viaputLogMsg, and theMessageClassabstraction was born. There might be diagnostic messages emitted in GHC that are not using theMCDiagnosticconstructor, but one of the otherMCOutputorMCFatal. To say it differently, everything which is not logged usingMCDiagnosticwon't enter the proper diagnostic infrastructure, and it will be treated by GHC like any other "general purpose" logging message, which means, for example, that-Werrorwon't be honoured etc etc.
Action: Do agrepfor every type constructor ofMessageClass, and make sure that everything we is morally a diagnostic message is actually outputted and treated like so.
Example:GHC.Linker.ExtraObj.mkExtraObjToLinkIntoBinaryhas the following:mkExtraObjToLinkIntoBinary :: Logger -> TmpFs -> DynFlags -> UnitState -> IO (Maybe FilePath) mkExtraObjToLinkIntoBinary logger tmpfs dflags unit_state = do when (gopt Opt_NoHsMain dflags && haveRtsOptsFlags dflags) $ logInfo logger dflags $ withPprStyle defaultUserStyle (text "Warning: -rtsopts and -with-rtsopts have no effect with -no-hs-main." $$ text " Call hs_init_ghc() from your main() function to set these options.")I am not entirely sure if this should be a "proper" GHC warning or if it ought to stay a general informational message, but these are the kind of candidates we should review.
-
Add a JSONinterface. This is a big one in itself, but there might be tooling that would be interested in just getting its hands on the JSON output of the diagnostics that GHC emits, without worrying about linking against the GHC API and work with the Haskell values. Action: I am not sure what is the best plan of action, but ideally we should be able to define a JSON instance for our diagnostic types. The crucial thing here would be to make the format of this JSON documented and predictable, so that it doesn't break with every GHC release and users don't have to read the GHC source code to reverse engineer the format
. Ticket(s): #19278
Little general improvements
-
@shayne-fletcher-da suggested to rename getMessages (either in the Parser API or in the getter of the Messagesnewtype) to avoid import collisions.
Issue: #19920 (closed)
There are probably many other things I am forgetting right now, and I will add them as they come to mind.