[GHC] #4342: Review containers changes

[Simon Peyton-Jones]

Narrowing cc list slightly to avoid span

Milan

|   Bang patterns are not in any standards, so this would rule out other
|   compilers not supporting them. Your rename seems reasonable, I would go
|   with that.

Perhaps add a comment in the code to explain this point?

|   The natFromInt && friends are the same, just with explicit INLINEs (at
|   some point it did not get inlined and that caused a lot of problems).

Can you add a comment at the INLINE pragma to explain why the pragma is so important?  You know, but in two years time someone looking at the code won't know.

|   > | nomatch x p m = False
|   > }}}
|   > clause in `member`, as compared to `lookup`.
|  
|   This is as it was in 0.3. It is semantically correct both ways. The only
|   think is time complexity. The member is a bit faster if the key looked for
|   is _not_ in the set, but it does slightly more work if the key is present.
|   We can decide both ways and make it consistent.

Again, please add a comment to explain this point.  You know, but in two years time someone else is going to be puzzled.


You'll notice a common pattern to my suggestions :-). Twenty years experience with GHC has taught me to be scrupulous about documenting tricky points in the code.  I have taken ot using the "Note [blah]" style a lot, because it avoids cluttering the code itself.

Simon