clang-tidy: fix explicit virtual functions in remaining headers #3063
clang-tidy: fix explicit virtual functions in remaining headers #3063sakirr05 wants to merge 2 commits intopgRouting:developfrom
Conversation
WalkthroughThis PR enables the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 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 |
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Fix all issues with AI agents
In `@include/dagShortestPath/dagShortestPath.hpp`:
- Around line 199-200: The template member function examine_vertex(V u, B_G &)
in class inheriting from boost::default_dijkstra_visitor should not use the C++
override specifier because override cannot be applied to function templates when
matching a non-template virtual base; remove the trailing "override" from the
examine_vertex template declaration so the method compiles and still provides
the intended visitor behavior with boost::default_dijkstra_visitor.
In `@include/visitors/astar_visitors.hpp`:
- Around line 45-46: The member function template examine_vertex (template
<class B_G> void examine_vertex(V u, B_G &g)) in astar_visitors.hpp incorrectly
uses the override specifier even though
boost::default_astar_visitor::examine_vertex is a template and not a virtual
function; remove the trailing "override" from the examine_vertex
declaration/definition in the astar visitor class (same fix as you applied in
prim_dijkstra_visitor.hpp) so the code compiles.
In `@include/visitors/dfs_visitor_with_root.hpp`:
- Around line 50-55: The template member functions tree_edge(E e, const B_G&)
and start_vertex(V v, const B_G&) in dfs_visitor_with_root.hpp are incorrectly
declared with the override specifier; remove the override keyword from both
method declarations (the ones that push to m_data and check v != m_roots) so the
class compiles as a Boost graph visitor using static (template) dispatch.
In `@include/visitors/dfs_visitor.hpp`:
- Around line 61-62: The three member function templates start_vertex,
examine_edge, and tree_edge in your dfs visitor class are incorrectly marked
with override (these are templates matching boost::default_dfs_visitor
non-virtual template methods), so remove the trailing "override" from the
template method declarations/definitions (e.g., the template<typename B_G> void
start_vertex(V v, const B_G&), template<typename B_G> void examine_edge(E e,
const B_G&), and template<typename B_G> void tree_edge(E e, const B_G&) in your
dfs_visitor class) so they compile without attempting to override non-virtual
base methods.
In `@include/visitors/dijkstra_visitors.hpp`:
- Line 93: Remove the invalid override specifiers from the template callback
methods in the Dijkstra visitor classes: delete the `override` keyword from the
template member functions `examine_vertex`, `examine_edge`, `edge_not_relaxed`,
and `discover_vertex` where they appear in the classes
`dijkstra_many_goal_visitor`, `dijkstra_distance_visitor`, and
`dijkstra_distance_visitor_no_init` (these are the non-virtual template methods
intended to satisfy Boost's visitor concept); leave other non-template visitors
(e.g. `dijkstra_one_goal_visitor`, `dijkstra_max_distance_visitor`) unchanged.
In `@include/visitors/edges_order_bfs_visitor.hpp`:
- Around line 46-47: The member function template declaration "template <class
B_G> void tree_edge(E e, const B_G&)" in edges_order_bfs_visitor.hpp cannot use
the 'override' specifier against boost::default_bfs_visitor (a non-template base
method), so remove the 'override' token from the tree_edge declaration; locate
the template<class B_G> void tree_edge(...) definition and delete 'override' so
the method compiles without attempting to override a non-template base member.
In `@include/visitors/edges_order_dfs_visitor.hpp`:
- Around line 47-48: The member function template tree_edge(E e, const B_G&) in
the visitor class is marked with override but cannot override a template from
the boost::default_dfs_visitor base; remove the trailing "override" specifier
from the tree_edge template declaration (and any other member function templates
in the same class that use "override") so the compiler stops reporting the
invalid override against boost::default_dfs_visitor.
In `@include/visitors/prim_dijkstra_visitor.hpp`:
- Around line 47-48: The finish_vertex member function template in the
prim_dijkstra_visitor class (the declaration "template<class B_G> void
finish_vertex(V v, B_G&) override") is ill-formed because member function
templates cannot be virtual and thus cannot use the override specifier; remove
the trailing "override" from the finish_vertex template declaration so it
becomes a regular template method (locate the finish_vertex template in
prim_dijkstra_visitor and delete the "override" keyword).
Boost graph visitors use non-virtual template callbacks, so `override` is not valid on these member functions. Remove `override` where the base class does not declare virtual methods. Fixes pgRouting#3045
5998180 to
5d455e1
Compare
|
Hi @cvvergara, this PR finishes the remaining work for #3045 under #3043. I enabled cppcoreguidelines-explicit-virtual-functions and added override only where the base class actually defines virtual methods. For Boost graph visitors, override was removed since those callbacks are non-virtual templates. The changes are header-only, mechanical, and don’t affect behavior or APIs; assert.hpp was already handled in #3044. Would appreciate a review when you get a chance |
Fixes: #3045
Summary
This PR enables the
cppcoreguidelines-explicit-virtual-functionscheck and fixes the remaining violations by explicitly marking overridden virtual functions withoverride.The changes are limited to C++ header files and are purely mechanical.
Details
cppcoreguidelines-explicit-virtual-functionsin.clang-tidyoverrideto virtual function overrides in remaining include headersassert.hppis intentionally excluded (handled separately in Check and fix assert.hpp for cppcoreguidelines-explicit-virtual-functions #3044)Example
Before:
After:
Summary by CodeRabbit