Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Build failure with hashable-1.4.0.0 #126

Closed
RyanGlScott opened this issue Nov 10, 2021 · 4 comments · Fixed by #127
Closed

Build failure with hashable-1.4.0.0 #126

RyanGlScott opened this issue Nov 10, 2021 · 4 comments · Fixed by #127

Comments

@RyanGlScott
Copy link
Contributor

RyanGlScott commented Nov 10, 2021

Trying to build parameterized-utils with hashable-1.4.0.0 fails thusly:

$ cabal build --allow-newer=hashable
Build profile: -w ghc-8.10.7 -O1
In order, the following will be built (use -v for more details):
 - parameterized-utils-2.1.4.0 (lib) (first run)
Preprocessing library for parameterized-utils-2.1.4.0..
Building library for parameterized-utils-2.1.4.0..
[ 3 of 35] Compiling Data.Parameterized.Classes

src/Data/Parameterized/Classes.hs:354:10: error:
    • Could not deduce (TestEquality f)
        arising from the superclasses of an instance declaration
      from the context: HashableF f
        bound by the instance declaration
        at src/Data/Parameterized/Classes.hs:354:10-46
      Possible fix:
        add (TestEquality f) to the context of the instance declaration
    • In the instance declaration for ‘Hashable (TypeAp f tp)’
    |
354 | instance HashableF f => Hashable (TypeAp f tp) where
    |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

This is because hashable-1.4.0.0 makes Eq a superclass of Hashable. Because TypeAp's Eq instance has a TestEquality constraint, the Hashable instance for TypeAp now also requires TestEquality. This isn't a problem currently, as parameterized-utils constraints hashable < 1.4 in its .cabal file, but it will be an issue eventually.

Two possible fixes:

  1. Add TestEquality constraints to all instances that need them. Something like this:

    diff --git a/parameterized-utils.cabal b/parameterized-utils.cabal
    index 6df2300..aa86ded 100644
    --- a/parameterized-utils.cabal
    +++ b/parameterized-utils.cabal
    @@ -56,8 +56,8 @@ library
                    , containers
                    , deepseq
                    , ghc-prim
    -               , hashable       >=1.2  && <1.4
    -               , hashtables     ==1.2.*
    +               , hashable       >=1.2  && <1.5
    +               , hashtables     >=1.2  && <1.4
                    , indexed-traversable
                    , lens           >=4.16 && <5.1
                    , mtl
    @@ -142,4 +142,4 @@ test-suite parameterizedTests
     
       if impl(ghc >= 8.6)
         build-depends:
    -      hedgehog-classes
    \ No newline at end of file
    +      hedgehog-classes
    diff --git a/src/Data/Parameterized/Classes.hs b/src/Data/Parameterized/Classes.hs
    index b90754d..df1072a 100644
    --- a/src/Data/Parameterized/Classes.hs
    +++ b/src/Data/Parameterized/Classes.hs
    @@ -351,7 +351,7 @@ instance OrdF f => Ord (TypeAp f tp) where
     instance ShowF f => Show (TypeAp f tp) where
       showsPrec p (TypeAp x) = showsPrecF p x
     
    -instance HashableF f => Hashable (TypeAp f tp) where
    +instance (HashableF f, TestEquality f) => Hashable (TypeAp f tp) where
       hashWithSalt s (TypeAp x) = hashWithSaltF s x
     
     ------------------------------------------------------------------------
    diff --git a/src/Data/Parameterized/Context/Safe.hs b/src/Data/Parameterized/Context/Safe.hs
    index 96da3c0..5bdd196 100644
    --- a/src/Data/Parameterized/Context/Safe.hs
    +++ b/src/Data/Parameterized/Context/Safe.hs
    @@ -595,10 +595,10 @@ instance Hashable (Index ctx tp) where
     instance HashableF (Index ctx) where
       hashWithSaltF s i = hashWithSalt s (indexVal i)
     
    -instance HashableF f => HashableF (Assignment f) where
    +instance (HashableF f, TestEquality f) => HashableF (Assignment f) where
       hashWithSaltF = hashWithSalt
     
    -instance HashableF f => Hashable (Assignment f ctx) where
    +instance (HashableF f, TestEquality f) => Hashable (Assignment f ctx) where
       hashWithSalt s AssignmentEmpty = s
       hashWithSalt s (AssignmentExtend asgn x) = (s `hashWithSalt` asgn) `hashWithSaltF` x
     
    diff --git a/src/Data/Parameterized/Context/Unsafe.hs b/src/Data/Parameterized/Context/Unsafe.hs
    index 43e9def..e3f5845 100644
    --- a/src/Data/Parameterized/Context/Unsafe.hs
    +++ b/src/Data/Parameterized/Context/Unsafe.hs
    @@ -850,10 +850,10 @@ instance HashableF (Index ctx) where
     instance Hashable (Index ctx tp) where
       hashWithSalt = hashWithSaltF
     
    -instance HashableF f => Hashable (Assignment f ctx) where
    +instance (HashableF f, TestEquality f) => Hashable (Assignment f ctx) where
       hashWithSalt s (Assignment a) = hashWithSaltF s a
     
    -instance HashableF f => HashableF (Assignment f) where
    +instance (HashableF f, TestEquality f) => HashableF (Assignment f) where
       hashWithSaltF = hashWithSalt
     
     instance ShowF f => Show (Assignment f ctx) where
    diff --git a/src/Data/Parameterized/Some.hs b/src/Data/Parameterized/Some.hs
    index 3df9359..75e9c55 100644
    --- a/src/Data/Parameterized/Some.hs
    +++ b/src/Data/Parameterized/Some.hs
    @@ -33,7 +33,7 @@ instance TestEquality f => Eq (Some f) where
     instance OrdF f => Ord (Some f) where
       compare (Some x) (Some y) = toOrdering (compareF x y)
     
    -instance HashableF f => Hashable (Some f) where
    +instance (HashableF f, TestEquality f) => Hashable (Some f) where
       hashWithSalt s (Some x) = hashWithSaltF s x
       hash (Some x) = hashF x
    
  2. In the spirit of hashable-1.4.0.0, we could make TestEquality a superclass of HashableF. This comes with its own set of challenges, however, as there are data types that have HashableF instances but not TestEquality instances. For instance, naïvely adding a TestEquality instance to HashableF will result in a different build error in parameterized-utils:

    [ 3 of 35] Compiling Data.Parameterized.Classes
    
    src/Data/Parameterized/Classes.hs:331:10: error:
        • Could not deduce (TestEquality (Const a))
            arising from the superclasses of an instance declaration
          from the context: Hashable a
            bound by the instance declaration
            at src/Data/Parameterized/Classes.hs:331:10-42
        • In the instance declaration for ‘HashableF (Const a)’
        |
    331 | instance Hashable a => HashableF (Const a) where
        |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    

    It's not entirely clear to me how you'd even define TestEquality for Const.

@RyanGlScott
Copy link
Contributor Author

This issue is blocking support for GHC 9.0.2, as only hashable-1.4.0.2 has version bounds that are compatible with 9.0.2.

@RyanGlScott
Copy link
Contributor Author

Option (2) in #126 (comment) is further complicated by the fact that it's unclear if TestEquality is even the right notion of equality to use—see haskell/core-libraries-committee#21. In light of this, I'm hesitant to commit to adding a new superclass to HashableF that could change pending the results of haskell/core-libraries-committee#21.

In light of this, I think it would be best to go with option (1). A quick search reveals that the libraries that would be most impacted by this change are what and crucible. Luckily, these libraries can be patched before a new parameterized-utils release is made to make the migration easier.

RyanGlScott added a commit that referenced this issue Jan 5, 2022
Because `hashable-1.4.0.0` adds an `Eq` superclass to `Hashable`, several
`Hashable` instances in `parameterized-utils` now must add additional
constraints to satisfy the corresponding `Eq` instances. For instance,
several `Eq` instances have `TestEquality` constraints, so the `Hashable`
instances must have corresponding `TestEquality` constraints as well.

Fixes #126.
RyanGlScott added a commit to GaloisInc/what4 that referenced this issue Jan 5, 2022
This was prompted by GaloisInc/parameterized-utils#126, but the changes needed
on `what4`'s end can be applied independently.
RyanGlScott added a commit to GaloisInc/crucible that referenced this issue Jan 5, 2022
This was prompted by GaloisInc/parameterized-utils#126, but the changes needed
on `crucible`'s end can be applied independently.
@robdockins
Copy link
Contributor

I agree, let's not bake in TestEquality as a superclass given the uncertainty about its future.

RyanGlScott added a commit to GaloisInc/crucible that referenced this issue Jan 5, 2022
This was prompted by GaloisInc/parameterized-utils#126, but the changes needed
on `crucible`'s end can be applied independently.
RyanGlScott added a commit to GaloisInc/what4 that referenced this issue Jan 5, 2022
This was prompted by GaloisInc/parameterized-utils#126, but the changes needed
on `what4`'s end can be applied independently.
@RyanGlScott
Copy link
Contributor Author

hashable-1.3.5.0 has been revised to allow building with GHC 9.0.2, so this issue is no longer blocking 9.0.2 support. Still, it would be nice to land #127 in time for the next major release of parameterized-utils.

andreistefanescu pushed a commit to GaloisInc/crucible that referenced this issue Jan 7, 2022
This was prompted by GaloisInc/parameterized-utils#126, but the changes needed
on `crucible`'s end can be applied independently.
RyanGlScott added a commit that referenced this issue Mar 21, 2022
Because `hashable-1.4.0.0` adds an `Eq` superclass to `Hashable`, several
`Hashable` instances in `parameterized-utils` now must add additional
constraints to satisfy the corresponding `Eq` instances. For instance,
several `Eq` instances have `TestEquality` constraints, so the `Hashable`
instances must have corresponding `TestEquality` constraints as well.

Fixes #126.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants