SOLVED: Registering orphan instances and filling the `deps` field of ModIface [RE: Where (else) do I need to register instances from loaded modules?]

[Erdi, Gergo]

PUBLIC

So these orphans got less loving attention on the mailing list than Oliver Twist, but I ended up solving this so just in case anyone else runs into this, here's what was going wrong.

It turns out the key to everything was this:

> 2. I also changed mkModIface [...] by copy-pasting more code from mkIfaceTc. I'm really starting to think that I'd be better off just changing mkIfaceTc to return a PartialModIface...

As you may recall from an earlier email from me (attached), the reason I couldn't use mkIfaceTc wholesale was because the fingerprint calculation code panicked with `lookupVers1`. Frustratingly, I never got any reply on that. Since I am feeding GHC with module sources instead of letting GHC itself find them on the file system, and I don't generate `.hi` files at all, I thought the fingerprinting simply isn't going to work in this setup. Consequently, I just worked around this by splitting `mkIfaceTc` into a `PartilaModIface`-creating part and never calling `addFingerprints`.

It turns out `addFingerprints` does more than just filling in fingerprints -- it also fills in a field that tells if the given module contains any orphan instances at all! So of course importing a module with orphan instances didn't work for me -- the "this module contains orphan instances" flag was never set... At this point I could have just copied some logic from `addFingerprints` to my code, but instead I decided to figure out why/how exactly `addFingerprints` panics.

It all comes down to this part of my code:

```
prepareModule unit modName = do
    -- ...

    (errs, mss) <- do
        env <- setHomeUnit (toUn
TheitId env) <$> getSession
        liftIO $ partitionEithers <$> downsweep env [] exclude False
    reportErrors errs

    let menv = mkModuleEnv [(ms_mod ms, ms) | ms <- mss]

    let Just ms = lookupModuleEnv menv $ mkModule unit mod
    return ms

prepareSource ms = do
    pmod <- parseModule ms
    tmod <- typecheckModule pmod

    -- ...
```

So I set the home unit to whatever unit we want to load the module into, before calling `downsweep`. This returns a `ModGraph` from which I retrieve the `ModSummary` I want. However, when I then pass that `ModSummary` to `parseModule >=> typecheckModule` , this choice of home unit is discarded. I expected it to be stored in the `ModSummary` by `downsweep`; isn't this exactly the kind of situation that `ms_hspp_opts` is for?! So anyway, because of this, during typechecking, GHC got confused about where to look for the interface of an already loaded module (since it was compiled in home unit = mainUnit, but then loaded into myUnit) and that is what lead to the panic during fingerprinting.

IN SUMMARY (or, "tl;dr"), the solution for my problem was to fix the handling of the home unit by setting it in the global session before calling `preareSource`, and then using full `mkIfaceTc` which correctly fills in the "this module has orphans" flag. This has been a very awkward experience because:
* `addFingerprints` doesn't just do what it says on the tin.
* "does this module has orphans" is not, prima facie, something that couldn't be in `PartialModIface`.
* The `lookupVers1` panic message is gibberish (even now I don't know what the "vers" stands for!) and doesn't give any hints to what is actually going wrong.
* Asking about the specific GHC panic on this mailing list yielded absolutely zero replies.
* `ModSummary` contains its own `DynFlags` but that `DynFlags` isn't used for everything in `parseModule` &co., so it doesn't really bake in the choices made during the preparation of the source file.

-----Original Message-----
From: Erdi, Gergo 
Sent: Wednesday, September 8, 2021 5:55 PM
To: 'Matthew Pickering' <matthewtpickering at gmail.com>; 'GHC' <ghc-devs at haskell.org>
Cc: 'gergo at erdi.hu' <gergo at erdi.hu>; Montelatici, Raphael Laurent <Raphael.Montelatici at sc.com>
Subject: Re: Registering orphan instances and filling the `deps` field of ModIface [RE: Where (else) do I need to register instances from loaded modules?]

PUBLIC

Hi,

Any other ideas I could try?

Gergo

-----Original Message-----
From: Erdi, Gergo 
Sent: Friday, August 27, 2021 2:42 PM
To: 'Matthew Pickering' <matthewtpickering at gmail.com>
Cc: 'GHC' <ghc-devs at haskell.org>; 'gergo at erdi.hu' <gergo at erdi.hu>; Montelatici, Raphael Laurent <Raphael.Montelatici at sc.com>
Subject: Registering orphan instances and filling the `deps` field of ModIface [RE: Where (else) do I need to register instances from loaded modules?]

PUBLIC

Re: #1., I've simplified the code by removing all the Unit shuffling: everything is now in mainUnit. Unfortunately, this doesn't change anything other than cutting a cool 100 lines from the MWE code; I'm attaching the new smaller & simpler version. It is based on GHC 9.0.1 with an extra patch to export the first half of mkIfaceTc, just before calling mkFullIface. With this change, it really should be filling everything in ModIface the same way as the real GHC (so e.g. `deps` should be properly filled), modulo the fingerprints. 

Unfortunately, the `deps` field of the `ModIface`s created this way still doesn't contain any orphan instance modules: `Class.src`'s deps are empty (which is what I'd expect), `Instance.src` has a dependency in dep_mods (but not in dep_orphs) on `Class {-# SORUCE #-}` (I'm not sure why it's marked as a source import when `Class` module was already loaded into the module providers map before typechecking `Instance`...), and of course `Use.src` still fails so I can't get to its dependencies.

It is confusing to me that the problem should be because of the dep_orphs, because `Instance.src` does NOT use any orphan instances from `Class.src; merely, it provides a new orphan instance for other modules to depend on. Can someone explain this part to me?

Thanks,
	Gergo

-----Original Message-----
From: Erdi, Gergo 
Sent: Wednesday, August 25, 2021 10:37 AM
To: Matthew Pickering <matthewtpickering at gmail.com>
Cc: GHC <ghc-devs at haskell.org>; gergo at erdi.hu; Montelatici, Raphael Laurent <Raphael.Montelatici at sc.com>
Subject: Re: Where (else) do I need to register instances from loaded modules?

Hi,

Thanks Matt! Unfortunately, I couldn't get it working even with these suggestions.

1. I changed registerModule so that it only changes the HPT when toUnitId (moduleUnit mod) == homeUnitId (hsc_dflags env). But I'm pretty sure this will be vacuously true, since the unit of the module comes from prepareModule setting the home unit before the call to `downsweep`.

TBH I don't understand the whole idea behind the home package, and maybe all my pain comes from misusing packages in the first place. As you can see in the original code I've sent, I'm loading modules from different units; in my real use case, I'd even like to do things like load module1 from unit1, then module2 from unit2, then module3 from unit1 again -- basically just using the units as a cheap way to classify modules so that later downstream, I can make decisions based on the unit of the module of the source of a given definition. Should I give up on this idea, is this going to lead to problems later on?

2. I also changed mkModIface to fill in the `deps` and while I'm at it, the `usages` field, by copy-pasting more code from mkIfaceTc. I'm really starting to think that I'd be better off just changing mkIfaceTc to return a PartialModIface... Anyway, the new definitions of `deps` and `usages` are:

    let pluginModules = map lpModule (cachedPlugins (hsc_dflags hsc_env))
        unit_id = homeUnitId (hsc_dflags hsc_env)
    deps <- mkDependencies
        unit_id (map mi_module pluginModules) tcg

    dep_files <- readIORef dependent_files
    let used_names = mkUsedNames tcg
    usages <- mkUsageInfo hsc_env this_mod (imp_mods imports) used_names
              dep_files merged pluginModules

With these changes applied, I am still seeing the same error. Printing shows that `dep_orphs deps` is empty for Instance.src; further tracing, in turn, shows that `imp_orphs . tcg_imports $ tcg` is also empty!

I have also found the wording in the documentation regarding dep_orphs confusing: it sounds like, in my case, this would mean that Use has a dependency on Instance, because it is using an orphan instance from there. But my problems are way before I am making a ModIface for Use, since it fails already during typechecking. But you saying the problem is the incomplete deps in ModIface must mean that dep_orphs needs to be filled in the ModIface for Instance (or, oh god I hope not, Class), right?

If I sound somewhat confused, rest assured that in reality I am even MORE confused!

Any further advice welcome,
	Gergo

-----Original Message-----
From: Matthew Pickering <matthewtpickering at gmail.com> 
Sent: Tuesday, August 24, 2021 5:16 PM
To: Erdi, Gergo <Gergo.Erdi at sc.com>
Cc: GHC <ghc-devs at haskell.org>; gergo at erdi.hu; Montelatici, Raphael Laurent <Raphael.Montelatici at sc.com>
Subject: [External] Re: Where (else) do I need to register instances from loaded modules?


Hi Gergo,

Some things I noticed:

* The HPT only contains modules from the current home unit. It seems that registerModule ignores this so you break this invariant.
* It seems that `Instance` contains an orphan, but you don't fill in the `deps` field when making a `ModIface`, which should contain all the orphan modules below the module you are compiling.

I think that if you fill in `deps` correctly then things may work out.
`Dependencies` eventually becomes `ImportAvail`, which gets consulted by `tcVisibleOrphanMods` which is called in `tcGetInstEnvs` which is called by `matchInstEnv`.

Matt

This email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please delete all copies and notify the sender immediately. You may wish to refer to the incorporation details of Standard Chartered PLC, Standard Chartered Bank and their subsidiaries at https: //www.sc.com/en/our-locations

Where you have a Financial Markets relationship with Standard Chartered PLC, Standard Chartered Bank and their subsidiaries (the "Group"), information on the regulatory standards we adhere to and how it may affect you can be found in our Regulatory Compliance Statement at https: //www.sc.com/rcs/ and Regulatory Compliance Disclosures at http: //www.sc.com/rcs/fm

Insofar as this communication is not sent by the Global Research team and contains any market commentary, the market commentary has been prepared by the sales and/or trading desk of Standard Chartered Bank or its affiliate. It is not and does not constitute research material, independent research, recommendation or financial advice. Any market commentary is for information purpose only and shall not be relied on for any other purpose and is subject to the relevant disclaimers available at https: //www.sc.com/en/regulatory-disclosures/#market-disclaimer.

Insofar as this communication is sent by the Global Research team and contains any research materials prepared by members of the team, the research material is for information purpose only and shall not be relied on for any other purpose, and is subject to the relevant disclaimers available at https: //research.sc.com/research/api/application/static/terms-and-conditions. 

Insofar as this e-mail contains the term sheet for a proposed transaction, by responding affirmatively to this e-mail, you agree that you have understood the terms and conditions in the attached term sheet and evaluated the merits and risks of the transaction. We may at times also request you to sign the term sheet to acknowledge the same.

Please visit https: //www.sc.com/en/regulatory-disclosures/dodd-frank/ for important information with respect to derivative products.
-------------- next part --------------
An embedded message was scrubbed...
From: "Erdi, Gergo" <Gergo.Erdi at sc.com>
Subject: mkIfaceTc panics with "lookupVers1"
Date: Tue, 13 Jul 2021 08:02:09 +0000
Size: 23597
URL: <http://mail.haskell.org/pipermail/ghc-devs/attachments/20210928/593e6044/attachment.mht>