Major Overhaul of Pattern Match Checking (Fixes #595)
AbandonedPublic

Authored by bgamari on Nov 27 2015, 4:59 AM.

Details

Reviewers
gkaracha
goldfire
austin
Trac Issues
#595
Summary

I am posting this on behalf of Georgios Karachalias so that this patch has some
representation on Phabricator.

This is a rewrite of the pattern-match exhaustiveness checker which, among
other things, substantially improves detection of redundant patterns in matches
on GADTs.

See https://ghc.haskell.org/trac/ghc/wiki/PatternMatchCheckImplementation
and https://ghc.haskell.org/trac/ghc/wiki/PatternMatchCheck
and Georgios' paper *GADTs meet their match*
for details.

Test Plan

Validate

Diff Detail

Lint
Lint Skipped
Unit
Unit Tests Skipped
Build Status
Buildable 7112
Build 8501: GHC Patch Validation (amd64/Linux)
bgamari retitled this revision from to Pattern Match Checking (Fixes #595).Nov 27 2015, 4:59 AM
bgamari updated this object.
bgamari edited the test plan for this revision. (Show Details)
bgamari updated the Trac tickets for this revision.

I've left a few comments inline most of which are exceedingly pedantic. If you have time feel free to take care of them but they certainly aren't critical. Either way, try to keep these things in mind in the future.

compiler/deSugar/Check.hs
63

No need for LaTeX markup.

66

This isn't quite right as the Utilities comment will be applied as a description to the nullaryConPattern definition. Instead you probably meant

-- * Utilities
69

At this point we are phasing out the old LaTeX-style comments. Feel free to drop the \subsection.

130

For greppability it's best to be consistent in formatting references to Notes. Ideally this would read as -- Note [Translating As Patterns].

137

Looks like this can be dropped.

162

What is going on with this comment block?

180

These could probably live in UniqSupply.

713

Turn these into proper Haddock comments.

718

Use Haddock section.

730

Use Haddock section.

832

No need for LaTeX markup.

968

Eliminate second |.

1030

It's not clear to me what the | -- means.

1216

Use Haddock section.

compiler/deSugar/PmExpr.hs
57

Use Haddock section.

89

Use Haddock section.

103

Use Haddock section.

143

Use Haddock section.

262

Use Haddock section.

gkaracha commandeered this revision.Nov 27 2015, 7:48 AM
gkaracha added a reviewer: bgamari.

Please don't use {-# OPTIONS_GHC -Wwarn #-}. Use -fno-warn-unused-binds if you have to.

Where are the tests? I saw them before on the wip/ branch.

In D1535#45148, @thomie wrote:

Where are the tests? I saw them before on the wip/ branch.

George has indicated via email that he is working on reincorporating them locally.

In D1535#45148, @thomie wrote:

Please don't use {-# OPTIONS_GHC -Wwarn #-}. Use -fno-warn-unused-binds if you have to.

Sorry about this, it is left there from the previous implementation. I am not sure if we need this, I will
make sure if we do/don't and fix it.

Where are the tests? I saw them before on the wip/ branch.

I haven't committed them yet, after fast forwarding branch I kept them only locally
because I am adding some more. I will have them up in a few hours.

George

gkaracha updated this revision to Diff 5420.Dec 2 2015, 2:32 PM
gkaracha retitled this revision from Pattern Match Checking (Fixes #595) to Major Overhaul of Pattern Match Checking (Fixes #595).
gkaracha removed rGHC Glasgow Haskell Compiler as the repository for this revision.

Compared to the previous diff I:

  • Addressed all comments by thomie, bgamari and hlint
  • Added several comments on obscure parts of the check
  • Squashed all commits to one
  • Fast-forwarded the implementation (I think now it is merge-able)
gkaracha set the repository for this revision to rGHC Glasgow Haskell Compiler.Dec 2 2015, 2:33 PM
bgamari commandeered this revision.Dec 4 2015, 4:25 AM
bgamari edited reviewers, added: gkaracha; removed: bgamari.
bgamari abandoned this revision.Dec 4 2015, 4:25 AM