[GHC] #13114: UniqSet definition seems shady
#13114: UniqSet definition seems shady -------------------------------------+------------------------------------- Reporter: dfeuer | Owner: dfeuer Type: task | Status: new Priority: low | Milestone: 8.4.1 Component: Compiler | Version: 8.1 Keywords: | Operating System: Unknown/Multiple Architecture: | Type of failure: None/Unknown Unknown/Multiple | Test Case: | Blocked By: Blocking: | Related Tickets: Differential Rev(s): | Wiki Page: -------------------------------------+------------------------------------- Currently, {{{#!hs type UniqSet a = UniqFM a }}} The key invariant of `UniqSet` is expressed in the somewhat-poorly-named `Note [Unsound mapUniqSet]`, and not enforced by the types. It seems likely that the clean thing is {{{#!hs newtype UniqSet a = US (UniqFM a) }}} Unfortunately there's an awful lot of code using `UniqSet` and assuming it's the same as `UniqFM`. To make this work, we'd need to expand the `UniqSet` API somewhat and figure out what to do at use sites using it interchangeably with `UniqFM`. -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/13114> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler
#13114: UniqSet definition seems shady -------------------------------------+------------------------------------- Reporter: dfeuer | Owner: dfeuer Type: task | Status: patch Priority: low | Milestone: 8.4.1 Component: Compiler | Version: 8.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:D3146 Wiki Page: | -------------------------------------+------------------------------------- Changes (by dfeuer): * status: new => patch * differential: => Phab:D3146 -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/13114#comment:1> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler
#13114: UniqSet definition seems shady -------------------------------------+------------------------------------- Reporter: dfeuer | Owner: dfeuer Type: task | Status: closed Priority: low | Milestone: 8.2.1 Component: Compiler | Version: 8.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:D3146 Wiki Page: | -------------------------------------+------------------------------------- Changes (by dfeuer): * status: patch => closed * resolution: => fixed * milestone: 8.4.1 => 8.2.1 Comment: Fixed in cbe569a56e2a82bb93a008beb56869d9a6a1d047 {{{ Upgrade UniqSet to a newtype The fundamental problem with type UniqSet = UniqFM is that UniqSet has a key invariant UniqFM does not. For example, fmap over UniqSet will generally produce nonsense. Upgrade UniqSet from a type synonym to a newtype. Remove unused and shady extendVarSet_C and addOneToUniqSet_C. Use cached unique in tyConsOfType by replacing unitNameEnv (tyConName tc) tc with unitUniqSet tc. Reviewers: austin, hvr, goldfire, simonmar, niteria, bgamari Reviewed By: niteria Subscribers: thomie Differential Revision: https://phabricator.haskell.org/D3146 }}} -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/13114#comment:2> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler
#13114: UniqSet definition seems shady -------------------------------------+------------------------------------- Reporter: dfeuer | Owner: dfeuer Type: task | Status: closed Priority: low | Milestone: 8.2.1 Component: Compiler | Version: 8.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:D3146 Wiki Page: | -------------------------------------+------------------------------------- Comment (by Sebastian Graf <sebastian.graf@…>): In [changeset:"5b98a38a32f2bc8491dc897631be8892919e2143/ghc" 5b98a38a/ghc]: {{{ #!CommitTicketReference repository="ghc" revision="5b98a38a32f2bc8491dc897631be8892919e2143" Make `UniqDSet` a newtype Summary: This brings the situation of `UniqDSet` in line with `UniqSet`. @dfeuer said in D3146#92820 that he would do this, but probably never got around to it. Validated locally. Reviewers: AndreasK, mpickering, bgamari, dfeuer, simonpj Reviewed By: simonpj Subscribers: simonpj, rwbarton, carter, dfeuer GHC Trac Issues: #15879, #13114 Differential Revision: https://phabricator.haskell.org/D5313 }}} -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/13114#comment:3> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler
participants (1)
-
GHC