Skip to content

Draft: Dead code warning consistency (#17543)

cgibbard requested to merge obsidiansystems/ghc:dead-code-consistency into master

This MR replaces !2080 (closed) and !3331 (closed), since those were confusingly separate. In an attempt to deal with the issue #17543 the goal here is to handle cases where a user wants to work with absurd constraints in a more flexible fashion. While such constraints generally indicate the presence of dead code somewhere in the program, especially when generating code, or working with type-level proofs of properties, it can sometimes be hard to avoid writing code that deals with absurd constraints in some way.

We want to turn the cases where absurd constraints would indicate the presence of dead code into warnings rather than errors, so that users are still informed that they might have written either something containing dead code, or which requires writing something containing dead code in order to use, while still allowing for cases where generated code runs head-long into absurdity e.g. by pattern matching on too many GADT constructors.

This code removes a check from simplifyAmbiguityCheck that generated these errors originally, but relaxes the conditions under which warnings are generated by reportWanteds.

When we first approached this, we immediately started noticing new warnings being generated by derived instances, for example, the Ord instance for certain GADTs contains code that triggers the new warning. The fact that GHC's own generated code runs into the issue we're trying to solve (and was previously evading the errors in the constraint solver by fiat) is a good example of why we'd like the compiler to be more permissive about these constraints. It's hard for machine-generated code to solve the potentially undecidable problems about whether pattern matching a GADT constructor would induce an absurd situation, without repeating an implementation of GHC's constraint solver -- and even GHC doesn't try to use its own constraint solver to determine what the generated code ought to be! In the case of these instances (specifically the Ord instance for the GADT in T15398), they're still only warnings if written out by hand, but regardless, the fact that sometimes these things become errors depending on where in the compiler they get detected is the issue we're trying to fix.

So we'd like to not have the user be presented with these warnings stemming from derived instances. GHC already "knows" that these insoluble constraints were generated by code created by a deriving declaration, as it prints "To see the code I am typechecking, use -ddump-deriv", however, it only knew it in the form of what is essentially an SDoc that was appended to the list (tcl_ctxt) in the TcLclEnv. In order to obtain the information that we're in a function which is being derived and be able to skip the warning, we added a new tcl_deriving :: Bool field to TcLclEnv, which is turned on at the same moment that the aforementioned message is. Note that there is already a field tcl_in_gen_code which sounds like it would be appropriate for this kind of use, but:

  1. It's presently used only for code that is generated by rebindable syntax.
  2. It's unset by anything which uses setSrcSpan on a real code span, which happens in several locations throughout the deriving mechanism to indicate to the user which part of their deriving declaration was involved in an error.

So, it made some sense to have a second flag. If someone can spot an easier way to go about avoiding this warning, I'd be happy to try it out though.

Another quirk we ran into and don't completely understand is that our first attempt at using tcl_deriving (ctLocEnv (ctLoc ct)) in mkGivenWarningReporter bypassed the entire rest of that function, but somehow that ended up converting the warning back into an error. Apparently some part of what happens right before the reportWarning has an important side-effect for dismissing the bad constraint that I haven't yet fully comprehended.

Fixes #17543

Edited by Andreas Klebinger

Merge request reports