Clear designated-initializers and internal-linkage; drop ifelse-braces - #284
Merged
Conversation
modernize-use-designated-initializers (32): applied clang-tidy's fixes to
diff_test.cc, diff_benchmark.cc and diff_myers.cc - aggregate initialisers
gain field names, which is what STYLE_CPP.md already shows.
misc-use-internal-linkage (19):
* glob_main.cc `Entries` moved into an anonymous namespace - it is
file-local, so this is the real fix.
* stringify_ostream.cc got 3 NOLINTs instead. The check's own fix broke
the build: those functions are DECLARED in stringify_ostream.h and
called from it, so marking them static produced
`-Werror,-Wunused-function` on all three. That fix was reverted.
readability-inconsistent-ifelse-braces (25) is disabled instead of fixed.
20 of its findings are the `if` inside MBO_RETURN_IF_ERROR /
MBO_ASSIGN_OR_RETURN: it reports the macro's CALL SITE and asks for braces
that can only be written in the macro definition, so no edit at the
reported location satisfies it. clang-format's `InsertBraces` already adds
braces to real if/else bodies on every commit, so nothing is lost.
All three checks report zero through .clang-tidy.
bazel test --config=clang //... - 109/109 pass.
Signed-off-by: helly25 <6420169+helly25@users.noreply.github.com>
Fab-Cat
approved these changes
Aug 9, 2026
helly25
enabled auto-merge (squash)
August 9, 2026 19:31
helly25
added a commit
that referenced
this pull request
Aug 9, 2026
Applied clang-tidy's fixes for readability-math-missing-parentheses, modernize-type-traits, modernize-use-auto, modernize-use-constraints and readability-static-definition-in-anonymous-namespace: explicit precedence parentheses, `std::remove_reference_t<T>` for `std::remove_reference<T>::type`, `auto` for a repeated cast type, and `requires` clauses in place of `std::enable_if_t` parameters. readability-redundant-parentheses is DISABLED, because its fix corrupts code rather than tidying it. On __builtin_dump_struct(ptr, &DumpStructVisitor, fields, field_index); it deletes the callee - exactly the 21 characters of the builtin's name - leaving (ptr, &DumpStructVisitor, fields, field_index); a comma expression that does nothing and fails to compile under -Werror,-Wunused-value. It did this in struct_names_clang.h and extend_test.cc, i.e. the reflection machinery. Both files were reverted. The findings it reports are cosmetic; the hazard of anyone running --fix is not. That is the third check whose auto-fix is unsafe here, after misc-const-correctness (#275, wrong const on assigned variables) and misc-use-internal-linkage (#284, static on header-declared functions). bazel test --config=clang //... - 109/109 pass. Signed-off-by: helly25 <6420169+helly25@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
76 findings across the three checks agreed in triage. Two were fixed as intended; the third turned out not to be fixable at the reported location.
modernize-use-designated-initializers(32) — fixedApplied clang-tidy's fixes to
diff_test.cc,diff_benchmark.ccanddiff_myers.cc. Aggregate initialisers gain field names, which is whatSTYLE_CPP.mdalready shows:- {"equal_10k", {NumberedLines(10'000, "line-"), "lhs"}, {NumberedLines(10'000, "line-"), "rhs"}}, + {.name = "equal_10k", + .lhs = {.data = NumberedLines(10'000, "line-"), .name = "lhs"}, + .rhs = {.data = NumberedLines(10'000, "line-"), .name = "rhs"}},misc-use-internal-linkage(19) — fixed, with one exceptionglob_main.cc—Entriesis genuinely file-local and is now inside an anonymous namespace. That is the real fix.stringify_ostream.cc— 3NOLINTs instead, because the check's own fix broke the build. Those functions are declared instringify_ostream.h(lines 33/41/47) and called from it, soFixMode: UseStaticproduced:That hunk was reverted. This is the second check whose auto-fix is unsafe here, after
misc-const-correctnessin #275.readability-inconsistent-ifelse-braces(25) — disabled, not fixed20 of the 25 are the
ifinsideMBO_RETURN_IF_ERROR/MBO_ASSIGN_OR_RETURN. The check reports the macro's call site:There is no edit at that location that satisfies it — the braces would have to go in the macro definition. And nothing is lost by disabling: clang-format's
InsertBracesalready adds braces to realif/elsebodies on every commit, which is the rule this check duplicates.Test
.clang-tidy(measured viatools/clang_tidy.sh, not with the checks re-enabled on the command line).bazel test --config=clang //...— 109/109 pass.pre-commit run -agreen.