New bindings leak C FFI types into the public API about direct-sqlite HOT 4 CLOSED

It'd be good to decide what to do about this before the next release, as this relates to the public API.

from direct-sqlite.

Hmm.. Now that I used it a bit more, it doesn't seem too bad, actually. Just need a few fromIntegrals here and there.

That being said, I feel like this is a bit leaky.

from direct-sqlite.

Yeah, I don't like it as CInt, but oh well, since I released before I saw this. I do think the CInt to Int change should happen and before next time,whenever that is.

Sent from my iPad

On Sep 28, 2012, at 11:38 AM, Janne Hellsten [email protected] wrote:

I wonder if this was intentional in @joeyadams new code:

newtype ParamIndex = ParamIndex CInt
deriving (Eq, Ord, Enum, Num, Real, Integral)
This specifies ParamIndex to be a CInt instead of just an Int. This has surprising consequences, like:

-- in sqlite-simple code that uses direct-sqlite
ParamIndex stmtParamCount <- bindParameterCount stmt
when (length qp /= stmtParamCount) (throwColumnMismatch qp stmtParamCount)
Leads to a type error:

Database/SQLite/Simple.hs:115:22:
Couldn't match expected type Int' with actual typeForeign.C.Types.CInt'
In the second argument of (/=)', namelystmtParamCount'
In the first argument of when', namely (length qp /= stmtParamCount)'
In a stmt of a 'do' block:
when
(length qp /= stmtParamCount)
(throwColumnMismatch qp stmtParamCount)
stmtParamCount here will be a CInt which at least to me was a surprise. I would expect counts like these to just use normal Ints, rather than types that are used for C bindings.

I think this will surprise users.

IMO the direct-sqlite public API should use just Ints, not CInts.

—
Reply to this email directly or view it on GitHub.

from direct-sqlite.

Hmm, I had forgotten about this old issue. I think I agree that Int would be better than CInt. Now we just need to remember to do it before 3.0 sometime. :)

from direct-sqlite.