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.

Sign up for GitHub

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

Jump to bottom

Remove C-isms? #37

Open
danluu opened this issue May 24, 2016 · 2 comments
Open

Remove C-isms? #37

danluu opened this issue May 24, 2016 · 2 comments
Labels
c++

Comments

@danluu
Copy link
Contributor

danluu commented May 24, 2016 •
edited
Loading

We have a lot of C-isms in the older BitFunnel code. From talking to @MikeHopcroft , there seems to be no particular reason for this expect that this code is old and was written by people coming in with a C background.

I suspect this bug is so low priority that we'll never get to it, but it would be nice to clean some of this up.
@hausdorff
Copy link
Member

One particular suggestion, should we ever get around to it: remove C-style casts. :)

@danluu
Copy link
Contributor Author

danluu commented Jun 30, 2016

I think we'll never fix those if we get too many of them, so I'm enabling -Wold-style-cast for our gcc/clang builds (it appears that might be a placebo for clang, but it works for gcc) and fixing all current instances. Having the compiler option should force us to be C-cast clean as we port code over and I don't think it will feel like all that much work to fix them as they come in.

Sending out a codeflow shortly.

danluu added a commit to danluu/BitFunnel that referenced this issue Jun 30, 2016
@danluu
Remove C-style casts. See BitFunnel#37.
5e6da65 
Enable -Wold-style-cast and also remove all existing C-style casts. Doing this
found multiple places where const was incorrectly being cast away, as well as
some pointless casting of size_t down to int.
@danluu danluu mentioned this issue Jun 30, 2016
Closed
danluu added a commit to danluu/BitFunnel that referenced this issue Jul 1, 2016
@danluu
Remove C-style casts. See BitFunnel#37.
00a8c76 
Enable -Wold-style-cast and also remove all existing C-style casts. Doing this
found multiple places where const was incorrectly being cast away, as well as
some pointless casting of size_t down to int.
danluu added a commit to danluu/BitFunnel that referenced this issue Jul 1, 2016
@danluu
Remove C-style casts. See BitFunnel#37.
636d252 
Enable -Wold-style-cast and also remove all existing C-style casts. Doing this
found multiple places where const was incorrectly being cast away, as well as
some pointless casting of size_t down to int.
danluu added a commit to danluu/BitFunnel that referenced this issue Jul 14, 2016
@danluu
Remove C-style casts. See BitFunnel#37.
eb9f9d1 
Enable -Wold-style-cast and also remove all existing C-style casts. Doing this
found multiple places where const was incorrectly being cast away, as well as
some pointless casting of size_t down to int.
@danluu danluu added the c++ label Aug 20, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
c++
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@danluu @hausdorff