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

Fix implicit fallthrough warning and unused variable warning in GCC #1984

Merged
merged 7 commits into from Aug 2, 2019

Conversation

@jackgerrits
Copy link
Member

commented Jul 31, 2019

No description provided.

jackgerrits added some commits Jul 31, 2019

@jackgerrits jackgerrits requested a review from peterychang Jul 31, 2019

vowpalwabbit/gen_cs_example.h Show resolved Hide resolved
case 2: k1 ^= tail[1] << 8;
case 3:
k1 ^= tail[2] << 16;
// fall through

This comment has been minimized.

Copy link
@lokitoth

lokitoth Aug 1, 2019

Member

Does the presence of the comment suppress the warning, or does the command have to be exactly // fall through? If not the latter, could you add a note about this suppressing the warning, so it does not get removed later?

This comment has been minimized.

Copy link
@jackgerrits

jackgerrits Aug 1, 2019

Author Member

Yep! GCC see's this as an annotation and silences the warning. There is also a gnu specific attribute available. In C++17 a [[fallthrough]] attribute was added for this exact purpose.

I figured a comment is simple and suffices for now. I'll add a description so we don't accidentally remove it.

jackgerrits added some commits Aug 1, 2019

@jackgerrits jackgerrits merged commit 36e44bc into VowpalWabbit:master Aug 2, 2019

10 of 11 checks passed

MacOS CI Build #20190801.25 failed
Details
LGTM analysis: C# No code changes detected
Details
LGTM analysis: C/C++ No new or fixed alerts
Details
LGTM analysis: Java No code changes detected
Details
LGTM analysis: JavaScript No code changes detected
Details
LGTM analysis: Python No code changes detected
Details
Linux CI Build #20190801.9 succeeded
Details
Windows CI Build #20190801.25 succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.003%) to 73.019%
Details

@jackgerrits jackgerrits deleted the jackgerrits:jagerrit/gcc_warnings branch Aug 6, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.