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

Total type prettyprinting #69

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

luna-spirito
Copy link
Contributor

Current Pretty (Type s) implementation contains possibly infinite path ... -> prettyQuantifiedType -> prettyFunctionType -> prettyApplicationType -> prettyPrimitiveType -> prettyQuantifiedType -> ....

While the current implementation never runs into this infinite recursion, adding new types while forgetting to update prettyQuantifiedType yields no warnings and causes run-time infinite recursion that is hard to debug.

This PR removes multiple functions with error-prone wildcard matches and adds a single function that matches on all possibilities to generate a render tree, which is then automatically reduced and prettyprinted.

@Gabriella439
Copy link
Owner

Yeah, I'm aware of the potential for infinite recursion but I'm sort of partial (pun intended) to the current implementation and I explain why a bit in this post: https://www.haskellforall.com/2020/11/pretty-print-syntax-trees-with-this-one.html

@luna-spirito
Copy link
Contributor Author

Totally (pun intended) agree that the current implementations looks cleaner. But I proposed this PR due to the "ready-to-fork" nature of the language: I'm a compilation-error-driven developer, and I'm afraid that many people who are new to the codebase, just like me, will forget to add the type prettyprinting and then wiill have to start up the debugger and step through everything in order to find the infinite loop.

As an alternative, maybe I could make a PR that introduces "consumed" Bool flag to all the functions to detect the infinite loop and at least throw impure error exception? Or some other approach?

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