Skip to content

WIP: Implement 'shrink' for Docs by hand#113

Open
sjakobi wants to merge 3 commits intomasterfrom
manual-shrinking
Open

WIP: Implement 'shrink' for Docs by hand#113
sjakobi wants to merge 3 commits intomasterfrom
manual-shrinking

Conversation

@sjakobi
Copy link
Collaborator

@sjakobi sjakobi commented Jan 20, 2020

No description provided.

@sjakobi
Copy link
Collaborator Author

sjakobi commented Jan 20, 2020

If I cause the fusion tests to fail, for example with

--- a/prettyprinter/src/Data/Text/Prettyprint/Doc/Internal.hs
+++ b/prettyprinter/src/Data/Text/Prettyprint/Doc/Internal.hs
@@ -551,7 +551,7 @@ changesUponFlattening :: Doc ann -> Maybe (Doc ann)
 changesUponFlattening = \doc -> case doc of
     FlatAlt _ y     -> Just (flatten y)
     Line            -> Just Fail
-    Union x _       -> changesUponFlattening x <|> Just x
+    Union x _       -> Nothing
     Nest i x        -> fmap (Nest i) (changesUponFlattening x)
     Annotated ann x -> fmap (Annotated ann) (changesUponFlattening x)

shrink still produces nonsensical Docs like Cat Empty Fail:

Tests
  Fusion
    Deep fusion does not change rendering:                          FAIL (0.03s)
      *** Failed! (after 61 tests and 44 shrinks):
      Exception:
        »SFail« must not appear in a rendered »SimpleDocStream«. This is a bug in the layout algorithm! Please report this as a bug
        CallStack (from HasCallStack):
          error, called at src/Data/Text/Prettyprint/Doc/Render/Util/Panic.hs:13:21 in prettyprinter-1.5.1-inplace:Data.Text.Prettyprint.Doc.Render.Util.Panic
      Cat Empty Fail
      LayoutPretty (LayoutOptions {layoutPageWidth = AvailablePerLine 55 56.4237144509971})
      Exception thrown while showing test case:
        »SFail« must not appear in a rendered »SimpleDocStream«. This is a bug in the layout algorithm! Please report this as a bug
        CallStack (from HasCallStack):
          error, called at src/Data/Text/Prettyprint/Doc/Render/Util/Panic.hs:13:21 in prettyprinter-1.5.1-inplace:Data.Text.Prettyprint.Doc.Render.Util.Panic
      
      Use --quickcheck-replay=824164 to reproduce.

I think what could help is to filter the raw shrink results with a valid function that checks certain invariants such as the absence of Fail (except in a first Union alternative?!).

Such a valid function could be good documentation too…

Nest _ x -> go mayFail x
Union x y -> go True x && go mayFail y
Column f -> all (go mayFail) (map f [0..80])
WithPageWidth f -> all (go mayFail) (map f (Unbounded : [AvailablePerLine c r | c <- [1..80], r <- [0, 0.1 .. 1]]))
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is 0 a valid ribbon width at all? Not sure how to interpret the docs there:

https://github.com/quchen/prettyprinter/blob/ff555e19b7b17a74f16e6fb062256a57dabe4d92/prettyprinter/src/Data/Text/Prettyprint/Doc/Internal.hs#L1609-L1618

Suggested change
WithPageWidth f -> all (go mayFail) (map f (Unbounded : [AvailablePerLine c r | c <- [1..80], r <- [0, 0.1 .. 1]]))
WithPageWidth f -> all (go mayFail) (map f (Unbounded : [AvailablePerLine c r | c <- [1..80], r <- [0.1, 0.2 .. 1]]))

Cat x y -> go mayFail x && go mayFail y
Nest _ x -> go mayFail x
Union x y -> go True x && go mayFail y
Column f -> all (go mayFail) (map f [0..80])
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure how to best handle these functions. Checking them exhaustively would take too long. Even the current check with "typical" values 0..80 seems excessive.

shrink doc = filter valid $ case doc of
Fail -> [Empty]
Empty -> []
Char c -> Empty : map Char (filter (/= '\n') (shrink c))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be one of the few places where a list comprehension is nicer to read! :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants