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

EXPERIMENTAL clang-tidy readability-braces-around-statements #3698

Closed

Conversation

rwgk
Copy link
Collaborator

@rwgk rwgk commented Feb 7, 2022

Description

The work under this experimental PR was used to inform PR #3699.

Suggested changelog entry:

@rwgk rwgk force-pushed the clang-tidy_readability-braces-around-statements branch from 47df05c to ffa3eed Compare February 7, 2022 20:34
@rwgk
Copy link
Collaborator Author

rwgk commented Feb 7, 2022

Just for entertainment, this

(original)

                #if defined(__cpp_aligned_new) && (!defined(_MSC_VER) || _MSC_VER >= 1912)
                    if (type->type_align > __STDCPP_DEFAULT_NEW_ALIGNMENT__)
                        vptr = ::operator new(type->type_size,
                                              std::align_val_t(type->type_align));
                    else
                #endif
                vptr = ::operator new(type->type_size);

really threw

  1. clang-tidy
                #if defined(__cpp_aligned_new) && (!defined(_MSC_VER) || _MSC_VER >= 1912)
                if (type->type_align > __STDCPP_DEFAULT_NEW_ALIGNMENT__) {
                    vptr = ::operator new(type->type_size, std::align_val_t(type->type_align));
                } else {
#endif
                    vptr = ::operator new(type->type_size);
                }
  1. me
#if defined(__cpp_aligned_new) && (!defined(_MSC_VER) || _MSC_VER >= 1912)
                if (type->type_align > __STDCPP_DEFAULT_NEW_ALIGNMENT__) {
                    vptr = ::operator new(type->type_size, std::align_val_t(type->type_align));
                } else {
                    vptr = ::operator new(type->type_size);
                }
#endif

into a tail spin.

Note that clang-tidy made several other mistakes around other #ifdefs, and that I had no awareness of the original when I looked at 1. above.

Final correctly working version:

#if defined(__cpp_aligned_new) && (!defined(_MSC_VER) || _MSC_VER >= 1912)
                if (type->type_align > __STDCPP_DEFAULT_NEW_ALIGNMENT__) {
                    vptr = ::operator new(type->type_size, std::align_val_t(type->type_align));
                } else {
                    vptr = ::operator new(type->type_size);
                }
#else
                vptr = ::operator new(type->type_size);
#endif

rwgk pushed a commit to rwgk/pybind11 that referenced this pull request Feb 8, 2022
@rwgk rwgk changed the title clang-tidy readability-braces-around-statements EXPERIMENTAL clang-tidy readability-braces-around-statements Feb 8, 2022
@rwgk rwgk closed this Feb 8, 2022
rwgk pushed a commit to rwgk/pybind11 that referenced this pull request Feb 8, 2022
rwgk pushed a commit that referenced this pull request Feb 8, 2022
…d-statements.

Informed by experiments under PR #3698.
@rwgk rwgk deleted the clang-tidy_readability-braces-around-statements branch February 8, 2022 21:06
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.

1 participant