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

better pretty-printing of module shapes #11697

Merged
merged 2 commits into from
Nov 4, 2022
Merged

Conversation

gasche
Copy link
Member

@gasche gasche commented Nov 3, 2022

(Remember: "shapes" are an abstraction of the module structure of OCaml programs introduced in #10718.)

I am trying again to understand what it is about the Irmin source code that made earlier implementations of shape-reduction blow up. This involves looking at the -dshape output for a manually-minized subset of irmin.ml, whose printing starts like this:

 "Maker_ext"[module] ->
     Abs<Irmin_short.167>
        (CA/1399,
         Abs
            (AW/1400,
             Abs
                (N/1479,
                 Abs
                    (CT/1523,
                     {
                      "Make"[module] ->
                          Abs<Irmin_short.166>
                             (M/1529,
                              Abs
                                 (C/1555,
                                  Abs
                                     (P/1574,
                                      Abs
                                         (B/1588,
                                          Abs
                                             (H/1589,
                                              {
                                               "AW"[module] ->
                                                   Abs<Irmin_short.111>
                                                      (K/1234,
                                                       Abs
                                                          (V/1235,
                                                           {
                                                            "clear"[value] ->
                                                                <Irmin_short.97>;
                                                            "close"[value] ->
                                                                <Irmin_short.95>;
                                                            "find"[value] ->
                                                                <Irmin_short.62>;

(I won't show the whole output because it takes 2GiB.)

This PR introduces a compact notation for n-ary functors (and reduces the structure indentation slightly). -dshape then gives the following result:

 "Maker_ext"[module] ->
   Abs<Irmin_short.167>
      (CA/1399, AW/1400, N/1479, CT/1523,
       {
        "Make"[module] ->
          Abs<Irmin_short.166>
             (M/1529, C/1555, P/1574, B/1588, H/1589,
              {
               "AW"[module] ->
                 Abs<Irmin_short.111>
                    (K/1234, V/1235,
                     {
                      "clear"[value] -> <Irmin_short.97>;
                      "close"[value] -> <Irmin_short.95>;

which is much nicer to read and work with.

Copy link
Member

@dra27 dra27 left a comment

Choose a reason for hiding this comment

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

LGTM - perhaps let @trefis or @voodoos have a chance to see prior to merging?

@voodoos
Copy link
Contributor

voodoos commented Nov 4, 2022

Looks good to me too. It does read much easier than before.

@gasche
Copy link
Member Author

gasche commented Nov 4, 2022

Thanks! I must say that the existing pretty-printing code was rather good already. I rarely use Format and need some time to get back into it whenever, and there was an impressive display of various options used in the existing pretty-printer. I spent some amount of time trying to tweak the code in various ways, to generally worse results.

@gasche gasche merged commit 466bd5b into ocaml:trunk Nov 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants