"Move guards forward" can be more precise

[LeventErkok]

For this input:

foo = [() | Just x <- [Nothing], true]

Hlint says:

a.hs:1:7-38: Suggestion: Move guards forward
Found:
  [() | Just x <- [Nothing], true]
Perhaps:
  [() | true, Just x <- [Nothing]]

I see the intention of this suggestion, and in general I do like it. But the clause Just x <- [Nothing] is a guard itself, and in this case it'll actually better to have it first.

(Of course this is a made-up example, but I had this in a real program with the guard-match weeding out way more than what the actual test would.)

I wonder if the suggesting can be made precise: i.e., don't suggest it if what we're moving over has a pattern match which automatically weeds things out.

[ndmitchell]

What it's really hoping to do is suggest you replace an N tests with 1 test, which is almost always a good idea. In this particular case though N is 1. I wonder if just special casing guards with an explicit singleton list would be the right thing to do?

[LeventErkok]

Perhaps.. My actual example was more along the lines of a uniplate application. Something like:

[() | x <- universeBi e, Just o <- [getOp x], isSpecial (children x)]

This grabs all "subexpressions", finds ones that have a special kind of operator, and checks if the immediate children are "special," for some definition of special. If I adopt the suggestion, I get:

[() | x <- universeBi e, isSpecial (children x), Just o <- [getOp x]]

If isSpecial and children are expensive to compute as opposed to what getOp does, this would be less efficient.

Of course, I could write the above in a form that doesn't trigger the hlint warning. But maybe there's a pattern here where the hlint suggestion would always be an improvement. If you implement your new strategy, would it trigger for this example?

[ndmitchell]

If we consider things wrapped in singleton lists as being just guards, then your [getOp x] is considered a guard, and nothing would trigger. So it does seem like this is the right heuristic.

[LeventErkok]

Yes, N==1 seems to be the right heuristic indeed. Thanks!