#12532: Remove sum and tuple names from knownKeyNames -------------------------------------+------------------------------------- Reporter: bgamari | Owner: Type: task | Status: new Priority: normal | Milestone: Component: Compiler | Version: 8.0.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by bgamari): I have an initial attempt (Phab:D2469) at doing this. Here I remove all of the sum and tuples names from `knownKeyNames` implement the codepaths listed above for handling original name lookups and `Name`s from `Unique`s. However, I found that there is one wrinkle: there are actually two places where we need to lookup in the original name cache during interface file deserialization, 1. While deserializing the symbol table. This is the case that I had anticipated and indeed it poses no problem since we have a special encoding for known-key names, so indeed we should never need to do a name cache lookup in this case. 2. To lookup `IfaceTopBndr`s of, e.g., `IfaceDecl`s. This case I only realized after neck-deep in the refactoring and requires a bit more work to handle correctly. = My approach: Changing `IfaceTopBndr` = My approach to (2) is to essentially duplicate the special encoding of known key names that we use for the symbol table for `IfaceTopBndr`. This is done by changing it from a synonym for `OccName` to a sum of, {{{#!hs data IfaceTopBndr = IfaceTopOccName OccName | IfaceKnownKeyName Unique }}} the knock-on changes for this approach go through pretty easily. = An alternative: Using the symbol table = Simon PJ has suggested an alternative: rather than duplicating the known- key encoding, we should instead simply make `IfaceTopBndr` a synonym for `Name`; in principle this would mean that known-key names would "just work" since `IfaceTopBndr` would be encoding via the symbol table. Sadly, it seems that this is too good to be true. The trouble comes from the fingerprinting logic in `MkIface` (namely `addFingerprints`) which works roughly as follows, 1. we look at the `IfaceDecl`s being processed and compute strongly- connect groups of them. 2. we walk the list of groups in dependency order and compute a fingerprint for each. As we progress we maintain an `NameEnv Fingerprint` which maps each name we've looked at to its fingerprint. 3. The fingerprint for a group is determined by serializing it with `Binary` to a buffer and MD5ing the result. However, this isn't quite normal serialization: we override the `putName` operation to, rather than merely serializing the `Name`, lookup the name in the fingerprint environment and instead writing the fingerprint. To see the issue consider what happens when we try to fingerprint a single `IfaceId` with `IfaceTopBndr ~ Name`: the very first thing we do is `put` the `IfaceId`'s `ifName`; this will attempt to lookup the `Name` in the fingerprint environment but this will of course fail since we haven't yet fingerprinted it (which results in a panic exclaiming "urk! lookup local fingerprint"). "Well," you might say, "perhaps we can simply add a special case to the dummy `putName` logic not to attempt to lookup the `Name` of the `IfaceDecl` which we are currently fingerprinting. Sadly, however, this isn't enough since we also need the other names which the decl binds implicitly (e.g. the datacons of a `IfaceData`). It is indeed possible to compute these but all-in-all the situation ends up looking much messier than the relatively simple solution I proposed above. -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/12532#comment:1> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler

#12532: Remove sum and tuple names from knownKeyNames -------------------------------------+------------------------------------- Reporter: bgamari | Owner: Type: task | Status: new Priority: normal | Milestone: Component: Compiler | Version: 8.0.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by bgamari): Right, but the problem that I describe is actually quite orthogonal to the known-key encoding issue. The problem is that while generating a fingerprint for an `IfaceDecl` we override the implementation of `putName` in to resolve ensure that the fingerprints the things it refers to are included in its fingerprint. We do this by building up a `NameEnv Fingerprint` as we walk the `IfaceDecl`s in dependency order. Previously this was fine since none of the `Name`s we would serialize were binding occurrences. However, now that `IfaceTopBndr` is also a `Name` we have lost the ability to distinguish binding from non-binding occurrences. This breaks the fingerprinting implementation since we then end up attempting to lookup the `Name` (and implicit `Name`s) of the thing we are trying to fingerprint since its serialization contains a binding `Name`. In writing this the solution here became fairly obvious: simply tell `putName` whether it is serializing a binding or non-binding occurrence. -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/12532#comment:3> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler

#12532: Remove sum and tuple names from knownKeyNames -------------------------------------+------------------------------------- Reporter: bgamari | Owner: Type: task | Status: patch Priority: normal | Milestone: Component: Compiler | Version: 8.0.1 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:D2467 Wiki Page: | -------------------------------------+------------------------------------- Changes (by bgamari): * status: new => patch * differential: => Phab:D2467 -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/12532#comment:5> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler

#12532: Remove sum and tuple names from knownKeyNames -------------------------------------+------------------------------------- Reporter: bgamari | Owner: Type: task | Status: closed Priority: normal | Milestone: 8.2.1 Component: Compiler | Version: 8.0.1 Resolution: fixed | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D2467 Wiki Page: | -------------------------------------+------------------------------------- Changes (by bgamari): * status: patch => closed * resolution: => fixed * milestone: => 8.2.1 Comment: It has been done. -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/12532#comment:7> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler