#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