[GHC] #15247: Use empty types for TTG extension constructors
#15247: Use empty types for TTG extension constructors -------------------------------------+------------------------------------- Reporter: adamgundry | Owner: (none) Type: task | Status: new Priority: normal | Milestone: 8.6.1 Component: Compiler | Version: 8.5 Keywords: TTG | Operating System: Unknown/Multiple Architecture: | Type of failure: None/Unknown Unknown/Multiple | Test Case: | Blocked By: Blocking: | Related Tickets: Differential Rev(s): | Wiki Page: -------------------------------------+------------------------------------- I happened to be looking at `AmbiguousFieldOcc` and noticed that it has gained an `XAmbiguousFieldOcc` extension constructor for Trees That Grow, but lots of the destructor functions in `HsTypes` panic if they see this constructor (which is understandable, because they aren't expecting extensions). In general, perhaps it should be the case that for concrete phase descriptors (e.g. `GhcRn`) where extension constructors are not expected, the extension constructor argument type should be empty? It would then be clear that they should not be expected (barring abuse of laziness) and pattern matching on them could eliminate the empty type. That is, instead of the existing {{{#!hs data NoExt = NoExt type instance XXAmbiguousFieldOcc (GhcPass _) = NoExt }}} we would define {{{#!hs data NoExtConstructor -- empty data type noExtConstructor :: NoExtConstructor -> a noExtConstructor x = case x of {} type instance XXAmbiguousFieldOcc (GhcPass _) = NoExtConstructor }}} and similarly for other `XX...` type families. Alan, Shayan, is there any reason this couldn't work? -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/15247 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#15247: Use empty types for TTG extension constructors -------------------------------------+------------------------------------- Reporter: adamgundry | Owner: (none) Type: task | Status: new Priority: normal | Milestone: 8.6.1 Component: Compiler | Version: 8.5 Resolution: | Keywords: TTG 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 simonpj): Whatever is decided, the answers should go in [wiki:ImplementingTreesThatGrow/TreesThatGrowGuidance] -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/15247#comment:1 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#15247: Use empty types for TTG extension constructors -------------------------------------+------------------------------------- Reporter: adamgundry | Owner: (none) Type: task | Status: new Priority: normal | Milestone: 8.6.1 Component: Compiler | Version: 8.5 Resolution: | Keywords: TTG 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 alanz): I just tried this, and it seems the additional match is still required. I end up with {{{#!hs compiler/hsSyn/HsTypes.hs:1286:1: error: [-Wincomplete-patterns, -Werror =incomplete-patterns] Pattern match(es) are non-exhaustive In an equation for ‘unambiguousFieldOcc’: Patterns not matched: (XAmbiguousFieldOcc _) | 1286 | unambiguousFieldOcc (Unambiguous rdr sel) = FieldOcc rdr sel }}} having removed the last line of {{{#!hs unambiguousFieldOcc :: AmbiguousFieldOcc GhcTc -> FieldOcc GhcTc unambiguousFieldOcc (Unambiguous rdr sel) = FieldOcc rdr sel unambiguousFieldOcc (Ambiguous rdr sel) = FieldOcc rdr sel unambiguousFieldOcc (XAmbiguousFieldOcc _) = panic "unambiguousFieldOcc" }}} Perhaps I misunderstand how this is intended to work? -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/15247#comment:2 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#15247: Use empty types for TTG extension constructors -------------------------------------+------------------------------------- Reporter: adamgundry | Owner: (none) Type: task | Status: new Priority: normal | Milestone: 8.6.1 Component: Compiler | Version: 8.5 Resolution: | Keywords: TTG 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 Shayan-Najd): Hi Adam, Take a look at [wiki:ImplementingTreesThatGrow/TreesThatGrowGuidance], there we use the "standard" empty type `Void` and its eliminator `absurd` from `Data.Void`. Currently, TTG does not fully follow the guidelines as the extension constructors are barely used. I will soon have a pass over the current TTG implementation and fix it to match the guidelines. As we used `NoExt` instead of `()`, we can possibly use a GHC-Specific empty datatype instead of `Void`. Is there anything else that I may be missing? -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/15247#comment:3 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#15247: Use empty types for TTG extension constructors -------------------------------------+------------------------------------- Reporter: adamgundry | Owner: (none) Type: task | Status: new Priority: normal | Milestone: 8.6.1 Component: Compiler | Version: 8.5 Resolution: | Keywords: TTG 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 adamgundry): Replying to [comment:2 alanz]:
I just tried this, and it seems the additional match is still required. I end up with
{{{#!hs compiler/hsSyn/HsTypes.hs:1286:1: error: [-Wincomplete-patterns, -Werror =incomplete-patterns] Pattern match(es) are non-exhaustive In an equation for ‘unambiguousFieldOcc’: Patterns not matched: (XAmbiguousFieldOcc _) | 1286 | unambiguousFieldOcc (Unambiguous rdr sel) = FieldOcc rdr sel }}}
having removed the last line of
{{{#!hs unambiguousFieldOcc :: AmbiguousFieldOcc GhcTc -> FieldOcc GhcTc unambiguousFieldOcc (Unambiguous rdr sel) = FieldOcc rdr sel unambiguousFieldOcc (Ambiguous rdr sel) = FieldOcc rdr sel unambiguousFieldOcc (XAmbiguousFieldOcc _) = panic "unambiguousFieldOcc" }}}
Perhaps I misunderstand how this is intended to work?
You can't remove the last line entirely because it can still potentially match (if called with `XAmbiguousFieldOcc undefined`). But you can eliminate the empty data type with an empty case analysis (or calling my `noExtConstructor`). Replying to [comment:3 Shayan-Najd]:
Take a look at ImplementingTreesThatGrow/TreesThatGrowGuidance, there we use the "standard" empty type Void and its eliminator absurd from Data.Void.
Currently, TTG does not fully follow the guidelines as the extension constructors are barely used. I will soon have a pass over the current TTG implementation and fix it to match the guidelines.
Okay, it sounds like we are in agreement. That page could be a bit more explicit about the use of the empty type (the only mention I can see is one line of an example using `Void`). Moreover, it doesn't seem to mention `NoExt`.
As we used NoExt instead of (), we can possibly use a GHC-Specific empty datatype instead of Void.
Yes, I think this would be a good idea. It's clearer for documentation purposes to use a custom empty datatype, and given that the only duplication consists of precisely one type constructor and one function, reusing `Data.Void` doesn't buy much. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/15247#comment:4 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
participants (1)
-
GHC