Skip to content

fix: unify virtual/override keyword usage in destructors#1763

Open
LHT129 wants to merge 1 commit intoantgroup:mainfrom
LHT129:2026-03-27-统一-virtual-override-关键字使用

Hidden character warning

The head ref may contain hidden characters: "2026-03-27-\u7edf\u4e00-virtual-override-\u5173\u952e\u5b57\u4f7f\u7528"
Open

fix: unify virtual/override keyword usage in destructors#1763
LHT129 wants to merge 1 commit intoantgroup:mainfrom
LHT129:2026-03-27-统一-virtual-override-关键字使用

Conversation

@LHT129
Copy link
Copy Markdown
Collaborator

@LHT129 LHT129 commented Mar 27, 2026

Summary

Unify virtual/override keyword usage in destructors following C++ Core Guidelines C.128.

Changes

  • Add override to destructors in DiskANN, FP32Quantizer, INT8Quantizer, SINDI, Pyramid
  • Remove redundant virtual keyword in MRLETransformer, RandomOrthogonalMatrix

Files Changed

  • src/index/diskann.h
  • src/quantization/fp32_quantizer.h
  • src/quantization/int8_quantizer.h
  • src/algorithm/sindi/sindi.h
  • src/algorithm/pyramid.h
  • src/impl/transform/mrle_transformer.h
  • src/impl/transform/random_orthogonal_transformer.h

Testing

  • Build: make release passed
  • Tests: Functional and unit tests passed

Checklist

  • Code follows VSAG coding style
  • All tests pass
  • PR description is clear

@LHT129 LHT129 requested a review from wxyucs as a code owner March 27, 2026 03:53
Copilot AI review requested due to automatic review settings March 27, 2026 03:53
@LHT129 LHT129 self-assigned this Mar 27, 2026
@LHT129 LHT129 added kind/improvement Code improvements (variable/function renaming, refactoring, etc. ) version/1.0 labels Mar 27, 2026
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request adds the override specifier to the destructors of the Pyramid and SINDI classes to explicitly indicate they override a virtual destructor from the base interface. I have no feedback to provide.

Copy link
Copy Markdown
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

Adds explicit override specifiers to destructors in two InnerIndexInterface implementations (SINDI and Pyramid) to enforce correct virtual overriding at compile time.

Changes:

  • Mark ~SINDI() as override = default
  • Mark ~Pyramid() as override = default

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/algorithm/sindi/sindi.h Adds override to SINDI destructor to explicitly override InnerIndexInterface’s virtual destructor.
src/algorithm/pyramid.h Adds override to Pyramid destructor to explicitly override InnerIndexInterface’s virtual destructor.

- Add override to destructors in DiskANN, FP32Quantizer, INT8Quantizer
- Remove redundant virtual keyword in MRLETransformer, RandomOrthogonalMatrix
- Follow C++ Core Guidelines C.128: use only override for overriding functions

Signed-off-by: LHT (glm-5) <agent@vsag.dev>
Signed-off-by: LHT129 <tianlan.lht@antgroup.com>
@LHT129 LHT129 force-pushed the 2026-03-27-统一-virtual-override-关键字使用 branch from fc6a144 to e72fb20 Compare March 27, 2026 06:32
@LHT129 LHT129 changed the title fix(algorithm): add override to destructor in SINDI and Pyramid classes fix: unify virtual/override keyword usage in destructors Mar 27, 2026
@mergify mergify bot added the module/index label Mar 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/improvement Code improvements (variable/function renaming, refactoring, etc. ) module/index size/S version/1.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants