[GHC] #14759: ListSetOps WARNING causes tests to fail

[GHC]

#14759: ListSetOps WARNING causes tests to fail
-------------------------------------+-------------------------------------
        Reporter:  ezyang            |                Owner:  (none)
            Type:  bug               |               Status:  patch
        Priority:  normal            |            Milestone:
       Component:  Compiler          |              Version:  8.2.2
      Resolution:                    |             Keywords:
Operating System:  Unknown/Multiple  |         Architecture:
                                     |  Unknown/Multiple
 Type of failure:  None/Unknown      |            Test Case:
      Blocked By:                    |             Blocking:
 Related Tickets:                    |  Differential Rev(s):  Phab:D4628
       Wiki Page:                    |
-------------------------------------+-------------------------------------

Comment (by niteria):

 Replying to [comment:10 simonpj]:
 > 1.  Represent `imp_finsts` as a `ModuleSet`
 > There's a helpful `Note [Combine ImportAvails]` in `RnNames` which
 explains why a set is a bit awkward.

 I think a Set can work out if you go all the way, also changing
 `dep_finst`.

 > My instinct is to try a `Bag` (so that `plusAvails` can use `unionBags`
 which is always fast), and move the removing-duplicates work to the
 consumer.  Then we could simplify the code in `RnNames`.
 >
 > I think there are two consumers of `imp_finsts`:
 >
 > * `tcExtendLocalFamInstEnv` loads up dependent modules.  So we'd need
 eliminate duplicates, but determinacy is not an issue
 > * `DsUsage.mkDependencies` which already does a sort, so removing dups
 at the same time should surely not be hard.

 I think this can work, but we'd have to remove dups from `A`'s
 `imp_finsts` before computing `imp_finsts` for `B` if `B` imports `A`.
 Otherwise for bigger module hierarchies a lot of duplicates will
 accumulate.

 I believe the existing tests already measure this, so it should be
 possible to compare the approaches quantitatively.

 It's unfortunate that we must keep and store a set of transitive
 dependencies for every module. The size just grows with the size of the
 codebase. I think for `imp_finsts` and `dep_finsts` the only reason we do
 that is so that we can do type family instance checks and avoid redoing
 some checks. See `Note [Checking family instance optimization]`. There's
 an assumption somewhere in there that having the transitive set in hand
 lets us load less interfaces. I'm skeptical of how it works out in
 practice. Perhaps we could avoid explicitly storing the whole transitive
 closure if when checking type family consistency we did a traversal with
 pruning instead. I haven't thought it through fully, so take this with a
 grain of salt.

-- 
Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/14759#comment:12>
GHC <http://www.haskell.org/ghc/>
The Glasgow Haskell Compiler