Opened 6 months ago

Closed 3 months ago

Last modified 3 months ago

#15138 closed bug (fixed)

Unable to instantiate data members of kind Nat in backpack signatures.

Reported by: ppk Owned by:
Priority: normal Milestone: 8.8.1
Component: Compiler Version: 8.4.1
Keywords: backpack Cc:
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: None/Unknown Test Case:
Blocked By: Blocking:
Related Tickets: Differential Rev(s): Phab:D4951
Wiki Page:

Description

This is in context with backpack signatures and their instantiation with concrete implementation. Consider the signature Abstract which contains a data NatType of kind Nat.

{- skipped relevant language extensions -}

signature Abstract where
     import GHC.TypeLits
     data NatType :: Nat  -- comment this out
     

Concrete implementations are unable to instantiate this abstract class during linking

   module Concrete where
     type NatType = 42

This is neither working with explicit .bkp files nor actual cabal package. I have isolated a minimum example into a repository a cabal packages as well as a single bkp file. The urls above also have the associated ghc log messages.

The version of ghc I have tested with is 8.4.1

Change History (10)

comment:1 Changed 5 months ago by bgamari

Milestone: 8.6.18.8.1

These won't be addressed for GHC 8.6.

comment:2 Changed 4 months ago by bgamari

Differential Rev(s): Phab:D4951
Milestone: 8.8.18.6.1
Status: newpatch

This looks simple enough that I think we can sneak it in to 8.6.

comment:3 Changed 4 months ago by ezyang

Keywords: backpack added

comment:4 Changed 4 months ago by bgamari

Milestone: 8.6.18.8.1

Unfortunately it doesn't look like this will quite make it. Sorry ppk!

comment:5 in reply to:  4 Changed 4 months ago by ppk

Replying to bgamari:

Unfortunately it doesn't look like this will quite make it. Sorry ppk!

That is okey. Thanks for all the effort you and the ghc team makes. But here are some of the things about the failing test cases. I am documenting it here so that it can be useful for diagnosing them if this patch has to eventually land.

  1. The failing tests are all in the backpack tests should_fail section namely bkpfail23,25,26,27 and 46
  1. Of these all except bkpfail46 is still giving compilation error which but the stderr are not matching. As a result the most interesting test is bkpfail46 as that is a case that was rejected before the patch which is getting accepted now. So we should really understand bkpfail46 to make sense whether it is

  1. Making backpack more general (a good thing). This just means that the

test case is no more relevant and could just be removed or ignored.

  1. Or this could be a break in typesafety and hence bad.

I do not think I understand the issues there fully to make a comment on it one way or the other.

  1. The next critical test is the bkpfail25 as it gives an error message of the form
    bkpfail25.bkp:7:18: error:                                                                                                                       
        _ The type synonym _T_ should have 1 argument, but has been given none                                                                       
        _ In the instance declaration for _Functor T_ 
    
    There is a comment bkpfail25.bkp that precisely says that such an error message is "too late". So this too looks like a test case that needs some analysis.
  1. The other tests that fails, I do not have much to say.

I wanted to say that I also have a dependent phab which builds on this to fix #15379 where I did document this. These tests were failing on my machine and I was thinking that it could be some change that I introduced in that patch which made the difference. Now it looks like this is a problem even here.

In summary, this looks like a test that requires some serious thought from someone who understands the backpack system much more than me.

comment:6 Changed 4 months ago by ppk

I think the issue was a wild card pattern match gone wrong. The sharp eyes of @bgamari spotted it. The changes have been pushed to Phab. I think this patch is ready to land

comment:7 Changed 4 months ago by ppk

@bgamari now that the patch is not failing the tests can you schedule this for ghc-8.6 instead of ghc-8.8. Please also consider #15379 if that is the case as without it, this patch alone will not be as useful. We had this discussion on IRC but I am putting it here for documentation. Thanks for spotting that bug in any case.

comment:8 Changed 4 months ago by bgamari

Milestone: 8.8.18.6.1

Absolutely. Thanks ppk!

comment:9 Changed 3 months ago by bgamari

Milestone: 8.6.18.8.1
Resolution: fixed
Status: patchclosed

I just looked at the patch for #15379 and I'm afraid it's a bit large for 8.6. Retargetting this for 8.6. Sorry ppk!

comment:10 Changed 3 months ago by Ben Gamari <ben@…>

In 7d771987/ghc:

Support typechecking of type literals in backpack

Backpack is unable to type check signatures that expect a data which
is a type level literal. This was reported in issue #15138. These
commits are a fix for this. It also includes a minimal test case that
was mentioned in the issue.

Reviewers: bgamari, ezyang, goldfire

Reviewed By: bgamari, ezyang

Subscribers: simonpj, ezyang, rwbarton, thomie, carter

GHC Trac Issues: #15138

Differential Revision: https://phabricator.haskell.org/D4951
Note: See TracTickets for help on using tickets.