Skip to content

OccEnv should be deterministic

While debugging a subtle unique mismatch error in !11470 that became visible in test bkp15, I became aware that lookupGRE returns a lot of non-deterministic lookup results without proper justification. E.g.,

-- | Lookup an element in an 'OccEnv', ignoring 'NameSpace's entirely.
lookupOccEnv_AllNameSpaces :: OccEnv a -> OccName -> [a]
lookupOccEnv_AllNameSpaces (MkOccEnv as) (OccName _ s)
  = case lookupFsEnv as s of
      Nothing -> []
      Just r  -> nonDetEltsUFM r

-- | Look up all the record fields that match with the given 'FastString'
-- in an 'OccEnv'.
lookupFieldsOccEnv :: OccEnv a -> FastString -> [a]
lookupFieldsOccEnv (MkOccEnv as) fld =
  case lookupFsEnv as fld of
    Nothing   -> []
    Just flds -> nonDetEltsUFM $ filter_flds flds
  -- NB: non-determinism is OK: in practice we will either end up resolving
  -- to a single field or throwing an error.
  where
    filter_flds = filterUFM_Directly (\ uniq _ -> isFldNSUnique uniq)

The comment in lookupFieldsOccEnv is not quite the full story, even: Looking at its only use site in GHC.Tc.Gen.Export.check_occs, I see that the order becomes relevant in the generated error message. Now, codegen isn't really influenced by this non-determinism, but error messages will be affected, I think.

lookupOccEnv_AllNamespaces on the other hand doesn't even come with a warning in its haddock! It has transitive uses in

  • lookupImpOccEnv
  • lookupGRE

Non of which announce the non-determinism.

Proposal

-newtype OccEnv a = MkOccEnv (FastStringEnv (UniqFM NameSpace a))
+newtype OccEnv a = MkOccEnv (FastStringEnv (UniqDFM NameSpace a))
To upload designs, you'll need to enable LFS and have an admin enable hashed storage. More information