th-desugar
th-desugar copied to clipboard
Implement universal support for desugaring view patterns
There is machinery added in #175 which is able to desugar view-patterns and as-patterns into (DPat, [(DPat, DExp)])
(the original pattern, and any subsequent patterns which much match their associated expressions). It would be nice to use these in all desugarings, except that the list of subsequent DPat
are potentially fallible, and there are several places which expect these to be infallible.
One potential solution to this is to pass down a failure
clause into wherever this list is eliminated, allowing these patterns to be desugar into two clauses which together match everything.
From #175:
The bug here is that now dsPatOverExp
return a less fallible pattern, having moved most of the matching on as-patterns
into the subsequent let expression. The consequence of this is apparent in the following:
do
let xs = [1,2]
x@1 <- xs
pure x
This used to desugar to the equivalent of:
do
let xs = [1,2]
1 <- xs
let x = 1
pure x
But now it desugars to the equivalent of:
do
let xs = [1,2]
x <- xs
let 1 = x
pure x
Apart from it being incorrect, I do prefer the second one as there isn't any pattern -> expression transmutation happening.
If I understand this issue correctly, the difference comes down to how as-patterns are desugared in the PatM
instances for PatWithExp
and PatWithCases
. In the instance for PatWithExp
, we have:
dsAsP :: Name -> DPat -> PatWithExp q DPat
dsAsP name pat = do
pat' <- lift $ removeWilds pat
tell [(name, dPatToDExp pat')]
return pat'
But in the instance for PatWithCases
, we have:
dsAsP :: Name -> DPat -> PatWithCases q DPat
dsAsP name pat = do
tell [(pat, DVarE name)]
return (DVarP name)
There's a bit of asymmetry here. If you have an as-pattern n@p
, then the PatWithExp
instance will desugar it to let n = p_exp
(where p_exp
is an expression derived from the pattern p
), whereas the PatWithCases
instance will desugar it to let p = n
. What would happen if you used the let n = p_exp
approach in the PatWithCases
instance? That is, if you had this code?
dsAsP :: Name -> DPat -> PatWithCases q DPat
dsAsP name pat = do
pat' <- lift $ removeWilds pat
tell [(DVarP name, dPatToDExp pat')]
return pat'
I think that would allow this particular fix to work, however it would still be incorrect in some cases https://github.com/goldfirere/th-desugar/issues/178
Well spotted. But perhaps the issue is that we are trying to desugar patterns in do
notation the same way that we are trying to desugar let
bindings. For let
bindings, I think we should desugar them using case
expressions, similar to what you propose in https://github.com/goldfirere/th-desugar/issues/178#issue-1548409629. For patterns in do
notation, we could instead use the approach that I propose in https://github.com/goldfirere/th-desugar/issues/177#issuecomment-1387104880. Would that work?