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

cgir: remove the legacy CGIR variant #1234

Merged
merged 2 commits into from
Mar 13, 2024

Conversation

zerbina
Copy link
Collaborator

@zerbina zerbina commented Mar 12, 2024

Summary

All code generators use the new goto-using variant of the CGIR now,
rendering the legacy variant obsolete. This is an internal-only
refactoring.

Details

  • remove the cgirgen_legacy module
  • remove the node kinds belonging to the legacy variant
  • remove the sets introduced for the transition
  • remove some leftover usages of cnkStmtListExpr (they were dead code
    already)

These are the node kinds only produced by the legacy version of
`cgirgen`.
@zerbina zerbina added compiler/backend Related to backend system of the compiler simplification Removal of the old, unused, unnecessary or un/under-specified language features. labels Mar 12, 2024
@zerbina zerbina added this to the MIR phase milestone Mar 12, 2024
@saem
Copy link
Collaborator

saem commented Mar 12, 2024

/merge

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

  • removing cnkStmtList is a bit more involved, it's better done as a follow-up

@chore-runner chore-runner bot added this pull request to the merge queue Mar 12, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Mar 12, 2024
@zerbina zerbina added this pull request to the merge queue Mar 13, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Mar 13, 2024
@zerbina
Copy link
Collaborator Author

zerbina commented Mar 13, 2024

Hm, strange. The compiler crashed during the leak test (but only during the second attempt at merging). I don't see how the changes here could have caused that, so I'll give merging another go.

@zerbina zerbina added this pull request to the merge queue Mar 13, 2024
Merged via the queue into nim-works:devel with commit 0d03576 Mar 13, 2024
31 checks passed
@zerbina zerbina deleted the remove-legacy-cgir-variant branch March 13, 2024 01:21
alaviss pushed a commit to nim-works/nimskull-test-bed that referenced this pull request Jul 5, 2024
## Summary

Clean up the category and enable it for all targets by default. This
significantly increases test coverage of exceptions for the JS and VM
targets.

## Details

**General changes/cleanup:**
* references to issues like `nim-works#1234` are replaced with proper GitHub
  links
* the `targets` key is removed for tests that should be run on all
  targets
* usage of `cmd` is replaced with usage of `matrix` (allowing for
  multi-target testing)
* `write(stdout, ...)` is replaced with `echo` in order to support the
  JS and VM targets
* execution of `static:` blocks is replaced with using the VM target,
  when the test is not specific to compile-time execution
* tests that don't work but should are marked as `knownIssue`

**Specific changes:**
* where not necessary for the test, `getCurrentExceptionMsg` is
  replaced with `getCurrentException().msg`, as the former is not yet
  supported by the VM
* `texceptions2.nim` is removed -- it's redundant with `texception.nim`
* `texception_message_null_byte.nim` is turned into a normal test that
  doesn't rely on self-execution
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler/backend Related to backend system of the compiler simplification Removal of the old, unused, unnecessary or un/under-specified language features.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants