Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

some clang-tidy #309

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

some clang-tidy #309

wants to merge 3 commits into from

Conversation

neheb
Copy link
Contributor

@neheb neheb commented Feb 6, 2025

No description provided.

neheb added 3 commits February 6, 2025 13:02
Can avoid the lambdas by using operator==.

Also converted some to for range loops where it made sense.

Signed-off-by: Rosen Penev <[email protected]>
Found with modernize-min-max-use-initializer-list

Signed-off-by: Rosen Penev <[email protected]>
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Clang-Tidy found issue(s) with the introduced code (1/1)

@@ -85,7 +85,7 @@ Chunk::at_position(uint32_t pos, iterator itr) {

inline Chunk::iterator
Copy link

Choose a reason for hiding this comment

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

⚠️ clang-diagnostic-error ⚠️
Chunk is not a class, namespace, or enumeration

@@ -85,7 +85,7 @@ Chunk::at_position(uint32_t pos, iterator itr) {

inline Chunk::iterator
Chunk::find_address(void* ptr) {
Copy link

Choose a reason for hiding this comment

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

⚠️ clang-diagnostic-error ⚠️
Chunk is not a class, namespace, or enumeration

@rakshasa
Copy link
Owner

rakshasa commented Feb 7, 2025

Again these are not changes that materially improve the code, and in some places actually makes it more obtuse.

E.g. expanding compact if/else statements to multiple lines and adding operators to Chunks when the explicit member function call is more descriptive.

There are lots of places where effort to clean up the code would be greatly appreciated, but these changes are not the right ones.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants