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

Implement pretty-printer #29

Open
LPTK opened this issue Sep 30, 2017 · 4 comments
Open

Implement pretty-printer #29

LPTK opened this issue Sep 30, 2017 · 4 comments

Comments

@LPTK
Copy link
Member

LPTK commented Sep 30, 2017

The legacy IRs implemented pretty-printing by converting the code into a Scala AST and then using the Scala pretty-printer!
Not ideal because it's slow, and because Scala's pretty-printer can be glitchy.
The new pretty-printer should:

  • break lines only if necessary (can take inspiration from this);
  • print variables as original name + '_' + counter starting from 0 (to avoid clashes);
  • ideally, have a notion of precedence and associativity to avoid some parentheses;
  • ideally, be reusable (implemented as a Base), but with hooks to help print virtualized constructs such as if then else correctly (instead of as squid.lib.IfThenElse).
@fschueler
Copy link
Contributor

Hi, I started playing around with Squid after reading the paper and implemented a very simple Pretty-Printer to explore the structure a little bit. If it's of any interest I could try to make it comply with the things listed in this issue.

My first stab is under this commit and I followed the structure of the InterpreterTests.scala.

@LPTK
Copy link
Member Author

LPTK commented Oct 18, 2017

Hello Felix, thanks for your interest in this project! You contribution is very much welcome.
These commits are a good start. I'm going to leave a few comments to help you figure things out 😉

@fschueler
Copy link
Contributor

Thanks Lionel, for taking the time to look at it! I really like the project and look forward to playing around with it for some DSL work!

I have addressed your comments and also added the type-arguments to method applications. Everything is a String now so I removed the Runtime-trait, too.
The output is a little bit verbose now but that's probably a good thing (?). I kept the decoded names for method applications to make it not too verbose.

Let me know what you think! I would open a PR and address the other things that are mentioned in this issue if you think it's okay. The link for inspiration regarding the line-breaks gets me a 404 though...

@LPTK
Copy link
Member Author

LPTK commented Oct 18, 2017

Great! Feel free to make the PR when you're satisfied with the implementation.

The output is a little bit verbose now but that's probably a good thing (?)

Unfortunately, since the code is fully-typed and resolved, it's normal to get quite a lot of verbosity. The bright side is that I think we could factor names that are used more than once into local imports, which would make reading the code much easier! The idea would be to override wrapConstruct to make it instantiate some temporary state that keeps track of all type symbols being loaded (via loadTypSymbol), and then for names that are used several times use the short form and make wrapConstruct insert some imports at the beginning of the result.
The wrapConstruct method is supposed to be called whenever some code representation is constructed; for example when you call myCode reinterpretIn MyPrettyPrinter.

The link for inspiration regarding the line-breaks gets me a 404 though...

Right, sorry about that. Here is the code I was referring to (in particular, Document.scala): https://www.dropbox.com/s/rrbo60tu9ul9nln/document.zip?dl=0
Perhaps the cleanest way to do everything would be to generate instances of some Document-like data type instead of Strings, and then have a stringification method in Document that takes into account the max line width, the required imports, etc... ?

EDIT: I realize that the technique described above to introduce imports would be problematic to implement, because there is no easy way to know how many times a name introduced by loadTypSymbol will be used by the following code. However, if we have a Document-like abstraction that understands qualified names, this would not be a problem.

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

No branches or pull requests

2 participants