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

Fix Either/FilterValue compiler error #1

Merged
merged 5 commits into from
Dec 11, 2019
Merged

Fix Either/FilterValue compiler error #1

merged 5 commits into from
Dec 11, 2019

Conversation

MaxGabriel
Copy link
Collaborator

It looks like in old versions of persistent (e.g. https://hackage.haskell.org/package/persistent-2.9.2/docs/Database-Persist-Types.html), Filter had these constructors:

data Filter record = forall typ. PersistField typ => Filter
    { filterField  :: EntityField record typ
    , filterValue  :: Either typ [typ] -- FIXME
    , filterFilter :: PersistFilter -- FIXME
    }
    | FilterAnd [Filter record] -- ^ convenient for internal use, not needed for the API
    | FilterOr  [Filter record]
    | BackendFilter
          (BackendSpecificFilter (PersistEntityBackend record) record)

but in newer versions, it changed (https://hackage.haskell.org/package/persistent-2.10.4/docs/Database-Persist-Types.html#t:Filter):

data Filter record = forall typ. PersistField typ => Filter
    { filterField  :: EntityField record typ
    , filterValue  :: FilterValue typ
    , filterFilter :: PersistFilter -- FIXME
    }
    | FilterAnd [Filter record] -- ^ convenient for internal use, not needed for the API
    | FilterOr  [Filter record]
    | BackendFilter
          (BackendSpecificFilter (PersistEntityBackend record) record)

resulting in this compiler error:

/Users/maximiliantagher/Documents/Clones/Haskell/persistent-iproute/Database/Persist/IP.hs:42:71: error:
    • Couldn't match expected type ‘FilterValue IP’
                  with actual type ‘Either IP b0’
    • In the second argument of ‘Filter’, namely ‘(Left ip)’
      In the expression:
        Filter
          (unsafeCoerce field :: EntityField record IP)
          (Left ip)
          (BackendSpecificFilter ">>")
      In an equation for ‘>.>.’:
          field >.>. ip
            = Filter
                (unsafeCoerce field :: EntityField record IP)
                (Left ip)
                (BackendSpecificFilter ">>")
   |
42 | field >.>. ip = Filter (unsafeCoerce field :: EntityField record IP) (Left ip) (BackendSpecificFilter ">>")
   |                                                                       ^^^^^^^

FilterValue is defined like this:

data FilterValue typ where
  FilterValue  :: typ -> FilterValue typ
  FilterValues :: [typ] -> FilterValue typ
  UnsafeValue  :: forall a typ. PersistField a => a -> FilterValue typ

So, my reading of the situation is that it needed a third constructor, so what was previously Left/Right is now FilterValue/FilterValues, and just a switchout is necessary. I compiled with:

extra-deps: [persistent-2.10.4]

to confirm this fixes the compilation error.

I haven't tested this at runtime though.

It looks like in old versions of persistent (e.g. https://hackage.haskell.org/package/persistent-2.9.2/docs/Database-Persist-Types.html), Filter had these constructors:

```
data Filter record = forall typ. PersistField typ => Filter
    { filterField  :: EntityField record typ
    , filterValue  :: Either typ [typ] -- FIXME
    , filterFilter :: PersistFilter -- FIXME
    }
    | FilterAnd [Filter record] -- ^ convenient for internal use, not needed for the API
    | FilterOr  [Filter record]
    | BackendFilter
          (BackendSpecificFilter (PersistEntityBackend record) record)
```

but in newer versions, it changed (https://hackage.haskell.org/package/persistent-2.10.4/docs/Database-Persist-Types.html#t:Filter):

```
data Filter record = forall typ. PersistField typ => Filter
    { filterField  :: EntityField record typ
    , filterValue  :: FilterValue typ
    , filterFilter :: PersistFilter -- FIXME
    }
    | FilterAnd [Filter record] -- ^ convenient for internal use, not needed for the API
    | FilterOr  [Filter record]
    | BackendFilter
          (BackendSpecificFilter (PersistEntityBackend record) record)
```

resulting in this compiler error:

```
/Users/maximiliantagher/Documents/Clones/Haskell/persistent-iproute/Database/Persist/IP.hs:42:71: error:
    • Couldn't match expected type ‘FilterValue IP’
                  with actual type ‘Either IP b0’
    • In the second argument of ‘Filter’, namely ‘(Left ip)’
      In the expression:
        Filter
          (unsafeCoerce field :: EntityField record IP)
          (Left ip)
          (BackendSpecificFilter ">>")
      In an equation for ‘>.>.’:
          field >.>. ip
            = Filter
                (unsafeCoerce field :: EntityField record IP)
                (Left ip)
                (BackendSpecificFilter ">>")
   |
42 | field >.>. ip = Filter (unsafeCoerce field :: EntityField record IP) (Left ip) (BackendSpecificFilter ">>")
   |                                                                       ^^^^^^^
```

`FilterValue` is defined like this:

```
data FilterValue typ where
  FilterValue  :: typ -> FilterValue typ
  FilterValues :: [typ] -> FilterValue typ
  UnsafeValue  :: forall a typ. PersistField a => a -> FilterValue typ
```

So, my reading of the situation is that it needed a third constructor, so what was previously `Left`/`Right` is now `FilterValue`/`FilterValues`, and just a switchout is necessary. I compiled with:

```
extra-deps: [persistent-2.10.4]
```

to confirm this fixes the compilation error.

I haven't tested this at runtime though.
@MaxGabriel
Copy link
Collaborator Author

MaxGabriel commented Nov 29, 2019

Oh wait, I need to either use CPP or add a constraint on using a more recent persistent

Edit: went with CPP

@MaxGabriel
Copy link
Collaborator Author

Separately, would you support editing package metadata to require < 2.10.0 for persistent? https://hackage.haskell.org/package/persistent-iproute-0.2.3/persistent-iproute.cabal/edit


# Here starts the actual work to be performed for the package under test; any command which exits with a non-zero exit code causes the build to fail.
script:
- if [ -f configure.ac ]; then autoreconf -i; fi
- cabal configure --enable-tests --enable-benchmarks -v2 --disable-shared # -v2 provides useful information for debugging
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

--enable-tests --enable-benchmarks was causing errors on HEAD. I think it's fine to just remove it—this package doesn't have tests or benchmarks anyway

@@ -5,8 +5,6 @@ sudo: false

matrix:
include:
- env: CABALVER=1.18 GHCVER=7.8.4
addons: {apt: {packages: [cabal-install-1.18,ghc-7.8.4], sources: [hvr-ghc]}}
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aeson-iproute wasn't compiling on old GHC. I think it's fine to drop this—GHC 7.8 dates back to 2014

@greydot
Copy link
Owner

greydot commented Dec 6, 2019

Separately, would you support editing package metadata to require < 2.10.0 for persistent? https://hackage.haskell.org/package/persistent-iproute-0.2.3/persistent-iproute.cabal/edit

Just did it.

Would you squash this PR into a single commit please?

@MaxGabriel
Copy link
Collaborator Author

Oh, you can squash right from the Github UI now:

image

@MaxGabriel MaxGabriel merged commit cddde41 into master Dec 11, 2019
@MaxGabriel
Copy link
Collaborator Author

Squashed and released!

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 this pull request may close these issues.

2 participants