-
Notifications
You must be signed in to change notification settings - Fork 50
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
Simple, dependable recipe for client modules #34
Comments
So, I think I have a (terrible) workaround for this. Remove instance CoerceCat (->) a b where
coerceC = unsafeCoerce
|
@conal, do you have an opinion on this? I'm considering applying it to our fork, so if you see any problems with it, it'd be great to know before they bite us down the line. And I would happily push the change upstream if you think it's acceptable. The reason to apply it to this particular instance (although perhaps also document on the class that other instances should do it explicitly) is because often other instances are implemented in terms of the instances on newtype MyCat a b = MyCat (a -> b)
instance CoerceCat MyCat where
coerceC = MyCat coerceC` so getting it "right" for |
@sellout Thanks for the reminder about this issue. Offhand, what your suggesting sounds like a heuristic (i.e., undependable) and a brittle one at that. How about the following alternative? Import |
We started using a recipe like you propose above, but we were soon adding more re-exports to that module, like So, maybe we have some other issue going on, but re-exporting I agree that it's not an ideal solution, but part of our focus has been on how to make using the plugin as low-friction as possible, and AFAICT, the current newtype situation is very finicky. I am very hesitant to use |
Yipes—what a mess! Let’s find a better way. This whole issue,
seems dubious to me. I hope there is some way around this restriction or a way to change the compiling-to-categories plugin so as not to rely on |
Yeah, I do like the path you started down of using |
I preferred that direction as well. I remember getting exhausted with the effort rather than encountering a definitive blocker. We could try again, working through the puzzle together and asking for expert help if we get stuck. |
You are the expert help! So, let me try to think through this a bit ... first, we'll need to use abst/repr to convert between two types that have the same in-memory representation. That doesn't mean they have the same Given that those pieces align, a And finally, we need to map the Do you have any remnants of your earlier attempt? I think a lot of these Coercion cases are going to have me scratching my head. |
I guess I'm digging in the right place, because I just found this comment in GHC:
|
Hm. Given the large amount of code the CtoC plugin takes from libraries not written with CtoC in mind, I doubt that this assumption is a safe one.
By a “NOP”, if you mean
Not at run-time, since the generated code will hopefully eventually become coercions again.
IIRC, a tricky aspect of translating coercions is that a lot of simplification has already been done by GHC. The whole complex construction was spelled out at one point, and then GHC helpfully simplifies down to what the plugin gets to see. Reversing that simplification was harder than I expected.
The git repo contains my attempts. Searching my notes, the last mention of I wonder if it would be easier to get GHC tweaked so that our coercions worked without the explicit imports. The current policy was probably designed without considering core plugins. |
Absolutely, this was my first thought as well. That policy is one for Haskell source, and doesn’t apply to Core; you are writing a Core plugin. I think a patch to GHC that adds a |
For a concat client module to compile, GHC must be able to see some rewrite rules and some
newtype
constructors. I usually include the followingimport
lines:We need the
GHC.Generics
import purely due to how the concat plugin translates coercions (intocoerceC
from theCoerceCat
class), together with the following restriction:Without the import of
GHC.Generics
, the plugin will sometimes result in errors like the following:The failure here results from the
(:.:)
constructor (Comp1
) not being in scope. A solution is to add "import qualified GHC.Generics
".This obscure issue keeps biting users. Maybe we could have a single import line for all concat users, like "
import qualified ConCat
", in addition to (probably unqualified) imports for operations actually used in client code.I just added a
ConCat
module in plugin/:Tried in a sample concat program (replacing those three imports). It works but generates a warning:
If I add the "
()
", then the coercion hole error returns. Makes sense, though not what we want. Hm.A possible solution might be to change the
coerceC
method definition inCoerceCat (->)
and otherCoerceCat
instances.Currently,
When synthesizing a use of
coerceC
, the plugin is already looking at a Core coercion betweena
andb
, which it discards. It tries to synthesize a dictionary forCoerceCat k a b
(for target categoryk
), which sometimes leads to a coercion hole error. Could we instead use the coercion at hand? It's unclear to me how, since the generalcoerceC
method doesn't involveCoercible
. (Could it? Some of the instances are tricky due to coercion roles.)Better yet, I'd love to redesign the handling of casts/coercions, eliminating
CoerceCat
altogether. My original intent was to transform Corecast
expressions into combinations of existing categorical operations, usingRepCat
for thenewtype
axioms and(.)
for the coercion transitivity.The text was updated successfully, but these errors were encountered: