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

Cosmetic compiler cleanup #12718

Merged
merged 8 commits into from
Nov 28, 2019
Merged

Cosmetic compiler cleanup #12718

merged 8 commits into from
Nov 28, 2019

Conversation

Clyybber
Copy link
Contributor

@Clyybber Clyybber commented Nov 23, 2019

Summary

  • SomePNodeOrPType.sons[...] ->. SomePNodeOrPType[...]

  • SomeIndexable[SomeIndexable.len - x] -> SomeIndexable[^x]

  • Removed L/length variables where they went unused due to the above change

  • Removed the PTransfNode type from transf.nim

  • Removed addSon and replaced it with add since it is the same

  • Changed all add calls to use method call syntax

  • Changed all len/safeLen calls to use method call syntax

  • Changed ranges of the form x..something.len-(y+1) to x..<something.len-y

@Clyybber Clyybber changed the title Modernize/Cleanup compiler code base (cosmetic) Cosmetic compiler cleanup Nov 23, 2019
Copy link
Member

@narimiran narimiran left a comment

Choose a reason for hiding this comment

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

Regarding the "Unify range operators" commit — I would like to see it unified in the opposite direction, with spaces before and after .. and ..<, as that is the recommended style.

@narimiran narimiran closed this Nov 24, 2019
@narimiran narimiran reopened this Nov 24, 2019
@Clyybber
Copy link
Contributor Author

Clyybber commented Nov 24, 2019

@narimiran I think x..y/x..<y captures the idea of a range better, since the operator is connecting both ends, as opposed to x .. y/x ..< y.

compiler/ast.nim Outdated Show resolved Hide resolved
@Araq Araq merged commit 7e747d1 into nim-lang:devel Nov 28, 2019
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.

3 participants