clang-tiddy: add cppcoreguidelines-interfaces-global-init#3004
clang-tiddy: add cppcoreguidelines-interfaces-global-init#3004akronim26 wants to merge 9 commits intopgRouting:developfrom
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughUpdated Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
cvvergara
left a comment
There was a problem hiding this comment.
I compiled locally using on my local run.sh:
CXX=clang++ CC=clang cmake -DUSE_CLANG_TIDY=ON -DBUILD_HTML=OFF -DPOSTGRESQL_BIN=${PGBIN} ..
and these are some of the many warnings I get that are not fixed
/home/vicky/pgrouting/pgrouting/cvvergara/include/trsp/edgeInfo.hpp:39:7: warning: constructor does not initialize these fields: m_edge, m_edgeIndex [cppcoreguidelines-pro-type-member-init]
39 | class EdgeInfo {
| ^
[ 16%] Building CXX object src/max_flow/CMakeFiles/max_flow.dir/max_flow_driver.cpp.o
[ 16%] Building CXX object src/trsp/CMakeFiles/trsp.dir/trspHandler.cpp.o
[ 16%] Building CXX object src/allpairs/CMakeFiles/allpairs.dir/allpairs_driver.cpp.o
[ 17%] Building CXX object src/withPoints/CMakeFiles/withPoints.dir/withPoints.cpp.o
[ 17%] Building C object src/trsp/CMakeFiles/trsp.dir/trsp.c.o
[ 17%] Building C object src/withPoints/CMakeFiles/withPoints.dir/withPointsVia.c.o
[ 18%] Building CXX object src/max_flow/CMakeFiles/max_flow.dir/maximum_cardinality_matching_driver.cpp.o
[ 18%] Building CXX object src/pickDeliver/CMakeFiles/pickDeliver.dir/pickDeliver.cpp.o
[ 18%] Building C object src/ksp/CMakeFiles/ksp.dir/withPoints_ksp.c.o
[ 18%] Building CXX object src/driving_distance/CMakeFiles/driving_distance.dir/driving_distance_withPoints_driver.cpp.o
/home/vicky/pgrouting/pgrouting/cvvergara/include/cpp_common/ch_edge.hpp:42:7: warning: constructor does not initialize these fields: id, source, target, cost [cppcoreguidelines-pro-type-member-init]
42 | class CH_edge {
| ^
/home/vicky/pgrouting/pgrouting/cvvergara/include/cpp_common/ch_edge.hpp:44:6: warning: constructor does not initialize these fields: id, source, target, cost [cppcoreguidelines-pro-type-member-init]
44 | CH_edge() = default;
| ^
/home/vicky/pgrouting/pgrouting/cvvergara/include/cpp_common/path.hpp:166:40: warning: constructor does not initialize these fields: m_tot_cost [cppcoreguidelines-pro-type-member-init]
62 | template <typename G , typename V> Path(
| ^
/home/vicky/pgrouting/pgrouting/cvvergara/include/cpp_common/path.hpp:231:40: warning: constructor does not initialize these fields: m_tot_cost [cppcoreguidelines-pro-type-member-init]
231 | template <typename G , typename V> Path(
The warnings come from cppcoreguidelines-pro-type-member-init
From my calculations:
grep cppcore out.txt | wc -l
162
There are 162 warnings.
cvvergara
left a comment
There was a problem hiding this comment.
Please fix the warnings only in file include/cpp_common/path.hpp
and then click on Ready for Review button so that I can review again for that file.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
include/cpp_common/path.hpp
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Codacy Static Code Analysis
cvvergara
left a comment
There was a problem hiding this comment.
Hi, thanks,
The modifications on the requested file look good
I am waiting for tests to finish running, but looks fine
Now please remove the requested line because leaving it will create cppcoreguidelines-pro-type-member-init warnings when compiling the code and we know there are more to fix
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
.clang-tidy
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Codacy Static Code Analysis
| cppcoreguidelines-avoid-goto, | ||
| cppcoreguidelines-avoid-non-const-global-variables, | ||
| cppcoreguidelines-avoid-reference-coroutine-parameters, | ||
| cppcoreguidelines-interfaces-global-init, |
cvvergara
left a comment
There was a problem hiding this comment.
Please when you do modifications look at the sorrounding code to see if something else needs to be modified
Summary by CodeRabbit
Chores
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.