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

mitigated some modernize-use-auto clang-tidy warnings #5838

Merged
merged 2 commits into from
May 24, 2024

Conversation

firewave
Copy link
Collaborator

@firewave firewave commented Jan 5, 2024

No description provided.

@firewave
Copy link
Collaborator Author

firewave commented Jan 5, 2024

This produces a lot of warning for iterators. We could switch to auto in cases where the type is actually explicit like cbegin() etc.. But for calls like find() it depends on the container and we might end up with an iterator which would undermine the intended constness.

It will most likely not be of any harm but I would prefer to enforce the constness if possible. That could be done using std::as_const() but that isn't available until C++17.

As the iterator case already yields a separate message this is treated as a different group of issues. So I will file a ticket about making this configurable.

@firewave firewave force-pushed the tidy-auto-x branch 2 times, most recently from 6b57ec3 to c31b929 Compare March 7, 2024 14:04
@firewave firewave changed the title enabled modernize-use-auto clang-tidy check mitigated some modernize-use-auto clang-tidy warnings Mar 7, 2024
@firewave
Copy link
Collaborator Author

firewave commented Mar 7, 2024

I filed llvm/llvm-project#84324 upstream about the iterator issue.

@firewave firewave force-pushed the tidy-auto-x branch 2 times, most recently from 4766a17 to b4d5fe8 Compare March 7, 2024 15:08
@firewave firewave marked this pull request as ready for review May 24, 2024 08:21
@firewave firewave merged commit fa561e9 into danmar:main May 24, 2024
63 checks passed
@firewave firewave deleted the tidy-auto-x branch May 24, 2024 22:03
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