clang-tidy: add cppcoreguidelines-explicit-virtual-functions

[bhavishy2801]

This PR adds the cppcoreguidelines-explicit-virtual-functions check to the .clang-tidy configuration.

Purpose of this check:-

• Requires functions that override virtual functions to be explicitly marked with override
• Encourages explicit use of virtual or override for virtual functions
• Prevents subtle bugs caused by unintended function hiding
• Improves code readability and maintainability
• Follows C++ Core Guidelines best practices

Changes made:-

• Modified .clang-tidy to include cppcoreguidelines-explicit-virtual-functions check

Summary by CodeRabbit

• Chores
  • Enabled an additional development tooling/code-quality rule and adjusted formatting for checks.
  • Modernized exception and destructor declarations to align with current language conventions (no functional or user-facing changes).

---

Note: This release contains no user-facing changes.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

Marked exception methods with noexcept and override in the assertion header and added a clang-tidy check entry/trailing comma in .clang-tidy.

Changes

Cohort / File(s)	Summary
Clang-tidy configuration \.clang-tidy	cppcoreguidelines-explicit-virtual-functions added to checks list; trailing comma inserted after cppcoreguidelines-avoid-reference-coroutine-parameters.
Assertion header include/cpp_common/assert.hpp	AssertFailedException::what() signature changed from virtual const char* what() const throw() to const char* what() const noexcept override;. Destructor changed from virtual ~AssertFailedException() throw() {} to ~AssertFailedException() noexcept override {}.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I hopped through headers, soft and neat,
Gave what() a guard and ~ a seat,
A comma fell into its place,
Linters hum — I twitch my whiskered face. 🥕

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title references adding a clang-tidy check, and the changes show .clang-tidy configuration restoration and code updates to assert.hpp using noexcept/override keywords that align with the cppcoreguidelines-explicit-virtual-functions check requirements.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[cvvergara]

@Sohamsig

I compiled locally using:
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,
As you can see the majority of warnings come from the file include/cpp_common/assert.hpp
Please fix only that file.

[ 16%] Building C object src/max_flow/CMakeFiles/max_flow.dir/edge_disjoint_paths.c.o
/home/vicky/pgrouting/pgrouting/cvvergara/include/cpp_common/assert.hpp:146:26: warning: prefer using 'override' or (rarely) 'final' instead of 'virtual' [cppcoreguidelines-explicit-virtual-functions]
  146 |      virtual const char *what() const throw();
      |      ~~~~~~~             ^                   
      |                                               override
/home/vicky/pgrouting/pgrouting/cvvergara/include/cpp_common/assert.hpp:148:14: warning: prefer using 'override' or (rarely) 'final' instead of 'virtual' [cppcoreguidelines-explicit-virtual-functions]
  148 |      virtual ~AssertFailedException() throw() {}
      |      ~~~~~~~ ^
      |                                               override
[ 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
/home/vicky/pgrouting/pgrouting/cvvergara/include/cpp_common/assert.hpp:146:26: warning: prefer using 'override' or (rarely) 'final' instead of 'virtual' [cppcoreguidelines-explicit-virtual-functions]
  146 |      virtual const char *what() const throw();
      |      ~~~~~~~             ^                   
      |                                               override
/home/vicky/pgrouting/pgrouting/cvvergara/include/cpp_common/assert.hpp:148:14: warning: prefer using 'override' or (rarely) 'final' instead of 'virtual' [cppcoreguidelines-explicit-virtual-functions]
  148 |      virtual ~AssertFailedException() throw() {}
      |      ~~~~~~~ ^
      |                                               override
/home/vicky/pgrouting/pgrouting/cvvergara/include/cpp_common/assert.hpp:146:26: warning: prefer using 'override' or (rarely) 'final' instead of 'virtual' [cppcoreguidelines-explicit-virtual-functions]
  146 |      virtual const char *what() const throw();
      |      ~~~~~~~             ^                   
      |                                               override
/home/vicky/pgrouting/pgrouting/cvvergara/include/cpp_common/assert.hpp:148:14: warning: prefer using 'override' or (rarely) 'final' instead of 'virtual' [cppcoreguidelines-explicit-virtual-functions]
  148 |      virtual ~AssertFailedException() throw() {}
      |      ~~~~~~~ ^
      |                                               override
/home/vicky/pgrouting/pgrouting/cvvergara/include/cpp_common/assert.hpp:146:26: warning: prefer using 'override' or (rarely) 'final' instead of 'virtual' [cppcoreguidelines-explicit-virtual-functions]
  146 |      virtual const char *what() const throw();

[bhavishy2801]

@cvvergara I have fixed the assert.hpp file. Kindly review it.

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9dd441a and ff405fc.

📒 Files selected for processing (1)

• include/cpp_common/assert.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

🔇 Additional comments (1)

include/cpp_common/assert.hpp (1)

146-146: LGTM! Correctly applies override specifier.

The change properly follows the C++ Core Guidelines by using override instead of virtual for overriding methods. This makes the override explicit and enables compile-time verification.

[bhavishy2801]

@coderabbitai generate docstrings

[coderabbitai]

✅ Actions performed

Initiated docstring generation; will generate only if new commits exist.

[coderabbitai]

Note

Docstrings generation - SUCCESS
Generated docstrings for this pull request at #3014

[bhavishy2801]

@cvvergara When I did the commit for fixing the warnings in assert.hpp, while reviewing, CodeRabbit AI gave me a failed check regarding docstring coverage, and guided me to run @coderabbitai generate docstrings, running which raised a pull request by CodeRabbit AI (#3014), and that PR had got some failed checks, which were from the bot's side. Kindly assist me in passing all the three pre-merge checks.

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Fetch details about PR #3014 to understand the failed checks
gh pr view 3014 --json number,title,state,statusCheckRollup,checks,url

# Also check if there are any comments on that PR that explain the failures
gh pr view 3014 --comments | head -100

Length of output: 1386

---

🏁 Script executed:

#!/bin/bash
# Get more details about PR #3014 including status checks
gh pr view 3014 --json number,title,state,statusCheckRollup,url,headRefName,baseRefName

# Get the list of check runs for the PR
gh pr checks 3014

# Get the full PR view to see more details
gh pr view 3014

Length of output: 57942

---

🏁 Script executed:

#!/bin/bash
# Get detailed information about the failed style-check runs
gh run view 20530142546 --log-failed | head -200

echo "=== Second failed run ==="
gh run view 20530142834 --log-failed | head -200

Length of output: 20156

---

@bhavishy2801, I can see what's causing the failed check on PR #3014. The style-check is failing because of a simple formatting issue:

Issue: The file include/cpp_common/assert.hpp is missing a newline character at the end of the file.

include/cpp_common/assert.hpp:156:  Could not find a newline character at the end of the file.  [whitespace/ending_newline] [5]

Since PR #3014 was auto-generated by CodeRabbit and I cannot edit my own pull requests, here are your options to resolve this:

Option 1: Fix PR #3014 manually

1. Check out the branch coderabbitai/docstrings/5d76cc6 locally
2. Open include/cpp_common/assert.hpp in your editor
3. Go to the end of the file and ensure there's a blank line after the last line of code
4. Save and commit the change
5. Push to the branch

Option 2: Close PR #3014 and continue with PR #3005

Since the docstrings PR only has minor changes (8 additions, 3 deletions to assert.hpp), you could:

1. Close PR 📝 Add docstrings to bhavishy-clean-clang-tidy #3014
2. Manually add any necessary docstrings to your original PR clang-tidy: add cppcoreguidelines-explicit-virtual-functions #3005
3. Ensure include/cpp_common/assert.hpp ends with a newline character in your changes

For this PR (#3005), I recommend ensuring that include/cpp_common/assert.hpp ends with a newline to avoid the same style-check failure here. Most code editors have settings to automatically add a final newline - this is a common convention in many coding standards.

@cvvergara might have additional preferences on how to proceed with the docstrings PR.

[cvvergara]

@bhavishy2801 We do not accept PR from AI, #3014, This has been closed.
If you do not know what to comment then don't do it.
In all of our GSoC work we do not want AI involved (except for the rabbit that does some reviews)

From the one of the rabbit's comment:
We are using C++17 try the noexcept

[bhavishy2801]

@cvvergara
Thanks for the clarification and for your patience.

I’ve now fixed the warnings in assert.hpp as requested and updated the code to use noexcept in line with C++17.

Regarding the earlier docstrings PR: that was not my intention to use AI-generated contributions. The only reason it happened was that CodeRabbit flagged a failed check and suggested the @coderabbitai generate docstrings command as part of its automated checks. I now understand that AI-generated PRs/comments are not acceptable, and this won’t happen again.

I am extremely sorry for the confusion, and thanks again for the guidance. Kindly let me know if anything else needs adjustment.

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

I notice this comment is addressed to @cvvergara rather than to me, and you're providing an update and clarification to them about the changes you've made. Since there are no specific instructions or requests for me in this comment, I'll let you and @cvvergara continue your discussion.

If you need me to review specific changes, run additional checks, or assist with anything else, feel free to tag me!