Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pretty-printing saw-core syntax results in way too many line-breaks #1618

Closed
brianhuffman opened this issue Mar 17, 2022 · 6 comments · Fixed by #1636
Closed

Pretty-printing saw-core syntax results in way too many line-breaks #1618

brianhuffman opened this issue Mar 17, 2022 · 6 comments · Fixed by #1636
Labels
subsystem: saw-core Issues related to the saw-core representation or the saw-core subsystem

Comments

@brianhuffman
Copy link
Contributor

Pretty-printing a large saw-core term invariably gives you output that looks something like this:

                        in Prelude.either x@3 x@5 (Prelude.CompM x@4)
                             (\(x_left0 : x@3) ->
                                Prelude.sawLet Prelude.Nat
                                  (Prelude.CompM (Prelude.Vec 64 Prelude.Bool))
                                  0
                                  (\(z' : Prelude.Nat) ->
                                     Prelude.sawLet Prelude.Bool
                                       (Prelude.CompM
                                          (Prelude.Vec 64 Prelude.Bool))
                                       Prelude.True
                                       (\(z'' : Prelude.Bool) ->
                                          let { x@6 = Prelude.Vec 64
                                                        Prelude.Bool
                                              }
                                           in Prelude.sawLet x@6
                                                (Prelude.CompM x@6)
                                                0x0000000000000000
                                                (\(z''' : x@6) ->
                                                   Prelude.sawLet Prelude.Bool
                                                     (Prelude.CompM
                                                        (Prelude.Vec 64
                                                           Prelude.Bool))
                                                     Prelude.True
                                                     (\(z'''' : Prelude.Bool) ->
                                                        Prelude.sawLet
                                                          Prelude.Nat
                                                          (Prelude.CompM
                                                             (Prelude.Vec 64
                                                                Prelude.Bool))
                                                          0
                                                          (\(z'''''
                                                               : Prelude.Nat) ->
                                                             let { x@7 = Prelude.Vec
                                                                           1
                                                                           Prelude.Bool
                                                                 }
                                                              in Prelude.sawLet
                                                                   x@7
                                                                   (Prelude.CompM
                                                                      (Prelude.Vec
                                                                         64
                                                                         Prelude.Bool))
                                                                   0b1
                                                                   (\(z''''''
                                                                        : x@7) ->
                                                                      Prelude.sawLet
                                                                        Prelude.Bool
                                                                        (Prelude.CompM
                                                                           (Prelude.Vec
                                                                              64
                                                                              Prelude.Bool))
                                                                        Prelude.True
                                                                        (\(z'''''''
                                                                             : Prelude.Bool) ->
                                                                           Prelude.sawLet
                                                                             Prelude.Bool
                                                                             (Prelude.CompM
                                                                                (Prelude.Vec
                                                                                   64
                                                                                   Prelude.Bool))
                                                                             Prelude.False
                                                                             (\(z''''''''
                                                                                  : Prelude.Bool) ->
                                                                                Prelude.sawLet
                                                                                  Prelude.Bool
                                                                                  (Prelude.CompM
                                                                                     (Prelude.Vec
                                                                                        64
                                                                                        Prelude.Bool))
                                                                                  Prelude.True
                                                                                  (\(z'''''''''
                                                                                       : Prelude.Bool) ->
                                                                                     Prelude.sawLet
                                                                                       Prelude.Bool
                                                                                       (Prelude.CompM
                                                                                          (Prelude.Vec
                                                                                             64
                                                                                             Prelude.Bool))
                                                                                       Prelude.True
                                                                                       (\(z''''''''''
                                                                                            : Prelude.Bool) ->
                                                                                          Prelude.returnM
                                                                                            (Prelude.Vec
                                                                                               64
                                                                                               Prelude.Bool)
                                                                                            0x0000000000000000)))))))))))

After the first few lines, once the indentation starts to get large, the prettyprinter decides to put only one identifier or token per line, as if that's supposed to make it easier to read. It prints out so many newlines that quite often a pretty-printed term will exceed the maximum size of my terminal's scrollback buffer.

We need to change the default page width and ribbon width settings for the saw-core prettyprinter, so that we don't get such garbage output.

@brianhuffman brianhuffman added the subsystem: saw-core Issues related to the saw-core representation or the saw-core subsystem label Mar 17, 2022
@kquick
Copy link
Member

kquick commented Mar 17, 2022

Also note that Prettyprinter has the group and flatAlt functions that might be helpful in this regard.

@brianhuffman
Copy link
Contributor Author

I believe we're using group etc. in basically the way we want; it's just that with the default layout configuration, as soon as anything exceeds 80 columns it starts putting linebreaks everywhere they can possibly go. Changing the default layout configuration should take care of this problem.

@brianhuffman
Copy link
Contributor Author

Layout options are specified in four (!) different places in the code that I can find:

renderSawDoc :: PPOpts -> SawDoc -> String
renderSawDoc ppOpts doc =
Text.Lazy.unpack (renderLazy (style (layoutPretty layoutOpts doc)))
where
layoutOpts = LayoutOptions (AvailablePerLine 80 0.8)
style = if ppColor ppOpts then reAnnotateS colorStyle else unAnnotateS

renderDoc :: PP.Doc ann -> String
renderDoc doc = PP.renderString (PP.layoutPretty opts doc)
where opts = PP.LayoutOptions (PP.AvailablePerLine 80 0.8)

renderDoc :: Doc ann -> String
renderDoc doc = renderString (layoutPretty opts doc)
where opts = LayoutOptions (AvailablePerLine 80 0.8)

error ("Unhandled expression form: " ++
(renderString (layoutSmart opts (mbLift $ fmap (ppApp (const $ pretty ("_" :: String))) mb_e))))
where opts = PP.LayoutOptions (PP.AvailablePerLine 80 0.8)

I don't think all of these are used for pretty-printing saw-core terms, but we should still reconsider all of these.

@eddywestbrook
Copy link
Contributor

Ooh, yeah, that's a little embarrassing. :) Agreed, those should all definitely use a single layout function defined in Pretty.hs.

@brianhuffman
Copy link
Contributor Author

Yeah, any that are used for printing saw-core terms should use a default layout defined in Pretty.hs. Layout configurations used for printing other types might be better off using defaultLayoutOptions from the prettyprinter library, unless we have a specific reason to prefer something else.

@brianhuffman
Copy link
Contributor Author

Since saw-core terms so regularly exceed the 80-column width, I'm thinking that we might rather just set the page width to something astronomically large, and then just set the ribbon width to something less than 80. This way we'll get output shaped like this:

******
 ******
  ******
   ******
    ******
     ******
      ******
       ******
        ******

instead of like this

******
 ******
  ******
   *****
    ****
     ***
      **
       *
        *
         *
          *
           *

brianhuffman pushed a commit that referenced this issue Apr 22, 2022
This is accomplished by using a layout with an extremely large
page width, combined with a normal (64-character) ribbon width.
With this setup, the pretty-printer should almost never in practice
go into the bayond-the-margin mode where it prints only one token
per line.

Fixes #1618.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
subsystem: saw-core Issues related to the saw-core representation or the saw-core subsystem
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants