[slachiewicz]

Adds a CI job that runs clang-format 18 on the lines a pull request changes in lib/cpp, so .clang-format is enforced from now on without reformatting existing code.

Six options are added to .clang-format so that it describes lib/cpp as it is written: FixNamespaceComments, ReflowComments, SortIncludes, BinPackArguments, SortUsingDeclarations and SpacesInLineCommentPrefix. The Base64 decode table is excluded with // clang-format off. With both, formatting all of lib/cpp/src changes 104 files and 1,583 lines, down from 149 files and 2,674. The remainder are files with a local style of their own (indented #if, nested namespaces on one line, aligned enum values) that no global option matches.

The job does not cover compiler/cpp or test/cpp.

Verified: git-clang-format-18 --diff HEAD^ -- lib/cpp exits 1 on a misformatted line in a .tcc file, and 0 on a correctly formatted change and on a change without C++ files.

• Did you create an Apache Jira ticket? (Request account here, not required for trivial changes)
• If a ticket exists: Does your pull request title follow the pattern "THRIFT-NNNN: describe my issue"?
• Did you squash your changes to a single commit? (not required, but preferred)
• Did you do your best to avoid breaking changes? If one was needed, did you label the Jira ticket with "Breaking-Change"?
• If your change does not involve any code, include [skip ci] anywhere in the commit message to free up build resources.

Generated-by: Claude Opus 5.5

[slachiewicz]

@Jens-G, is this the direction you want for THRIFT-6219? It takes the ticket's second option, adjusting .clang-format to the code, and checks only changed lines instead of reformatting lib/cpp once. If you would rather do the one-time reformat, the tuned config makes that commit 104 files instead of 149, and the job can then check whole files.

[Copilot]

🟢 Approval recommended

The incremental C++ formatting enforcement changes have no active review issues.

No open issues 🎉

What changed in this PR

Adds incremental C++ formatting enforcement for lib/cpp, checking only changed lines to avoid reformatting legacy code. The check excludes compiler/cpp and test/cpp.

Changes:

• Adds a pull-request workflow using clang-format 18, including .tcc files.
• Adjusts six formatting options to better match existing C++ style.
• Preserves the Base64 decode table’s layout with formatting-disable comments.

File	Description
lib/​cpp/​src/​thrift/​protocol/​TBase64Utils.cpp	Excludes the decode table from formatting without changing runtime behavior.
.github/​workflows/​cpp-format.yml	Checks changed lines in lib/cpp and reports formatting failures.
.clang-format	Aligns formatting settings with existing code conventions.

_{Review effort: Balanced}

[Jens-G]

Code review

This is a code-level review only. It does not answer the direction question above.

No issues found. Checked for bugs and CLAUDE.md compliance.

I ran the gate's exact command with clang-format 18.1.3 on merge commits built the way refs/pull/N/merge is, including a --depth 2 clone:

• A misformatted line in a .tcc, .cpp or .h file fails it with exit 1, and the diff shows only that line.
• A correctly formatted change in TSocket.cpp passes, although that file still has 45 lines of older drift.
• Changes to non-C++ files, or outside lib/cpp, pass.

The 149 → 104 file counts for lib/cpp/src reproduce.

Suggestion, not a blocker: the fix hint can reformat lines the PR never touched. CONTRIBUTING.md has contributors clone their fork, so origin/master is usually the fork's master. I ran the suggested command against a fork master one month behind the branch base. It rewrote 21 files in lib/cpp (318+/256−) that the change never touched. Against HEAD^ it changed nothing. For the single squashed commit the project asks for, HEAD^ keeps the hint to the PR's own lines; $(git merge-base upstream/master HEAD) works in general. Also, with only the clang-format-18 package installed, as in this workflow, there is no git clang-format; the command there is git clang-format-18, which is how I ran it.

thrift/.github/workflows/cpp-format.yml

Lines 65 to 67 in 5e08ce1

	--diff HEAD^ -- lib/cpp; then
	echo "::error::Changed lines in lib/cpp do not match .clang-format. Run 'git clang-format --binary clang-format-18 --extensions c,cc,cpp,cxx,h,hh,hpp,tcc origin/master -- lib/cpp' and amend the commit."
	exit 1

[Jens-G]

@slachiewicz, thanks for the groundwork. I'd prefer the one-time reformat. With only changed lines checked, most of lib/cpp stays out of step with .clang-format, and formatting a whole file still brings the unrelated diff the ticket is about: with the tuned config that is still 104 of 156 files in lib/cpp/src and 50 of 73 in lib/cpp/test. Your tuning is what makes the reformat affordable.

So, in this PR:

• reformat all of lib/cpp, lib/cpp/test included, with clang-format 18 and the tuned config, and let the job check whole files;
• make make style and lib/cpp/coding_standards.md use clang-format 18 like the job. Today CPPSTYLE_CMD runs whatever clang-format is on the path, and the doc still says "3.5 or newer".

Once this is merged, its commit can go into a new .git-blame-ignore-revs, so GitHub's blame view (and git blame --ignore-revs-file) skips the reformat.

Drafted with AI assistance (Claude Opus 5.5); reviewed and posted by Jens Geyer.