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: remove sons from leaf nodes empty/none #392

Merged
merged 1 commit into from
Aug 6, 2022

Conversation

saem
Copy link
Collaborator

@saem saem commented Jul 31, 2022

Summary

  • pulls nkEmpty and nkNone into a branch without sons
  • ensures more correctness by design

Details

  • various small fixes to get bootstrap working
  • no tests failed, so that's a good{?} sign

Copy link
Collaborator

@zerbina zerbina left a comment

Choose a reason for hiding this comment

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

Left a small suggestion.

As an aside, I think it'd make future tracking down of changes (e.g. via git blame) easier if the refactorings unrelated to the main topic of the PR (single-line statements to multi-line, for it in n instead of for i in 0..<n.len, etc) are done in a separate PR. I don't consider this to be a blocker however.

compiler/ast/ast.nim Outdated Show resolved Hide resolved
compiler/ast/ast.nim Outdated Show resolved Hide resolved
@saem saem force-pushed the saem-pnode-enforce-no-sons-on-leaves branch 2 times, most recently from e40ddb4 to 1c5f79c Compare August 2, 2022 00:19
- pulls nkEmpty and nkNone into a branch without sons
- various small fixes to get bootstrap working
- no tests failed, so that's a good{?} sign
@saem saem force-pushed the saem-pnode-enforce-no-sons-on-leaves branch from 1c5f79c to 0f14ce0 Compare August 2, 2022 00:29
@saem
Copy link
Collaborator Author

saem commented Aug 2, 2022

Bots r+

Comment on lines +433 to +436
if father.len == 0:
return
for i in idx..<father.len - 1:
father[i] = father[i + 1]
Copy link
Contributor

@Clyybber Clyybber Aug 2, 2022

Choose a reason for hiding this comment

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

Preexisting, but that if check is unneccessary, as the loop body will not execute if father.len is zero.

@saem
Copy link
Collaborator Author

saem commented Aug 6, 2022

bors r+

bors bot added a commit that referenced this pull request Aug 6, 2022
392: internal: remove sons from leaf nodes empty/none r=saem a=saem

## Summary
- pulls nkEmpty and nkNone into a branch without sons
- ensures more correctness by design

## Details
- various small fixes to get bootstrap working
- no tests failed, so that's a good{?} sign



Co-authored-by: Saem Ghani <[email protected]>
@bors
Copy link
Contributor

bors bot commented Aug 6, 2022

Timed out.

@saem
Copy link
Collaborator Author

saem commented Aug 6, 2022

Bots r+

@disruptek
Copy link
Contributor

bors r+

@bors bors bot merged commit 086304b into nim-works:devel Aug 6, 2022
@bors
Copy link
Contributor

bors bot commented Aug 6, 2022

Build succeeded:

@haxscramper haxscramper added this to the Sem phase refactoring milestone Nov 21, 2022
@saem saem deleted the saem-pnode-enforce-no-sons-on-leaves branch January 22, 2023 18:44
@saem saem restored the saem-pnode-enforce-no-sons-on-leaves branch January 22, 2023 18:45
@saem saem deleted the saem-pnode-enforce-no-sons-on-leaves branch January 22, 2023 18:45
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.

None yet

5 participants