chore(lint): update clang-tidy configuration #1088
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
wxyucs
added
the
kind/improvement
wxyucs
requested review from
ShawnShawnYou,
inabao and
jiaweizone
as code owners
|
Important Installation incomplete: to start using Gemini Code Assist, please ask the organization owner(s) to visit the Gemini Code Assist Admin Console and sign the Terms of Services. |
Reviewer's GuideThis PR updates the clang-tidy configuration throughout the codebase and applies corresponding lint-driven modernizations: it transitions raw pointer usage to make_shared, tidies up includes, simplifies API calls (e.g., BLAS and STL), adjusts CMake compile flags for AVX512, and propagates updated .clang-tidy files into subdirectories. Class diagram for simplified heap and container usageclassDiagram
class DistanceHeap {
+ Push(const DistanceRecord& record)
+ Push(float dist, InnerIdType id)
}
class MemmoveHeap {
+ Push(float dist, InnerIdType id)
}
DistanceHeap --> DistanceRecord
MemmoveHeap --> DistanceRecord
Class diagram for simplified container initialization in HGraph and ODescentclassDiagram
class HGraph {
- Vector<InnerIdType> route_graph_ids
- InnerIdType entry_point_id_
+ build_by_odescent(DatasetPtr data)
}
class ODescent {
- Vector<Linklist> graph_
+ Build(Vector<InnerIdType> ids_sequence, GraphInterfacePtr graph)
}
HGraph --> Vector
ODescent --> Linklist
Class diagram for IVFParameter bucket count calculationclassDiagram
class IVFParameter {
- BucketParam* bucket_param
- IVFPartitionStrategyParameter* ivf_partition_strategy_parameter
+ FromJson(JsonType json)
}
class BucketParam {
- int buckets_count
}
class IVFPartitionStrategyParameter {
- GNOIMIParam* gnoimi_param
- IVFPartitionStrategyType partition_strategy_type
}
class GNOIMIParam {
- int first_order_buckets_count
- int second_order_buckets_count
}
IVFParameter --> BucketParam
IVFParameter --> IVFPartitionStrategyParameter
IVFPartitionStrategyParameter --> GNOIMIParam
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- You have the same
reader_ = make_shared<LocalFileReader>(batch_read_); index_ = make_shared<diskann::PQFlashIndex<...>>(...)sequence in several methods—consider extracting that into a helper to reduce duplication. - The AVX-512 compile flags are duplicated with only the warning suppression added—consider defining a variable or property in CMake to DRY those flags and keep them in sync.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- You have the same `reader_ = make_shared<LocalFileReader>(batch_read_); index_ = make_shared<diskann::PQFlashIndex<...>>(...)` sequence in several methods—consider extracting that into a helper to reduce duplication.
- The AVX-512 compile flags are duplicated with only the warning suppression added—consider defining a variable or property in CMake to DRY those flags and keep them in sync.
## Individual Comments
### Comment 1
<location> `src/algorithm/ivf.cpp:943` </location>
<code_context>
throw VsagException(ErrorType::INTERNAL_ERROR, "invalid inner_id");
}
this->location_map_[ids[j]] =
- (static_cast<uint64_t>(i) << LOCATION_SPLIT_BIT) | static_cast<uint64_t>(j);
+ (static_cast<uint64_t>(i) << LOCATION_SPLIT_BIT) | j;
}
}
</code_context>
<issue_to_address>
Removing static_cast for j may introduce type mismatch if j is not uint64_t.
Please verify that j is always a uint64_t, or retain the cast to prevent type issues.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| throw VsagException(ErrorType::INTERNAL_ERROR, "invalid inner_id"); | ||
| } | ||
| this->location_map_[ids[j]] = | ||
| (static_cast<uint64_t>(i) << LOCATION_SPLIT_BIT) | static_cast<uint64_t>(j); |
There was a problem hiding this comment.
issue (bug_risk): Removing static_cast for j may introduce type mismatch if j is not uint64_t.
Please verify that j is always a uint64_t, or retain the cast to prevent type issues.
wxyucs
force-pushed
the
update-clang-tidy
branch
from
0aeafec to
42edb9c
Compare
Signed-off-by: Xiangyu Wang <[email protected]>
wxyucs
force-pushed
the
update-clang-tidy
branch
from
42edb9c to
9f3638b
Compare
Codecov Report❌ Patch coverage is @@ Coverage Diff @@
## main #1088 +/- ##
==========================================
- Coverage 91.53% 91.31% -0.22%
==========================================
Files 303 303
Lines 17497 17584 +87
==========================================
+ Hits 16016 16057 +41
- Misses 1481 1527 +46
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Update codebase to conform to revised clang-tidy rules by modernizing C++ idioms, refining build flags, and refreshing lint configurations.
Enhancements:
Build:
Documentation: