Pass include paths to cppcheck by jacobperron · Pull Request #117 · ament/ament_lint

jacobperron · 2018-12-18T23:55:57Z

Specifically, passing the parent directories of all source files since a typical include
for a C/C++ header are located in a subdirectory named after the package or library.

Resolves #116

Test CI with cppcheck v1.86 (up to rclcpp):

ament_cppcheck/ament_cppcheck/main.py

ament_cmake_cppcheck/cmake/ament_cmake_cppcheck_lint_hook.cmake

jacobperron · 2018-12-21T22:43:07Z

Linux
Linux-aarch64
macOS
Windows

And CI machine with new version of cppcheck (v1.86)

macOS

jacobperron · 2018-12-22T02:03:43Z

With lint fixes:

Linux
Linux-aarch64
macOS
Windows

cppcheck v1.86:

macOS

Specifically, passing the parent directories of all source files since a typical include for a C/C++ header are located in a subdirectory named after the package or library.

Co-Authored-By: jacobperron <jacob@openrobotics.org>

…directories

jacobperron · 2019-01-07T19:48:16Z

I've updated the PR to use BUILDSYSTEM_TARGETS if cmake version is >= 3.7. We're not experiencing cppcheck issues for systems with older cmake versions, like Xenial, so not passing include directories (as before) should remain okay.
@dirk-thomas Please take another look.

ament_cmake_cppcheck/cmake/ament_cmake_cppcheck_lint_hook.cmake

ament_cppcheck/ament_cppcheck/main.py

ament_cmake_cppcheck/cmake/ament_cppcheck.cmake

cppcheck can take significantly longer to execute when passing include directories.

jacobperron · 2019-01-07T23:40:06Z

A couple issues I've identified with this approach:

Passing include directories to cppcheck significantly increases the time it takes to run.
It doesn't seem like 120s is enough in some situations. Likely need to bump the timeout >200s (maybe 300s if we can live with it).
Errors are reported from included headers from outside the package being tested.
I have a hack to handle this in e67e564.

Thoughts?

…being tested This eliminates the need for a longer test timeout and avoids cppcheck from testing external files. Reverted prior changes accordingly.

jacobperron · 2019-01-08T02:31:16Z

Attempt to resolve timeout issue and errors from external packages by only including headers from the package being tested.

jacobperron · 2019-01-09T00:37:48Z

Ready for another round of review.

Passing include directories to cppcheck uncovered a potential issue in rcl (fixed in ros2/rcl#371). It's possible there will be more uncovered issues.

Linux
Linux-aarch64
macOS
Windows

jacobperron · 2019-01-09T17:37:32Z

CI is good. The one cppcheck error is handled by ros2/rcl#371.

ament_cmake_cppcheck/cmake/ament_cmake_cppcheck_lint_hook.cmake

ament_cppcheck/ament_cppcheck/main.py

ament_cmake_cppcheck/cmake/ament_cmake_cppcheck_lint_hook.cmake

jacobperron · 2019-01-09T23:51:44Z

CI (linter tests only):

Linux
Linux-aarch64
macOS
Windows

sloretz · 2019-03-01T17:29:03Z

ament_cmake_cppcheck/cmake/ament_cmake_cppcheck_lint_hook.cmake

+        PROPERTY INCLUDE_DIRECTORIES
+      )
+
+      # Only append include directories that are from the package being tested


I'm seeing a similar cppcheck complaint in rmw_connext and rmw_opensplice where they use a macro RMW_CHECK_TYPE_IDENTIFIERS_MATCH that is defined rmw.

https://ci.ros2.org/view/nightly/job/nightly_osx_extra_rmw_release/313/testReport/junit/rmw_opensplice_cpp/cppcheck/error__unknownMacro__src_rmw_wait_cpp_80_/

https://ci.ros2.org/job/nightly_osx_debug/1175/testReport/junit/rmw_connext_shared_cpp/cppcheck/error__unknownMacro___Users_osrf_jenkins_agent_workspace_nightly_osx_debug_ws_src_ros2_rmw_connext_rmw_connext_shared_cpp_include_rmw_connext_shared_cpp_wait_hpp_51_/

Any way to tell cppcheck where to look for macros without checking external files?

@sloretz See #125.

You can pass cppcheck one or more include directories that contain the macros it is trying to resolve.
I'm not satisfied with the solution, but I think anything better would require changes in cppcheck itself.

thanks, I missed that PR

ament_cppcheck/ament_cppcheck/main.py

dirk-thomas · 2020-06-17T21:50:53Z

ament_cmake_cppcheck/cmake/ament_cmake_cppcheck_lint_hook.cmake

+      # Only append include directories that are from the package being tested
+      # This accomplishes two things:
+      #     1. Reduces execution time (less include directories to search)
+      #     2. cppcheck will not check for errors in external packages


This fails on Windows in this case: ros2/ros2#942.

I don't understand the case that's failing. From the referenced ticket, it looks like the failures are due to headers from external packages not being passed to cppcheck (which is the intended behavior). For this case, we have ament_cmake_cppcheck_ADDITIONAL_INCLUDE_DIRS that can be set (introduced in #125).

Without the proper include directories cppcheck warns for unknown macros.

ros2/rclpy#577 sets an additional include dir.

jacobperron added the in progress Actively being worked on (Kanban column) label Dec 18, 2018

jacobperron mentioned this pull request Dec 19, 2018

Update to Uncrustify v0.68.1 ament/uncrustify_vendor#3

Merged

jacobperron added in review Waiting for review (Kanban column) and removed in progress Actively being worked on (Kanban column) labels Dec 19, 2018

tfoote reviewed Dec 19, 2018

View reviewed changes

ament_cppcheck/ament_cppcheck/main.py Outdated Show resolved Hide resolved

dirk-thomas reviewed Dec 20, 2018

View reviewed changes

ament_cmake_cppcheck/cmake/ament_cmake_cppcheck_lint_hook.cmake Outdated Show resolved Hide resolved

jacobperron and others added 6 commits January 3, 2019 13:18

Pass include paths to cppcheck

6e187e7

Specifically, passing the parent directories of all source files since a typical include for a C/C++ header are located in a subdirectory named after the package or library.

Fix syntax bugs

f46461d

Update ament_cppcheck/ament_cppcheck/main.py

ff52dd1

Co-Authored-By: jacobperron <jacob@openrobotics.org>

Replace parent directory heuristic with explicit call to get include …

d8fe783

…directories

lint

5a301a0

Use BUILDSYSTEM_TARGETS list for getting include directories

c3d0e8d

jacobperron force-pushed the pass_include_cppcheck branch from fce39dd to c3d0e8d Compare January 7, 2019 19:40

clalancette mentioned this pull request Jan 7, 2019

ROS 2 Crystal Clemmys Patch Release 1 ros2/ros2#638

Closed

nipick: case

d75d168

dirk-thomas reviewed Jan 7, 2019

View reviewed changes

jacobperron added 4 commits January 7, 2019 15:10

Address review and use BUILDSYSTEM_TARGETS correctly

fc7f33c

Increase ament_cppcheck test timeout to 120s

4a50e31

cppcheck can take significantly longer to execute when passing include directories.

Further increase timeout to 300s

75ef8c0

Ignore cppcheck errors in included files external to tested package

e67e564

Only pass include directories that are subdirectories of the package …

85fd5b8

…being tested This eliminates the need for a longer test timeout and avoids cppcheck from testing external files. Reverted prior changes accordingly.

Handle case when cppcheck reports error in filename with arbitrary path

cf38eef

dirk-thomas reviewed Jan 9, 2019

View reviewed changes

ament_cmake_cppcheck/cmake/ament_cmake_cppcheck_lint_hook.cmake Outdated Show resolved Hide resolved

ament_cppcheck/ament_cppcheck/main.py Outdated Show resolved Hide resolved

jacobperron added 2 commits January 9, 2019 14:08

Simplify

9eb4af9

Use defaultdict

fedce3e

dirk-thomas reviewed Jan 9, 2019

View reviewed changes

ament_cmake_cppcheck/cmake/ament_cmake_cppcheck_lint_hook.cmake Show resolved Hide resolved

Add find_package and dependency tag for ament_cmake_core

ba7108b

dirk-thomas approved these changes Jan 9, 2019

View reviewed changes

jacobperron merged commit 8d2ef3b into master Jan 10, 2019

jacobperron deleted the pass_include_cppcheck branch January 10, 2019 01:55

jacobperron removed the in review Waiting for review (Kanban column) label Jan 10, 2019

Karsten1987 mentioned this pull request Jan 10, 2019

The property "INCLUDE_DIRECTORIES" is not allowed. #120

Closed

sloretz reviewed Mar 1, 2019

View reviewed changes

This was referenced Mar 1, 2019

Add rmw includes to cppcheck ros2/rmw_connext#348

Merged

Add rmw includes to cppcheck ros2/rmw_opensplice#261

Merged

dirk-thomas mentioned this pull request Mar 22, 2019

fix cppcheck report to list checked files #134

Merged

dirk-thomas reviewed Mar 22, 2019

View reviewed changes

ament_cppcheck/ament_cppcheck/main.py Show resolved Hide resolved

dirk-thomas reviewed Jun 17, 2020

View reviewed changes

dirk-thomas mentioned this pull request Jun 17, 2020

Pass rcutils_include_dirs to cppcheck ros2/rclpy#577

Merged

ivanpauno mentioned this pull request Aug 31, 2020

rmw_cyclonedds GitHub actions broken ros2/rmw_cyclonedds#220

Closed

jacobperron mentioned this pull request Sep 9, 2020

Suppress cppcheck unknownMacro #268

Merged

Conversation

jacobperron commented Dec 18, 2018

Uh oh!

Uh oh!

Uh oh!

jacobperron commented Dec 21, 2018

Uh oh!

jacobperron commented Dec 22, 2018

Uh oh!

jacobperron commented Jan 7, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jacobperron commented Jan 7, 2019

Uh oh!

jacobperron commented Jan 8, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jacobperron commented Jan 9, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jacobperron commented Jan 9, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jacobperron commented Jan 9, 2019

Uh oh!

sloretz Mar 1, 2019

Choose a reason for hiding this comment

Uh oh!

jacobperron Mar 1, 2019

Choose a reason for hiding this comment

Uh oh!

sloretz Mar 1, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dirk-thomas Jun 17, 2020

Choose a reason for hiding this comment

Uh oh!

jacobperron Jun 17, 2020

Choose a reason for hiding this comment

Uh oh!

dirk-thomas Jun 17, 2020

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jacobperron commented Jan 8, 2019 •

edited

Loading

jacobperron commented Jan 9, 2019 •

edited

Loading