-
-
Notifications
You must be signed in to change notification settings - Fork 657
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
COMP: Fix ununsed code ( if(false) ) #4674
COMP: Fix ununsed code ( if(false) ) #4674
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This pattern is used to be able to quickly change the code to test a feature.
The proper fix here would be to expose a new command-line parameter and use it in the if clause. Bonus points if that new command line parameter is exercised in corresponding test specification, e.g. https://github.com/InsightSoftwareConsortium/ITK/blob/v5.4rc03/Modules/Registration/Metricsv4/test/CMakeLists.txt#L113-L123.
1a48219
to
85e46d2
Compare
For the last commit, we would like to know what better name would it be to replace foo. Here, foo represents the variable false. |
...gistration/Metricsv4/test/itkJointHistogramMutualInformationImageToImageRegistrationTest.cxx
Outdated
Show resolved
Hide resolved
Modules/Registration/Metricsv4/test/itkMultiGradientImageToImageMetricv4RegistrationTest.cxx
Outdated
Show resolved
Hide resolved
Modules/Registration/Metricsv4/test/itkMultiGradientImageToImageMetricv4RegistrationTest.cxx
Outdated
Show resolved
Hide resolved
85e46d2
to
ca85ce5
Compare
@@ -164,6 +164,10 @@ itkJointHistogramMutualInformationImageToImageRegistrationTest(int argc, char * | |||
{ | |||
numberOfDisplacementIterations = std::stoi(argv[5]); | |||
} | |||
if (argc >= 7) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You need to declare the variables, e.g.
bool useScalesEstimator = true;
if (argc >= 7)
...
This is a comment to fix the -Wunreachable-code warning generated by clang. This fix simply removes if (false) clauses. An investigation was done to see why these clause exist. This change was done 10 years ago. The clause at that time was if(0).
ca85ce5
to
ba12c7d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@andrei-sandor thank you!
This is a comment to fix the -Wunreachable-code warning generated by clang.
This fix simply removes if (false) clauses. An investigation was done to see why these clause exist. This change was done 10 years ago. The clauses at that time were if(0).