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

bits.h cleanups after #2879 #3156

Merged
merged 2 commits into from
Sep 22, 2020
Merged

bits.h cleanups after #2879 #3156

merged 2 commits into from
Sep 22, 2020

Conversation

kripken
Copy link
Member

@kripken kripken commented Sep 21, 2020

Improve some comments, and remove fast paths that are just optimizations for
compile time.

About the fast paths, is there a previous discussion I missed? They seem
unnecessary unless I'm not seeing something. A minor speedup from them
doesn't seem worth more complicated code - this code is pretty complex as
it is.

cc @MaxGraey

@kripken kripken changed the title Cleanups after #2879 bits.h after #2879 Sep 21, 2020
@kripken kripken changed the title bits.h after #2879 bits.h cleanups after #2879 Sep 21, 2020
@MaxGraey
Copy link
Contributor

LGTM

Copy link
Member

@tlively tlively left a comment

Choose a reason for hiding this comment

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

LGTM. I approved the code with the fast paths, but hadn't asked too many critical questions about them. Sorry if I missed any previous comments you had made about them.

@kripken kripken merged commit 0f9339d into master Sep 22, 2020
@kripken kripken deleted the bits2 branch September 22, 2020 15:48
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