haskell / primitive

This package provides various primitive memory-related operations.
Other
114 stars 58 forks source link

Use Proxy# in sizeOf# and alignment# #344

Closed oberblastmeister closed 1 year ago

oberblastmeister commented 2 years ago

Wouldn't using Proxy# be better then just ignoring the argument? It is annoying to have to pass undefined to these functions

chessai commented 2 years ago

Ideally we would use TypeApplications/AllowAmbiguousTypes. If not that, Proxy# would be better.

oberblastmeister commented 2 years ago

Yeah, AllowAmbiguousTypes might be better. Unfortunately the types are unlifted

treeowl commented 2 years ago

That shouldn't matter if they're class methods; the class dictionary is argument enough.

andrewthad commented 2 years ago

I agree that either of these (Proxy# or nothing at all) would be a better design. However, changing this would break the API, and I do not think that the aesthetic gain is enough to justify the breakage.

I'm sympathetic to companion functions defined like this:

{-# language AllowAmbiguousTypes #-}
sizeOf## :: forall a. Prim a => Int
sizeOf## = sizeOf# (undefined :: a)

I'd take a PR adding these.

konsumlamm commented 2 years ago

I noticed that using sizeOf# (undefined :: a) generates a new CallStack for every use (because undefined has a HasCallStack constraint), which unnecessarily clutters the core (although it should be optimized away for specialized uses). using TypeApplications + AllowAmbiguousTypes seems like the best solution currently, although in the future (when it is supported), forall a -> might be better? Another question is if it should return Int or Int#, I doubt this makes a difference though, it should be unpacked anyway.

sergv commented 2 years ago

One possible scenario to have sizeOf that works with Proxy# is to introduce new functions sizeProxy# :: Proxy# a -> Int# and alignProxy# :: Proxy# a -> Int# with default implementations (sizeOf# in terms of sizeProxy# and vice versa, ditto for alignment) so that old clients of the primitive package don't need to do anything but new clients will consciously prefer proxied versions. After some time it might even be possible to deprecate old versions requiring use of undefined (potentially even remove after much longer period when most of the ecosystem updates).

konsumlamm commented 1 year ago

I'd love to make a PR for this, but I'm wondering if it would be better to add it as a standalone function or to the Prim class (with a default implementation, so this would be backwards compatible). In the latter case, we could choose to deprecate sizeOf# in the future. Is that a realistic plan?

andrewthad commented 1 year ago

Adding to the Prim class, with a default implementation, seems like a fine plan. It would be a long time before we could deprecate (and then later remove) sizeOf, but at least there's a path forward to get there.