Opened 4 months ago

Closed 4 months ago

#13350 closed bug (fixed)

COMPLETE sets aren't read from external packages

Reported by: RyanGlScott Owned by: RyanGlScott
Priority: highest Milestone: 8.2.1
Component: Compiler Version: 8.1
Keywords: PatternSynonyms Cc: mpickering, rwbarton
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: Incorrect error/warning at compile-time Test Case: testsuite/tests/patsyn/should_compile/T13350
Blocked By: Blocking:
Related Tickets: Differential Rev(s): Phab:D3257
Wiki Page:

Description

If you define these two modules:

{-# LANGUAGE PatternSynonyms #-}
module Foo where

data KindRep = KindRepTyConApp
             | KindRepVar
             | KindRepApp
             | KindRepFun
             | KindRepTYPE
             | KindRepTypeLitS
             | KindRepTypeLitD

pattern KindRepTypeLit :: KindRep
pattern KindRepTypeLit = KindRepTypeLitD

{-# COMPLETE KindRepTyConApp, KindRepVar, KindRepApp, KindRepFun,
             KindRepTYPE, KindRepTypeLit #-}
module Bar where

import Foo

krInt :: KindRep -> Int
krInt KindRepTyConApp{} = 0
krInt KindRepVar{}      = 1
krInt KindRepApp{}      = 2
krInt KindRepFun{}      = 3
krInt KindRepTYPE{}     = 4
krInt KindRepTypeLit{}  = 5

And you compile Bar.hs with -Wall on, it will not emit any pattern-match exhaustiveness warnings, as expected.

However, something different happens if you import all of these KindRep conlikes from Type.Reflection.Unsafe instead:

module Bar where

import Type.Reflection.Unsafe

krInt :: KindRep -> Int
krInt KindRepTyConApp{} = 0
krInt KindRepVar{}      = 1
krInt KindRepApp{}      = 2
krInt KindRepFun{}      = 3
krInt KindRepTYPE{}     = 4
krInt KindRepTypeLit{}  = 5
$ ~/Software/ghc2/inplace/bin/ghc-stage2 --interactive -Wall Bar.hs
GHCi, version 8.1.20170228: http://www.haskell.org/ghc/  :? for help
Loaded GHCi configuration from /home/rgscott/.ghci
[1 of 1] Compiling Bar              ( Bar.hs, interpreted )

Bar.hs:6:1: warning: [-Wincomplete-patterns]
    Pattern match(es) are non-exhaustive
    In an equation for ‘krInt’:
        Patterns not matched:
            (KindRepTypeLitS _ _)
            (KindRepTypeLitD _ _)
  |
6 | krInt KindRepTyConApp{} = 0
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^...

When the COMPLETE set is defined in a module in an external package (base:Type.Reflection.Unsafe, in this example), GHC doesn't properly take it into account when emitting pattern-match exhaustiveness warnings! This makes COMPLETE sets not terribly useful in practice at the moment.

(NB: Type.Reflection.Unsafe's definitions of KindRepTyConApp et al. aren't quite the same as what I defined above, but their exact definitions aren't important here, just that they have the same names and COMPLETE set. And this is the only COMPLETE set that I could defined in the boot libraries at the moment, making it convenient to use.)

Change History (7)

comment:1 Changed 4 months ago by rwbarton

Milestone: 8.2.1
Priority: normalhighest

comment:2 Changed 4 months ago by rwbarton

Owner: set to RyanGlScott

comment:3 Changed 4 months ago by simonpj

That does look bogus. Thanks for looking at it Ryan.

comment:4 Changed 4 months ago by RyanGlScott

Differential Rev(s): Phab:D3257
Status: newpatch

comment:5 Changed 4 months ago by Ben Gamari <ben@…>

In 0d2f7330/ghc:

Read COMPLETE sets from external packages

Currently, `COMPLETE` pragmas are not read from external packages at
all, which quite limits their usefulness. This extends
`ExternalPackageState` to include `COMPLETE` sets from other packages,
and plumbs around the appropriate values to make it work the way you'd
expect it to.

Fixes #13350.

Test Plan: make test TEST=T13350

Reviewers: rwbarton, mpickering, austin, simonpj, bgamari

Reviewed By: simonpj

Subscribers: simonpj, thomie

Differential Revision: https://phabricator.haskell.org/D3257

comment:6 Changed 4 months ago by Ben Gamari <ben@…>

In 8ca4bb1c/ghc:

Read COMPLETE sets from external packages

Currently, `COMPLETE` pragmas are not read from external packages at
all, which quite limits their usefulness. This extends
`ExternalPackageState` to include `COMPLETE` sets from other packages,
and plumbs around the appropriate values to make it work the way you'd
expect it to.

Requires a `binary` submodule update.

Fixes #13350.

Test Plan: make test TEST=T13350

Reviewers: rwbarton, mpickering, austin, simonpj, bgamari

Reviewed By: simonpj

Subscribers: simonpj, thomie

Differential Revision: https://phabricator.haskell.org/D3257

comment:7 Changed 4 months ago by RyanGlScott

Resolution: fixed
Status: patchclosed
Test Case: testsuite/tests/patsyn/should_compile/T13350
Note: See TracTickets for help on using tickets.