Skip to content

STYLE: Remove virtual keyword from member functions marked override - #6861

Merged
hjmjohnson merged 1 commit into
InsightSoftwareConsortium:mainfrom
N-Dekker:Remove-virtual-from-overrides
Sep 15, 2026
Merged

hjmjohnson merged 1 commit into
InsightSoftwareConsortium:mainfrom
N-Dekker:Remove-virtual-from-overrides

Conversation

@N-Dekker

Copy link
Copy Markdown
Contributor

Following C++ Core Guidelines, Virtual functions should specify exactly one of virtual, override, or final

Using Notepad++, Replace in Files, doing:

Find what: virtual (.*[\r\n]*.*override)
Replace with: \1
[v] Match case
(*) Regular expression

@github-actions github-actions Bot added area:Core Issues affecting the Core module area:Registration Issues affecting the Registration module type:Style Style changes: no logic impact (indentation, comments, naming) labels Sep 15, 2026
@dzenanz

dzenanz commented Sep 15, 2026

Copy link
Copy Markdown
Member

Ah, virtual and override on different lines, so previous searches missed them.

Comment thread Modules/Core/Common/include/itkCellInterface.h
@N-Dekker
N-Dekker marked this pull request as ready for review September 15, 2026 15:14
@greptile-apps

greptile-apps Bot commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

  • Removes redundant virtual specifiers from declarations already marked override.
  • Keeps the existing public interfaces and dispatch behavior unchanged.

No new actionable issues were identified.

Confidence Score: 5/5

Safe to merge.

There are no outstanding severity-bearing findings.

Reviews (2): Last reviewed commit: "STYLE: Remove `virtual` keyword from mem..." | Re-trigger Greptile

@N-Dekker
N-Dekker marked this pull request as draft September 15, 2026 15:25
Following C++ Core Guidelines, Jun 14, 2026, "Virtual functions should specify
exactly one of `virtual`, `override`, or `final`",
https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c128-virtual-functions-should-specify-exactly-one-of-virtual-override-or-final

Using Notepad++, Replace in Files, doing:

  Find what: `  virtual (.*[\r\n]*.*override)`
  Replace with: `  \1`
  `[v]` Match case
  `(*)` Regular expression

Manually removed three more `virtual` keywords from
LandmarkRegistrationEstimator, as detected by Greptile AI.
@N-Dekker
N-Dekker force-pushed the Remove-virtual-from-overrides branch from 035a721 to 772c657 Compare September 15, 2026 15:30
@N-Dekker

Copy link
Copy Markdown
Contributor Author
  • Remove the leading virtual keyword from the three residual declarations, leaving override as their sole virtual-function specifier.

Thanks @greptile-apps! Addressed by this force-pushed amend

@N-Dekker
N-Dekker marked this pull request as ready for review September 15, 2026 15:37
@greptile-apps

greptile-apps Bot commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Addressed in the force-pushed amend. The residual virtual specifiers were removed while retaining override in itkCellInterface.h, itkEventObject.h, and itkLandmarkRegistrationEstimator.h.

@hjmjohnson
hjmjohnson merged commit f1e39a5 into InsightSoftwareConsortium:main Sep 15, 2026
15 of 17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:Core Issues affecting the Core module area:Registration Issues affecting the Registration module type:Style Style changes: no logic impact (indentation, comments, naming)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants