Skip to content

GitLab

  • Projects
  • Groups
  • Snippets
  • Help
    • Loading...
  • Help
    • Help
    • Support
    • Community forum
    • Submit feedback
  • Sign in / Register
GHC
GHC
  • Project overview
    • Project overview
    • Details
    • Activity
    • Releases
  • Repository
    • Repository
    • Files
    • Commits
    • Branches
    • Tags
    • Contributors
    • Graph
    • Compare
    • Locked Files
  • Issues 4,389
    • Issues 4,389
    • List
    • Boards
    • Labels
    • Service Desk
    • Milestones
    • Iterations
  • Merge Requests 372
    • Merge Requests 372
  • Requirements
    • Requirements
    • List
  • CI / CD
    • CI / CD
    • Pipelines
    • Jobs
    • Schedules
    • Test Cases
  • Operations
    • Operations
    • Incidents
    • Environments
  • Analytics
    • Analytics
    • CI / CD
    • Code Review
    • Insights
    • Issue
    • Repository
    • Value Stream
  • Wiki
    • Wiki
  • Snippets
    • Snippets
  • Members
    • Members
  • Activity
  • Graph
  • Create a new issue
  • Jobs
  • Commits
  • Issue Boards
Collapse sidebar
  • Glasgow Haskell Compiler
  • GHCGHC
  • Issues
  • #18610

Closed
Open
Opened Aug 26, 2020 by Ryan Scott@RyanGlScottMaintainer

Add mechanism to suppress pattern-match exhaustiveness checking at a finer granularity

Section 6 of Lower Your Guards (which describes GHC's current pattern-match coverage checker) notes an unfortunate side effect of making the exhaustiveness checker more intelligent. The issue can summarized by how the coverage checker treats this function from the HsYAML library:

go' _ _ _ xs | False = error (show xs)
go' _ _ _ xs = err xs

The exact details of what this function does aren't important. The important bit is that while previous versions of GHC would deem go' to be exhaustive, a modern GHC will now flag the first equation (with the False guard) as redundant:

warning: [-Woverlapping-patterns]
    Pattern match is redundant
    In an equation for ‘go'’: go' _ _ _ xs = ...
  |
  | go' _ _ _ xs | False = error (show xs)
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

And indeed, there is no way to possibly reach the GRHS of that equation, so GHC is well within its rights to complain. But there is something a bit unfortunate about this, since the authors of HsYAML likely put that equation in there for a reason: it is debugging code that the authors may occasionally use by commenting out the False part. Moreover, leaving the whole equation intact (rather than commenting it out) ensures that this code will not bitrot as the surrounding code changes over time. From this perspective, telling users to remove this equation because it is redundant misses the entire point.

HsYAML wasn't even the only example of this phenomenon in the wild. Section 6 of Lower Your Guards also found similar styles of code in Cabal and network, and since that paper only tested libraries in head.hackage, it is quite likely that there are other examples as well.¹ In order to avoid punishing users for writing such debugging code, we should introduce some mechanism to suppress -Woverlapping-patterns warnings on an equation-by-equation basis. One option, of course, is to just enable -Wno-overlapping-patterns at the top of a module, but that will disable coverage checking for all functions in that module, which is unlikely to be desirable.

One option is to introduce a primitive function considerAccessible such that when it is used, like in the following example:

go' _ _ _ xs | considerAccessible False = error (show xs)

Then considerAccessible False will not be flagged as redundant. We would likely have to add special support for considerAccessible in the coverage checker internals in order to make this work.

As an aside, one might wonder if it is possible to define considerAccessible without needing any special magic in the compiler. A first approximation is this:

considerAccessible :: a -> a
considerAccessible = id

This alone is enough to convince GHC that a considerAccessible False guard is no longer redundant, but it doesn't go far enough. Consider this more interesting example, adapted from Cabal:

f = case (False, False) of
      (True,  True)  -> "Debug 1"
      (False, True)  -> "Debug 2"
      (True,  False) -> "Debug 3"
      (False, False) -> ""

In this case expression, only the (False, False) case alternative can be reached. All other alternatives are debugging-only, and GHC flags them as redundant. You might think that you could utilize the considerAccessible = id definition above to suppress these warnings like so:

f = case (considerAccessible False, considerAccessible False) of ...

Surprisingly, this will only suppress the warnings for the (True, True) alternative, and GHC will still produce these two warnings:

warning: [-Woverlapping-patterns]
    Pattern match is redundant
    In a case alternative: (False, True) -> ...
  |
  |       (False, True)  -> "Debug 2"
  |       ^^^^^^^^^^^^^^^^^^^^^^^^^^^

warning: [-Woverlapping-patterns]
    Pattern match is redundant
    In a case alternative: (True, False) -> ...
  |
  |       (True,  False) -> "Debug 3"
  |       ^^^^^^^^^^^^^^^^^^^^^^^^^^^

The reason is because GHC's coverage checker is smart enough to realize that the expression considerAccessible False is used twice in the case scrutinee, and since these two expressions are identical, they must evaluate to the same result. As a consequence, GHC knows that the scrutinee must evaluate to either (True, True) or (False, False), which implies that the combinations of (False, True) and (True, False) cannot occur. Of course, we would like to suppress all of these warnings, which means that we will need a smarter version of considerAccessible than what is defined above.

cc @sgraf812

¹ @trac-lennart recalls experiencing this issue in a Q&A session for Lower Your Guards at ICFP 2020.

Assignee
Assign to
None
Milestone
None
Assign milestone
Time tracking
None
Due date
None
Reference: ghc/ghc#18610