Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Apply some modernization to C++ extensions #29280

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

Merged
merged 5 commits into from
Dec 12, 2024

Conversation

QuLogic
Copy link
Member

@QuLogic QuLogic commented Dec 11, 2024

PR summary

As we are now on C++17, this applies some of the modernization checks from clang-tidy, namely:

  • modernize-loop-convert and modernize-use-auto (but only applied to loops)
  • modernize-use-nullptr
  • modernize-use-emplace
  • modernize-use-equals-default and modernize-use-equals-delete
  • modernize-type-traits

PR checklist

Copy link
Member

@ianthomas23 ianthomas23 left a comment

Choose a reason for hiding this comment

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

I've checked this all and everything is fine except for the one nit.

src/tri/_tri.cpp Outdated Show resolved Hide resolved
Also use `auto` and structured binding for iterators.

This is based on running `clang-tidy` with the `modernize-loop-convert`
and `modernize-use-auto` checks. Note though that I did not (yet) apply
any `auto` suggestions that were not `for` loops.
This uses clang-tidy's `modernize-use-nullptr` check.
With `emplace_back`, we don't need to create an object ourselves, to be
copied into the container, plus it returns a reference to the new object
for us to use.

This used clang-tidy's `modernize-use-emplace` check, plus some
followups to use the return values.
This is applying clang-tidy's `modernize-use-equals-default` and
`modernize-use-equals-delete` checks.
This applies clang-tidy's `modernize-type-traits` check.
Copy link
Member

@ianthomas23 ianthomas23 left a comment

Choose a reason for hiding this comment

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

Approved assuming CI passes.

@WeatherGod WeatherGod merged commit 758a399 into matplotlib:main Dec 12, 2024
39 of 40 checks passed
@QuLogic QuLogic deleted the ext-modernizing branch December 12, 2024 20:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
Morty Proxy This is a proxified and sanitized view of the page, visit original site.