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

Remove inspection function for syntax tree nodes #200

Merged
merged 11 commits into from
Jan 2, 2024

Conversation

chengluyu
Copy link
Member

@chengluyu chengluyu commented Dec 30, 2023

This PR is extracted from #194. But I'm actually quite dubious about this PR, because manually creating a inspect function for every syntax nodes is really troublesome. I wonder if we can do this by macros or is there anything builtin that can achieve the similar results.

TL;DR

The most important change this PR makes is, if you want to:

  • see a code representation of a syntax tree, please call showDbg;
  • see how a syntax tree is constructed (the raw structure), please call toString.

Changes

  • Add showDbg function to Statement (incl. Term, etc), TypingUnit, and Pgrm. The function showDbg was there but not available at all of them.
  • Remove toString functions of these syntax nodes, so that the compiler will synthesize a default toString for them.
  • Remove mlscript.codegen.Helpers.inspect and replace all its call with toString.

Note

I spent some time to make sure that showDbg behaves in the same way as toString before. As a result, only test files that involves the output from mlscript.codegen.Helpers.inspect should be changed in this PR.

This commit was cherry-picked from a future commit which changed
many files.
I carefully created `showDbg` for many syntax nodes so that this
commit does not change any test files.
@chengluyu chengluyu changed the title Extract the inspect function as a general utility Remove inspection function for syntax tree nodes Dec 30, 2023
Copy link
Contributor

@LPTK LPTK left a comment

Choose a reason for hiding this comment

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

Nice simplification. But while we're at it, could you please remove the toString printing for all the compiler tests? We should only print this stuff when :p is on. It's far too verbose and noisy to be done by default.

shared/src/test/scala/mlscript/DiffTests.scala Outdated Show resolved Hide resolved
Copy link
Member Author

@chengluyu chengluyu left a comment

Choose a reason for hiding this comment

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

Hello, I have gone through it myself, and left two comments on areas I'm not quite sure about.

shared/src/main/scala/mlscript/helpers.scala Outdated Show resolved Hide resolved
shared/src/main/scala/mlscript/helpers.scala Outdated Show resolved Hide resolved
@chengluyu chengluyu marked this pull request as ready for review January 1, 2024 15:24
@LPTK LPTK merged commit 29e38d1 into hkust-taco:new-definition-typing Jan 2, 2024
1 check passed
@LPTK LPTK deleted the utils-inspect branch January 2, 2024 08:36
@chengluyu
Copy link
Member Author

Thanks!

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