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

Missing Hashable instance for clash-lib #2453

Closed
kleinreact opened this issue Apr 19, 2023 · 16 comments · Fixed by #2460
Closed

Missing Hashable instance for clash-lib #2453

kleinreact opened this issue Apr 19, 2023 · 16 comments · Fixed by #2460
Labels

Comments

@kleinreact
Copy link
Contributor

I get the following error when I try to compile clash-lib with ghc-9.0.2:

src/Clash/Core/Literal.hs:68:40: error:
    • No instance for (Hashable ByteArray)
        arising from the 'deriving' clause of a data type declaration
      Possible fix:
        use a standalone 'deriving instance' declaration,
          so you can specify the instance context yourself
      There are instances for similar types:
        instance Hashable data-array-byte-0.1.0.1:Data.Array.Byte.ByteArray
          -- Defined in ‘hashable-1.4.2.0:Data.Hashable.Class’
    • When deriving the instance for (Hashable Literal)
   |
68 |   deriving (Eq,Ord,Show,Generic,NFData,Hashable,Binary)
   |                                        ^^^^^^^^
@martijnbastiaan
Copy link
Member

Are you using master? Otherwise we might need to backport:

@kleinreact
Copy link
Contributor Author

kleinreact commented Apr 19, 2023

Yes. On master (fresh clone).

@martijnbastiaan
Copy link
Member

Right. I think this might be an interaction between the newer versions of hashable and primitive:

Primitive:

When building with base-4.17 and newer, re-export the ByteArray and MutableByteArray types from base instead of defining them in this library. This does not change the user-facing interface of Data.Primitive.ByteArray.

Hashable:

Add Hashable ByteArray instance using data-array-byte compat package
Add instance for Data.Array.Byte.ByteArray.

Plus Cabal somehow picking a cached version of either package over a new one. As a workaround could you:

  • cabal update and try again?
  • ... remove your .cabal and try again? This is how we "fixed" it on CI.

@kleinreact
Copy link
Contributor Author

rm -Rf dist-newstyle
rm -Rf ~/.cabal
cabal update
cd clash-lib
cabal build

Same result

@DigitalBrains1
Copy link
Member

DigitalBrains1 commented Apr 19, 2023

That's odd. The cabal update fixed it for me on GHC 9.0.2 just this day... After cabal update, it picked up a new version of primitive.

Maybe we need to implement the full logic then, support all combinations of hashable and primitive. This is like a perfect storm of in themselves benign changes in our dependencies that together completely go to shit.

[edit]
I still don't really understand what a GHC environment file does, but I do notice you don't seem to have deleted it. Could that have somehow tickled Cabal to pick an old version of primitive again? I just did

git clean -dfx
[copy cabal.project.local]
cabal update
cabal build clash clashi clash-cores

to be exact, and that got rid of the error. Please note the "force" in git clean if you copy-paste this, it is not forgiving on your files.
[/edit]

@kleinreact
Copy link
Contributor Author

Running

rm -Rf ~/.cabal
git clone https://github.com/clash-lang/clash-compiler.git && cd clash-compiler
cabal update
cabal build clash-lib

still results in the same error, so the GHC environment file should not be the problem here.

I am using

ghc 9.0.2
cabal-install version 3.6.2.0 (compiled using version 3.6.3.0.1 of the Cabal library)

The index state picked by cabal update is 2023-04-19T21:29:05Z.

If go back to 117e2ba, then everything compiles, so it must be #2448/#2449 that introduce the issue.

@martijnbastiaan
Copy link
Member

I'd be really surprised if this works, but does it work if you remove your .ghc.environment* file from the repository root? I agree with @DigitalBrains1 that we should probably fix this for the full cross product of relevant primitive/hashable versions.

@kleinreact
Copy link
Contributor Author

I do a fresh clone, which ensures that there cannot be any old .ghc.environment* files.

@martijnbastiaan
Copy link
Member

Okay thanks. I'll investigate.

@martijnbastiaan
Copy link
Member

martijnbastiaan commented Apr 26, 2023

Test results:

FAIL: ghc-8.10.7, hashable-1.4.2.0, primitive-0.7.4.0 
FAIL: ghc-8.6.5,  hashable-1.4.2.0, primitive-0.7.4.0 
FAIL: ghc-8.8.4,  hashable-1.4.2.0, primitive-0.7.4.0
FAIL: ghc-9.0.2,  hashable-1.4.2.0, primitive-0.7.4.0
FAIL: ghc-9.2.7,  hashable-1.4.2.0, primitive-0.7.4.0

OK:   ghc-8.10.7, hashable-1.4.0.2, primitive-0.7.4.0
OK:   ghc-8.10.7, hashable-1.4.0.2, primitive-0.8.0.0
OK:   ghc-8.10.7, hashable-1.4.1.0, primitive-0.7.4.0
OK:   ghc-8.10.7, hashable-1.4.1.0, primitive-0.8.0.0
OK:   ghc-8.10.7, hashable-1.4.2.0, primitive-0.8.0.0
OK:   ghc-8.6.5,  hashable-1.4.0.2, primitive-0.7.4.0
OK:   ghc-8.6.5,  hashable-1.4.0.2, primitive-0.8.0.0
OK:   ghc-8.6.5,  hashable-1.4.1.0, primitive-0.7.4.0
OK:   ghc-8.6.5,  hashable-1.4.1.0, primitive-0.8.0.0
OK:   ghc-8.6.5,  hashable-1.4.2.0, primitive-0.8.0.0
OK:   ghc-8.8.4,  hashable-1.4.0.2, primitive-0.7.4.0
OK:   ghc-8.8.4,  hashable-1.4.0.2, primitive-0.8.0.0
OK:   ghc-8.8.4,  hashable-1.4.1.0, primitive-0.7.4.0
OK:   ghc-8.8.4,  hashable-1.4.1.0, primitive-0.8.0.0
OK:   ghc-8.8.4,  hashable-1.4.2.0, primitive-0.8.0.0
OK:   ghc-9.0.2,  hashable-1.4.0.2, primitive-0.7.4.0
OK:   ghc-9.0.2,  hashable-1.4.0.2, primitive-0.8.0.0
OK:   ghc-9.0.2,  hashable-1.4.1.0, primitive-0.7.4.0
OK:   ghc-9.0.2,  hashable-1.4.1.0, primitive-0.8.0.0
OK:   ghc-9.0.2,  hashable-1.4.2.0, primitive-0.8.0.0
OK:   ghc-9.2.7,  hashable-1.4.0.2, primitive-0.7.4.0
OK:   ghc-9.2.7,  hashable-1.4.0.2, primitive-0.8.0.0
OK:   ghc-9.2.7,  hashable-1.4.1.0, primitive-0.7.4.0
OK:   ghc-9.2.7,  hashable-1.4.1.0, primitive-0.8.0.0
OK:   ghc-9.2.7,  hashable-1.4.2.0, primitive-0.8.0.0

So it's really the combination of hashable-1.4.2.0 and primitive-0.7.4.0. As a work around, does constraining hashable/primitive work?

cabal build --constraint=hashable==1.4.2.0 --constraint=primitive==0.8.0.0

Edit: terrible script

@DigitalBrains1
Copy link
Member

DigitalBrains1 commented Apr 26, 2023

Can't you just add a Hashable instance for Data.Primitive.ByteArray when primitive < 0.8.0.0? I mean, I can't immediately come up with the proper combination of the ifdefs, but it seems pretty doable? Since the problem is that hashable == 1.4.2.0 defines a Hashable instance for Data.Array.Byte.ByteArray but not for Data.Primitive.ByteArray.

@kleinreact
Copy link
Contributor Author

The workaround works for me. Thanks.

@DigitalBrains1
Copy link
Member

DigitalBrains1 commented Apr 26, 2023

Isn't it just this?

--- a/clash-lib/src/Data/Primitive/ByteArray/Extra.hs
+++ b/clash-lib/src/Data/Primitive/ByteArray/Extra.hs
@@ -19,6 +19,11 @@ import Control.DeepSeq (NFData(..))
 #if !MIN_VERSION_base(4,17,0)
 #define DEFINE_HASHABLE_BYTEARRAY
 #endif
+#elif !MIN_VERSION_primitive(0,8,0)
+-- Hashable defines hashable for Data.Array.Byte.ByteArray, but before
+-- primitive 0.8.0, that's a distinct type from
+-- Data.Primitive.ByteArray.ByteArray.
+#define DEFINE_HASHABLE_BYTEARRAY So the code I'm proposing is still wrong in that respect: also in that case, we'll need the extra `hashable` instance 
 #endif
 
 #ifdef DEFINE_HASHABLE_BYTEARRAY

Since I'm still somewhat ill, my mind might produce weird code, so read well.

[edit]
Ah, right, we're not actually testing the code path where base >= 4.17.0, I was wondering how that could go well, but it probably will not. It will still fail for hashable == 1.4.1.0 and primitive == 0.7.4.0, it's just that we can't test it because we can't build Clash with base == 4.17 :-D
[/edit]

@DigitalBrains1
Copy link
Member

I think the primitive Changelog might be wrong. It seems that with base >= 4.17.0, they import it from base, and with earlier base they import it from data-array-byte. So the whole base logic can go; it just depends on the version of primitive whether it's a distinct type or not.

@DigitalBrains1
Copy link
Member

DigitalBrains1 commented Apr 27, 2023

I have created a little repository to do the testing with Martijns script outside clash-compiler. This means we can quickly test the solution, and another big plus: we can actually test this base >= 4.17 stuff. The results are:

FAIL: ghc-8.10.7, hashable-1.4.2.0, primitive-0.7.4.0
FAIL: ghc-8.6.5,  hashable-1.4.2.0, primitive-0.7.4.0
FAIL: ghc-8.8.4,  hashable-1.4.2.0, primitive-0.7.4.0
FAIL: ghc-9.0.2,  hashable-1.4.2.0, primitive-0.7.4.0
FAIL: ghc-9.2.7,  hashable-1.4.2.0, primitive-0.7.4.0
FAIL: ghc-9.4.5,  hashable-1.4.1.0, primitive-0.7.4.0
FAIL: ghc-9.4.5,  hashable-1.4.2.0, primitive-0.7.4.0

OK:   ghc-8.10.7, hashable-1.4.0.2, primitive-0.7.4.0
OK:   ghc-8.10.7, hashable-1.4.0.2, primitive-0.8.0.0
OK:   ghc-8.10.7, hashable-1.4.1.0, primitive-0.7.4.0
OK:   ghc-8.10.7, hashable-1.4.1.0, primitive-0.8.0.0
OK:   ghc-8.10.7, hashable-1.4.2.0, primitive-0.8.0.0
OK:   ghc-8.6.5,  hashable-1.4.0.2, primitive-0.7.4.0
OK:   ghc-8.6.5,  hashable-1.4.0.2, primitive-0.8.0.0
OK:   ghc-8.6.5,  hashable-1.4.1.0, primitive-0.7.4.0
OK:   ghc-8.6.5,  hashable-1.4.1.0, primitive-0.8.0.0
OK:   ghc-8.6.5,  hashable-1.4.2.0, primitive-0.8.0.0
OK:   ghc-8.8.4,  hashable-1.4.0.2, primitive-0.7.4.0
OK:   ghc-8.8.4,  hashable-1.4.0.2, primitive-0.8.0.0
OK:   ghc-8.8.4,  hashable-1.4.1.0, primitive-0.7.4.0
OK:   ghc-8.8.4,  hashable-1.4.1.0, primitive-0.8.0.0
OK:   ghc-8.8.4,  hashable-1.4.2.0, primitive-0.8.0.0
OK:   ghc-9.0.2,  hashable-1.4.0.2, primitive-0.7.4.0
OK:   ghc-9.0.2,  hashable-1.4.0.2, primitive-0.8.0.0
OK:   ghc-9.0.2,  hashable-1.4.1.0, primitive-0.7.4.0
OK:   ghc-9.0.2,  hashable-1.4.1.0, primitive-0.8.0.0
OK:   ghc-9.0.2,  hashable-1.4.2.0, primitive-0.8.0.0
OK:   ghc-9.2.7,  hashable-1.4.0.2, primitive-0.7.4.0
OK:   ghc-9.2.7,  hashable-1.4.0.2, primitive-0.8.0.0
OK:   ghc-9.2.7,  hashable-1.4.1.0, primitive-0.7.4.0
OK:   ghc-9.2.7,  hashable-1.4.1.0, primitive-0.8.0.0
OK:   ghc-9.2.7,  hashable-1.4.2.0, primitive-0.8.0.0
OK:   ghc-9.4.5,  hashable-1.4.1.0, primitive-0.8.0.0
OK:   ghc-9.4.5,  hashable-1.4.2.0, primitive-0.8.0.0

Note that hashable-1.4.0.2 does not support GHC 9.4, so no results for that.

The script is here. Its accompanying repo (which it will clone automatically) is here. Its current branch current-situation is with the code from clash-compiler.

[edit]
Let's make this explicit: of course the results are the same for versions already tested earlier, but now we also have results for GHC 9.4.
[/edit]

@DigitalBrains1
Copy link
Member

And the version on branch fixed works for the full test matrix. I'd like to suggest we use that and be done with it :-).

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

Successfully merging a pull request may close this issue.

3 participants