Skip to content

Bump clang-format from 19.1.7 to 20.1.7 #1419

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 4 commits into from
Jul 1, 2025

Conversation

eramongodb
Copy link
Contributor

@eramongodb eramongodb commented Jun 27, 2025

Bumps the clang-format version from 19.1.7 to 20.1.7 as followup to mongodb/mongo-c-driver#2046 to improve tooling consistency across both repos.


The configuration file has been updated for Clang 20.1. Notable changes include:

  • AlignConsecutiveDeclarations.AlignFunctionDeclarations is set to false for consistency with other AlignConsecutiveDeclarations options (defaults to true).
  • ReflowComments is set to Always (equivalent to former value of true).

All other options are left unchanged from their prior (existing) or default (new) values.

Aside: it'd be nice if ClangFormat supported a ReflowStrings option...


Changes due to this version bump seem to primarily fall into two categories:

  • Attribute and typename macros are no longer wrapped even when they exceed the column limit, which I personally consider to be an improvement, although opinions may differ. e.g.:
// Before                                                                                                              v
    template <typename F, typename... Args, typename Fd = remove_cvref_t<F>>
    constexpr auto operator()(F&& fn, Args&&... args) const
        BSONCXX_PRIVATE_RETURNS(impl_invoke::invoker<
                                std::is_member_object_pointer<Fd>::value,
                                std::is_member_function_pointer<
                                    Fd>::value>::apply(static_cast<F&&>(fn), static_cast<Args&&>(args)...));
//                                                                                                                     ^

// After                                                                                                               v
    template <typename F, typename... Args, typename Fd = remove_cvref_t<F>>
    constexpr auto operator()(F&& fn, Args&&... args) const
        BSONCXX_PRIVATE_RETURNS(impl_invoke::invoker<std::is_member_object_pointer<Fd>::value, std::is_member_function_pointer<Fd>::value>::apply(static_cast<F&&>(fn), static_cast<Args&&>(args)...));
//                                                                                                                     ^

// Before                                                                                                              v
    MONGOCXX_ABI_EXPORT_CDECL(bsoncxx::v_noabi::stdx::optional<
                              bsoncxx::v_noabi::types::bson_value::view_or_value> const&)
    comment() const;
//                                                                                                                     ^

// After                                                                                                               v
    MONGOCXX_ABI_EXPORT_CDECL(bsoncxx::v_noabi::stdx::optional<bsoncxx::v_noabi::types::bson_value::view_or_value> const&)
    comment() const;
//                                                                                                                     ^
  • Breaking after opening parenthesis of (nested) function calls seems to have been fixed/improved...? The behavior seems to be more consistent than before, although it overall produces deeper indentation levels. e.g.:
// Before
result.append(builder::basic::kvp(
    "result", [matched_count, modified_count, upserted_count, upserted_id](builder::basic::sub_document subdoc) {
        subdoc.append(builder::basic::kvp("matchedCount", matched_count));

        if (modified_count) {
            subdoc.append(builder::basic::kvp("modifiedCount", *modified_count));
        }

        subdoc.append(builder::basic::kvp("upsertedCount", upserted_count));

        if (upserted_id) {
            subdoc.append(builder::basic::kvp("upsertedId", *upserted_id));
        }
    }));

// After
result.append(
    builder::basic::kvp(
        "result",
        [matched_count, modified_count, upserted_count, upserted_id](builder::basic::sub_document subdoc) {
            subdoc.append(builder::basic::kvp("matchedCount", matched_count));

            if (modified_count) {
                subdoc.append(builder::basic::kvp("modifiedCount", *modified_count));
            }

            subdoc.append(builder::basic::kvp("upsertedCount", upserted_count));

            if (upserted_id) {
                subdoc.append(builder::basic::kvp("upsertedId", *upserted_id));
            }
        }));

Although the diff is more than I expected from a ClangFormat 19 -> ClangFormat 20 bump, I am fine with accepting both of these changes as-is without further refinement of the updated configuration file.

@eramongodb eramongodb self-assigned this Jun 27, 2025
@eramongodb eramongodb requested a review from a team as a code owner June 27, 2025 16:16
Copy link
Contributor

@vector-of-bool vector-of-bool left a comment

Choose a reason for hiding this comment

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

LGTM.

I'm curious what changes might occur if you add our attribute-ish macros to the .clang-format file. It might reflow them better?

@eramongodb
Copy link
Contributor Author

eramongodb commented Jul 1, 2025

@vector-of-bool Unless I misunderstood your question, they are already present following #1299 and #1318, e.g. BSONCXX_PRIVATE_RETURNS is currently specified as an AttributeMacros. fwiw specifying BSONCXX_PRIVATE_RETURNS as a StatementMacros instead results in the following (update: fixed the comparison actually fixed the comparison, sorry third time's the charm...):

// As an "attribute macro" (current).
template <typename F, typename... Args, typename Fd = remove_cvref_t<F>>
constexpr auto operator()(F&& fn, Args&&... args) const
    BSONCXX_PRIVATE_RETURNS(impl_invoke::invoker<std::is_member_object_pointer<Fd>::value, std::is_member_function_pointer<Fd>::value>::apply(static_cast<F&&>(fn), static_cast<Args&&>(args)...));

// As a "statement macro".
template <typename F, typename... Args, typename Fd = remove_cvref_t<F>>
constexpr auto operator()(F&& fn, Args&&... args) const BSONCXX_PRIVATE_RETURNS(
    impl_invoke::invoker<
        std::is_member_object_pointer<Fd>::value,
        std::is_member_function_pointer<Fd>::value>::apply(static_cast<F&&>(fn), static_cast<Args&&>(args)...));

Moving it under TypenameMacros appears to make no difference compared to AttributeMacros. I don't think either is particularly better; I believe they are already appropriately specified in the ClangFormat file.

@eramongodb eramongodb merged commit 72f6501 into mongodb:master Jul 1, 2025
21 checks passed
@eramongodb eramongodb deleted the cxx-clang-format branch July 1, 2025 18:26
eramongodb added a commit to eramongodb/mongo-cxx-driver that referenced this pull request Jul 9, 2025
* Update Python package dependencies
* Update clang-format configuration file for Clang 20.1
* Format all source files
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.

3 participants