Skip to content

Add DCoVarSet to PluginProv

Mikolaj Konarski requested to merge wip/T23923-mikolaj-take-2 into master

This implements what @simonpj proposed in #23923 as summarised in #23923 (comment 543934):

The plan, as far as I understand what Simon suggests, would be to rip the relevant bits out of !3792 [...], including some documentation notes, maybe also a smart constructor for backward compatibility and/or a test. Then we'd need to think about the porting guide and perhaps some patches on head.hackage and/or contacting the affected plugin authors (whose plugins must be already faulty, but they would additionally become not-compiling after this change). Anything else?

with the changes summarised in !11977 (comment 546463) (adding to PluginProv does simplify the code, the copy-pasting from the old MR and it breaks the plugins contained in the GHC tests less and in a more useful way):

I will open a new MR with the variables in PluginProv.

Simon recommends to keep the sets deterministic for now. In the future we could store non-deterministic sets and turn them into deterministic when serializing, e.g., using the order in which variables appear in the source code (IIRC). Some tests show noticeable slowdown right now, so maybe this would help (but then, the vars in PluginProv are likely not to produce such an overhead). The code could get simpler, too.

Simon confirms we do need to serialize these variables and so keep them when converting from UnivCoProv to IfaceUnivCoProv just as the old MR does. Also, in the interface constructor, we need to store them in two sets, one bound variables, the other free variables, as in the old MR. We'd also keep the assertion that the free variables set is empty when serializing (because we serialize the whole module).

So far, I've added all the code I'm reasonably sure should be there, but not some extensive additions and refactorings from !3792 that perhaps can be avoided. I've inserted a few !!! markers into my changes, which signify I don't know what I'm doing in that particular spot. I'm blocked on those.

Edited by Mikolaj Konarski

Merge request reports