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

internal: use custom rendering for --expandArc #813

Merged
merged 2 commits into from
Jul 26, 2023

Conversation

zerbina
Copy link
Collaborator

@zerbina zerbina commented Jul 25, 2023

Summary

Report the output for --expandArc via manual msgWrite calls instead
of using localReport and use custom render-to-text logic for the
PNode tree. The rendering logic is kept simple, and while the
output of --expandArc stays largely the same, it doesn't represent
valid NimSkull code anymore.

This is a preparation for the introduction of a code-generator IR, as
with it, astgen is not going to output PNode AST anymore, meaning
that renderTree cannot be used there.

Details

The rsemExpandArc report is removed and everything associated with it.
While it could be kept, the general direction it to move aways from
reports for compiler tracing, and so msgWrite is instead used. This
also helps with getting around cyclic imports once the new IR is
introduced.

The rendering logic is a simplified version of renderTree, with
more complex text-layouting and conditional logic removed. The supported
AST shapes are similar to that of the planned initial version of the
code-generator IR, and the routines are added to the new cgirutils
module.

In order for changes to the expandArc-using tests to stay small, the
rendered output is kept compatible with that of
renderTree(n, {renderIr, renderNoComment}), where reasonable.

Summary
=======

Report the output for `--expandArc` via manual `msgWrite` calls instead
of using `localErport` and use custom render-to-text logic for the
`PNode` tree. The rendering logic is kept simple, and while the
output of `--expandArc` stays largely the same, it doesn't represent
valid NimSkull code anymore.

This is a preparation for the introduction of a code-generator IR, as
with it, `astgen` is not going to output `PNode` AST anymore, meaning
that `renderTree` cannot be used there.

Details
=======

The `rsemExpandArc` report is removed and everything associated with it.
While it could be kept, the general direction it to move aways from
reports for compiler tracing, and so `msgWrite` is instead used. This
also helps with getting around cyclic imports once the new IR is
introduced.

The rendering logic is a simplified version of `renderTree`, with
more complex text-layouting and conditional logic removed. The supported
AST shapes are similar to that of the planned initial version of the
code-generator IR, and the routines are added to the new `cgirutils`
module.

In order for changes to the `expandArc`-using tests to stay small, the
rendered output is kept compatible with that of
`renderTree(n, {renderIr, renderNoComment})`, where reasonable.
@zerbina zerbina added the compiler General compiler tag label Jul 25, 2023
@zerbina zerbina added this to the MIR phase milestone Jul 25, 2023
Copy link
Collaborator

@saem saem left a comment

Choose a reason for hiding this comment

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

overall change looks good.

taking a peek at CI seems tests/lang_objects/destructor/tv2_cast.nim needs an update.

@saem
Copy link
Collaborator

saem commented Jul 26, 2023

/merge

@github-actions
Copy link

Merge requested by: @saem

Contents after the first section break of the PR description has been removed and preserved below:


Notes for Reviewers

@chore-runner chore-runner bot added this pull request to the merge queue Jul 26, 2023
Merged via the queue into nim-works:devel with commit 61414fa Jul 26, 2023
18 checks passed
@zerbina zerbina deleted the independent-expand-arc branch July 26, 2023 22:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler General compiler tag
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants