Open jameslamb opened 3 years ago
471 warnings on master
(https://github.com/microsoft/LightGBM/commit/a926d4fe939118b6377455220e44ca9ded5b964e)
I've filtered out only the warnings from the full logs by running the following:
cat cppcheck.txt | grep -E '\[[a-zA-Z]+\]$'
The full logs are much larger. Here's a snippet of the format:
Checking src/application/application.cpp ...
include/LightGBM/train_share_states.h:159:3: warning: Member variable 'TrainingShareStates::bagging_use_indices' is not initialized in the constructor. [uninitMemberVar]
TrainingShareStates() {
^
include/LightGBM/train_share_states.h:159:3: warning: Member variable 'TrainingShareStates::bagging_indices_cnt' is not initialized in the constructor. [uninitMemberVar]
TrainingShareStates() {
^
include/LightGBM/feature_group.h:83:5: performance: Variable 'bin_offsets_' is assigned in constructor body. Consider performing initialization in initialization list. [useInitializationList]
bin_offsets_ = other.bin_offsets_;
^
include/LightGBM/feature_group.h:454:5: performance: Variable 'bin_offsets_' is assigned in constructor body. Consider performing initialization in initialization list. [useInitializationList]
bin_offsets_ = other.bin_offsets_;
^
src/application/predictor.hpp:43:5: performance: Variable 'early_stop_' is assigned in constructor body. Consider performing initialization in initialization list. [useInitializationList]
early_stop_ = CreatePredictionEarlyStopInstance(
^
include/LightGBM/dataset.h:291:19: style: Class 'Dataset' has a constructor with 1 argument that is not explicit. [noExplicitConstructor]
LIGHTGBM_EXPORT Dataset(data_size_t num_data);
^
include/LightGBM/utils/common.h:904:10: style: Class 'AlignmentAllocator < double , kAlignedSize >' has a constructor with 1 argument that is not explicit. [noExplicitConstructor]
inline AlignmentAllocator(const AlignmentAllocator<T2, N>&) throw() {}
^
include/LightGBM/utils/common.h:667:42: style: Condition 'num_threads<=1' is always true [knownConditionTrueFalse]
if (len <= kMinInnerLen || num_threads <= 1) {
384 warnings on master
(https://github.com/microsoft/LightGBM/commit/fa4ecf4c4da57b1889e39c872eb5449080f1e02e)
Filtered down the warnings using the (updated) steps in this issue's description.
@shiyu1994 can you fix above warnings when have time?
Sure. I can fix these step by step.
I am happy to continue helping with this as well.
Summary
Warnings caught by
cppcheck
should be resolved, if they are determined to be genuine issues and not false positives.Motivation
cppcheck
is a static analyzer for C++ code.This tool can catch some classes of issues that are not caught by other linters or tests with sanitizers, and it does that using a lightweight approach that doesn't require compiling an instrumented version of the library.
See https://sourceforge.net/p/cppcheck/wiki/ListOfChecks/ for a full list of checks.
Description
Similar to the approach taken in LightGBM with
cpplint
(#1990) andmypy
(#3867), for a time these checks might be run in continuous integration without blocking merges, just as extra information about the state of the code.See http://cppcheck.sourceforge.net/ for instructions on how to install
cppcheck
on different operating systems.LightGBM is a large, complex project and its codebase contains many possible combinations of
#ifdef
conditions. Each additional#ifdef
adds another combination of things to try, which can slow down checking substantially and have a big impact on the amount of logs produced.To ensure that
cppcheck
does not take too long and that you can see all of the logs after it's done, I recommend doing the following:git submodulue init && git submodule update --recursive
-U
to limit the number of combinations searched (you can always selectively remove these-U
checks to do more thorough checks!)To generate a list of just lines with warnings, run the following:
Piping that to
wc -l
will give you the number of warnings left.How to Contribute
Look at the current list of warnings by running
cppcheck
yourself locally (following the steps in the "Description" section above).Small pull requests focused on a single part of the codebase or type of
cppcheck
warning are greatly appreciated. Please do not submit a pull request that attempts to fix many of thecppcheck
warnings all at once.When submitting a pull request, include at least the following in the description:
References