Arrowcase1

[Simon Peyton-Jones]

Ross

The test arrows/should_compile/arrowcase1 is failing Lint. The bug is in the desugarer.

Here's a smaller test case:

h :: ArrowChoice a => Int -> a (Int,Int) Int
h x = proc (y,z) -> case compare x y of
GT -> returnA -< z+x

The type checker turns the case into

case compare x y of
GT { p77 = plusInt } -> returnA -< p77 z x

Here p77 is a local binding for the (+) operation. In general, patterns can contain bindings (used to discharge constraints that are bound by the pattern). In this case the binding isn't strictly necessary, but in general it is - consider existentials. It's equivalent to adding a 'let' around the RHS, but since the patters are perhaps nested, and one pattern might use a constraint that is bound by another, the pattern is the right place to attach the binding.

This has come up because GHC is binding things a little earlier than before, but an existential would have exposed it before.

The trouble is that the suspicious-looking replaceLeaves code in DsArrows (line 528 or so) doesn't know about these bindings.

I don't understand DsArrows at all. Indeed the whole Arrows code feels smelly to me. Maybe we tried to share too much code?

Anyway, could you look at it? Thanks. It's Trac #1080

Simon

[Ross Paterson]

Post by Simon Peyton-Jones
h :: ArrowChoice a => Int -> a (Int,Int) Int
h x = proc (y,z) -> case compare x y of
GT -> returnA -< z+x
The type checker turns the case into
case compare x y of
GT { p77 = plusInt } -> returnA -< p77 z x
Here p77 is a local binding for the (+) operation. In general, patterns
can contain bindings (used to discharge constraints that are bound by
the pattern). In this case the binding isn't strictly necessary, but
in general it is - consider existentials. It's equivalent to adding
a 'let' around the RHS, but since the patterns are perhaps nested,
and one pattern might use a constraint that is bound by another,
the pattern is the right place to attach the binding.

OK, so I'll need to put p77 in the pipe along with x and z. I think
my problem is I'm using collectPatsBinders to get the vars bound by
a pattern, but it deliberately doesn't include dictionary binders
from ConPatOut. Am I going to get the same issue everywhere there
are patterns?

Post by Simon Peyton-Jones
I don't understand DsArrows at all. Indeed the whole Arrows code
feels smelly to me.

It is a house of horrors, I'll admit, but it does a horrible job.

Post by Simon Peyton-Jones
Maybe we tried to share too much code?

Contrariwise, I think that using the same structures for commands that
correspond to expressions was the right thing to do. The complications
arise because the DsArrows stuff does its own computations of what's in
scope (so it knows exactly what to put in the pipe), doubtless duplicating
what's done elsewhere.

[Simon Peyton-Jones]

| OK, so I'll need to put p77 in the pipe along with x and z. I think
| my problem is I'm using collectPatsBinders to get the vars bound by
| a pattern, but it deliberately doesn't include dictionary binders
| from ConPatOut. Am I going to get the same issue everywhere there
| are patterns?

I guess so. It's very much as if instead of

Post by Simon Peyton-Jones
case compare x y of
GT { p77 = plusInt } -> returnA -< p77 z x

you had

case compare x y of
GT -> let p77 = plusInt in returnA -< p77 z x

Hmm. Would it be possible to desugar in the ordinary way first, and only *then* run over the result doing the proc-isng stuff? Then you would not have to deal with nested patterns etc.

Simon

[Ross Paterson]

Post by Simon Peyton-Jones
| OK, so I'll need to put p77 in the pipe along with x and z. I think
| my problem is I'm using collectPatsBinders to get the vars bound by
| a pattern, but it deliberately doesn't include dictionary binders
| from ConPatOut. Am I going to get the same issue everywhere there
| are patterns?
I guess so. It's very much as if instead of

Post by Simon Peyton-Jones
case compare x y of
GT { p77 = plusInt } -> returnA -< p77 z x

you had
case compare x y of
GT -> let p77 = plusInt in returnA -< p77 z x

In the comment under collectPatsBinders, ignoring dictionary binders in
ConPatOut is justified in terms of lazy patterns. Presumably there is
more to this story, because if such dictionaries are guaranteed to be
unused it would be safe for arrows too.

Post by Simon Peyton-Jones
Hmm. Would it be possible to desugar in the ordinary way first, and
only *then* run over the result doing the proc-isng stuff? Then you
would not have to deal with nested patterns etc.

Desugar Core? Hmm. Binding sites might be a bit easier to find,
but we'd also need to expand the Core language, not just with proc,
but with an arrow type system. That would make it hard to share the
desugaring too, I imagine.

[Simon Peyton-Jones]

| In the comment under collectPatsBinders, ignoring dictionary binders in
| ConPatOut is justified in terms of lazy patterns. Presumably there is
| more to this story, because if such dictionaries are guaranteed to be
| unused it would be safe for arrows too.

They are only unused in lazy patterns, but not for strict ones. Indeed, they should be *empty* for lazy patterns; so I guess it'd be safe to gather the dict bindings too, if that makes it easier for you.

| > Hmm. Would it be possible to desugar in the ordinary way first, and
| > only *then* run over the result doing the proc-isng stuff? Then you
| > would not have to deal with nested patterns etc.
|
| Desugar Core? Hmm. Binding sites might be a bit easier to find,
| but we'd also need to expand the Core language, not just with proc,
| but with an arrow type system. That would make it hard to share the
| desugaring too, I imagine.

I was wondering about an encoding of arrow stuff into Core using pseudo-function application of (-<). Probably doesn't work. Definitely don't want to extend Core!

S

[Ross Paterson]

Post by Simon Peyton-Jones
| In the comment under collectPatsBinders, ignoring dictionary binders in
| ConPatOut is justified in terms of lazy patterns. Presumably there is
| more to this story, because if such dictionaries are guaranteed to be
| unused it would be safe for arrows too.
They are only unused in lazy patterns, but not for strict ones. Indeed,
they should be *empty* for lazy patterns; so I guess it'd be safe to
gather the dict bindings too, if that makes it easier for you.

Changing the ConPatOut line to also collect the pat_binds fixes arrowcase1
(and presumably anywhere else I handle patterns), but I don't know if
it will break anything elsewhere.

[Simon Peyton-Jones]

Ross

I think the safest thing to do is to write a new pat-binder-collector.
Here's the problem. Consider

data T a where
C :: Num a => a -> Int -> T a

f ~(C (n+1) m) = (n,m)

Here, the pattern (C (n+1)) binds a hidden dictionary (d::Num a), and *also* uses that dictionary to match the (n+1) pattern. Yet, the variables bound by the lazy pattern are n,m, *not* the dictionary d. So in mkSelectorBinds in DsUtils, we want just m,n as the variables bound.


Perhaps when you do this, you could add this example, plus one about arrows, to explain why the two purposes need two different functions?

Thanks

Simon

| -----Original Message-----
| From: Ross Paterson [mailto:***@soi.city.ac.uk]
| Sent: 14 January 2007 01:06
| To: Simon Peyton-Jones
| Cc: cvs-***@haskell.org
| Subject: Re: Arrowcase1
|
| On Mon, Jan 08, 2007 at 12:01:42PM +0000, Simon Peyton-Jones wrote:
| > | In the comment under collectPatsBinders, ignoring dictionary binders in
| > | ConPatOut is justified in terms of lazy patterns. Presumably there is
| > | more to this story, because if such dictionaries are guaranteed to be
| > | unused it would be safe for arrows too.
| >
| > They are only unused in lazy patterns, but not for strict ones. Indeed,
| > they should be *empty* for lazy patterns; so I guess it'd be safe to
| > gather the dict bindings too, if that makes it easier for you.
|
| Changing the ConPatOut line to also collect the pat_binds fixes arrowcase1
| (and presumably anywhere else I handle patterns), but I don't know if
| it will break anything elsewhere.