Hello,
I'd like to propose bugfixes, documentation fixes and a performance
improvement for Text.PrettyPrint.HughesPJ. The changes shouldn't
effect the expected behaviour of the PP library.
I've written a QuickCheck test suite for the pretty printer (to test
the improvement), and found two bugs and some misconceptions/
ambiguities in the documentation. Additionally, there is a
microbenchmark for the suggested improvement.
Both are available at http://code.haskell.org/~bhuber/Text/
PrettyPrint/. Note that the QuickCheck tests need access to all top
level names in HughesPJ (i.e. ignore the export list).
In summary, I propose to
* fix a bug in fillNB and one in fillNB/sepNB
* correct documentation on laws and invariants.
* add more efficient implementations of vcat,hsep,hcat
More specifically:
(1) Bugfix fillNB: Additional case for fillNB Empty (Empty : ys)
(2) Bugfix [f](catsep): do not allow overlapping ($$) in vertical
composition
(3) Lazy implementations of vcat,hcat and hsep
(4) Law isn't always true
(5) Invariant 5 should be made stronger
(6) Change the comment about negative indentation
The details follow below (maybe a little long).
best regards,
benedikt
=
=
=
=
=
=
=
=
========================================================================
== Details (Long) ==
Bug Fixes

(1) Bugfix fillNB
Law states that
> sep (ps++[empty]++qs) = sep (ps ++ qs)
> ...ditto hsep, hcat, vcat, fill...
In the current implementation, this fails for the paragraph fill
variants.
> render' $ fill True [ text "c", text "c",empty, text "c", text "b"]
> where render' = renderStyle (Style PageMode 7 1.4)
>> c c c
>> b
The reason is a missing test for Empty in fillNB
2) Bugfix: overlap and f?(catsep)
The specification for cat/sep:
* oneLiner (hcat/hsep ps)
`union`
vcat ps [*]
But currently cat, sep, fcat and fsep attempt to overlap the second
line with the first one, i.e. they use
`foldr ($$) empty ps' instead of `foldr ($+$) empty ps' [*]. I assume
this is a mistake.
This bug can lead to situations, where the line in the right argument
of Union is actually longer:
> prettyDoc$ cat [ text "a", nest 2 ( text "b") ]
>> text "a"; union
>> (text "b"; empty)
>> (nilabove; nest 1; text "b"; empty)
> renderStyle (Style PageMode 1 1) $ cat [ text "a", nest 2 ( text
"b") ]
>> "a b"
In the implementation, we call `nilAbove False' instead of `nilAbove
True' (see patch).
Performance Improvements

3) Improving the space/time performance of vcat, hsep, hcat
vcat (hsep,cat) is implemented in an unneccessarily strict way.
We only get some output after all of vcat's arguments are evaluated
and checked against being Empty.
This can be improved by only checking the right argument of foldr
against being Empty, and
then applying an Emptyfilter on the resulting Doc. Space improvement
is obvious.
The microbenchmark (code.haskell.org/~bhuber/Text/PrettyPrint/
HughesPJPerfCheck.hs) suggests that
the improvements in time are remarkable too.
Documentation fixes

The QuickCheck tests revealed that
4) Law isn't always true
text "" <> x = x, if x nonempty
only holds if x does not start with nest (obviously, because of law
).
5) The invariant 5 should be extended:
A @[email protected] may only appear on the first line of the left argument of
an union. Therefore, the right argument of an union can never be
equivalent
to the empty set.
because this assumption is used when rendering.
6) Change the comment about negative indentation
In the source code we have:
 (spaces n) generates a list of n spaces
 It should never be called with 'n' < 0, but that can happen for
reasons I don't understand
If we compose a <> b, and the first line of b is deeply nested, but
other lines of b are not,
then, because <> eats the nest, the pretty printer will try to layout
some of b's lines with
negative indentation:
doc 0123345

d1 a
d2  b
c
d1<>d2 ab
c
Here is the reason why this is rather unavoidable: In John Hughes
paper, there is a line stating that
"composing layouts does not change the layouts being composed".
Another one states that
"<> should eat up nests"
But this leads to negative indentation  imho a user error.
== End ==
