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

Resolve cppcheck errors by adding explicit constructors and consts #291

Merged
merged 3 commits into from
Dec 2, 2021

Conversation

jwnimmer-tri
Copy link
Contributor

@jwnimmer-tri jwnimmer-tri commented Nov 24, 2021

🦟 Bug fix

Fixes cppcheck complaints.

Summary

At least for me on the head of the main branch, I see these errors:

jwnimmer@call-cps:~/jwnimmer-tri/build-ign-math/ign-math$ make codecheck

ign-math/include/ignition/math/Plane.hh:66:15: style: Class 'Plane' has a constructor with 1 argument that is not explicit. [noExplicitConstructor]
      public: Plane(const Vector3<T> &_normal, T _offset = 0.0)
              ^
ign-math/include/ignition/math/OrientedBox.hh:142:35: style: Parameter '_pose' can be declared with const [constParameter]
      public: void Pose(Pose3<T> &_pose)
                                  ^
ign-math/include/ignition/math/SemanticVersion.hh:66:15: style: Class 'SemanticVersion' has a constructor with 1 argument that is not explicit. [noExplicitConstructor]
      public: SemanticVersion(const unsigned int _major,
              ^
ign-math/include/ignition/math/graph/Edge.hh:50:5: style: Struct 'EdgeInitializer' has a constructor with 1 argument that is not explicit. [noExplicitConstructor]
    EdgeInitializer(const VertexId_P &_vertices,
    ^
ign-math/include/ignition/math/graph/Vertex.hh:63:13: style: Class 'Vertex' has a constructor with 1 argument that is not explicit. [noExplicitConstructor]
    public: Vertex(const std::string &_name,
            ^
ign-math/include/ignition/math/graph/GraphAlgorithms.hh:343:16: style: Consider using std::transform algorithm instead of a raw loop. [useStlAlgorithm]
      vertices.push_back(vPair.second.get());
               ^
ign-math/src/Frustum.cc:159:0: style: Consider using std::any_of algorithm instead of a raw loop. [useStlAlgorithm]
    if (plane.Side(_p) == Planed::NEGATIVE_SIDE)
^
ign-math/src/Kmeans_TEST.cc:51:10: style: Consider using std::transform algorithm instead of a raw loop. [useStlAlgorithm]
    elem += math::Vector3d(0.1, 0.2, 0.0);
         ^
ign-math/src/Temperature_TEST.cc:101:0: information: Unmatched suppression: knownConditionTrueFalse [unmatchedSuppression]
  EXPECT_TRUE(temp == temp2);
^
Built target cppcheck

This PR fixes all except the last one.

Checklist

  • Signed all commits for DCO
  • Added tests (N/A)
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge

@jwnimmer-tri jwnimmer-tri marked this pull request as draft November 24, 2021 01:18
Also add some suppressions.

Signed-off-by: Jeremy Nimmer <[email protected]>
@mjcarroll
Copy link
Contributor

I think for the last one, the suppression comment can just be dropped.

@jwnimmer-tri
Copy link
Contributor Author

I think for the last one, the suppression comment can just be dropped.

I've removed that line now. I had been wondering if I was running cppcheck incorrectly (or a different version, or something) where my results did not match with anyone else.

@mjcarroll mjcarroll merged commit efd9c9f into gazebosim:main Dec 2, 2021
@jwnimmer-tri jwnimmer-tri deleted the cppcheck branch December 2, 2021 16:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants