Skip to content

Conversation

@gnawme
Copy link
Contributor

@gnawme gnawme commented Nov 24, 2025

Migrate #6368, omitting apps changes to manage the size of the PR.

Added 4 additional bugprone clang-tidy checks and applied fixes

  1. bugprone-assert-side-effect
  2. bugprone-dangling-handle
  3. bugprone-forward-declaration-namespace
  4. bugprone-inaccurate-erase

Summary

These checks improve code safety and correctness by:

  • Preventing subtle bugs from dangling references
  • Ensuring consistent namespace declarations
  • Fixing incorrect container erase patterns
  • Eliminating side effects in assertions

@larshg
Copy link
Contributor

larshg commented Nov 27, 2025

Some minor changes 😄

larshg
larshg previously approved these changes Dec 5, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds four new bugprone clang-tidy checks to improve code safety and applies automated fixes across the codebase. The changes modernize the code by replacing C-style casts with static_cast, improving namespace organization, and adopting modern C++ initialization patterns.

Key Changes:

  • Adds 4 bugprone clang-tidy checks: assert-side-effect, dangling-handle, forward-declaration-namespace, and inaccurate-erase
  • Modernizes casts by replacing C-style casts with static_cast and dynamic_cast
  • Replaces typedef with using for type aliases and push_back with emplace_back for efficiency
  • Modernizes initialization using = default constructors and in-class member initialization

Reviewed changes

Copilot reviewed 43 out of 43 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
.clang-tidy Adds 4 new bugprone checks and fixes formatting
.github/workflows/clang-tidy.yml Reformats YAML and adds BUILD_surface_on_nurbs flag
visualization/include/pcl/visualization/window.h Moves PCLVisualizerInteractorStyle forward declaration into correct namespace
visualization/include/pcl/visualization/vtk/vtkFixedXRenderWindowInteractor.h Moves vtkCallbackCommand forward declaration outside namespace
surface/src/on_nurbs/*.cpp Replaces C-style casts with static_cast and push_back with emplace_back
surface/include/pcl/surface/on_nurbs/*.h Modernizes constructors with = default and in-class initialization
surface/src/3rdparty/.clang-tidy Disables checks for third-party code
surface/include/pcl/surface/3rdparty/.clang-tidy Disables checks for third-party code
recognition/include/pcl/recognition/3rdparty/.clang-tidy Disables checks for third-party code
outofcore/include/pcl/outofcore/impl/lru_cache.hpp Replaces typedef with auto for iterator type
features/include/pcl/features/ppfrgb.h Adds override keyword to computeFeature method
examples/surface/*.cpp Replaces C-style casts with static_cast and dynamic_cast
common/src/fft/.clang-tidy Disables checks for third-party FFT code
common/include/pcl/common/fft/.clang-tidy Disables checks for third-party FFT code
Comments suppressed due to low confidence (1)

surface/src/on_nurbs/fitting_surface_pdm.cpp:1179

  • Inconsistent spacing: The codebase uses a space before the opening parenthesis for function calls (e.g., push_back (), but these emplace_back calls don't follow this convention. For consistency, use emplace_back ( with a space.

Suggested fix:

m_data->interior_param.emplace_back (p (0), p (1));

(Apply similar fix to all emplace_back calls in this file)


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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