Skip to content

Commit 4d232d5

Browse files
authored
Parameterise clangformat_targets for reuse by other projects (#826)
The clangformat_targets() CMake function previously hardcoded the source glob patterns, exclusion regexes, target name, and tool version, making it unusable for projects with a different directory layout or toolchain. This PR makes the function fully configurable via optional keyword arguments, with defaults that preserve the existing behaviour for snmalloc: INCLUDE_PATTERNS — glob patterns for source file discovery (default: src/.cc src/.h src/.hh). EXCLUDE_PATTERNS — regex patterns to exclude from formatting (default: src/[^/]/[^/]*_concept.h$). VERSION — clang-format version to search for (default: 15). The versioned names (clang-format-15, clang-format150) are tried first; if not found, falls back to the unversioned clang-format with a warning. TARGET_NAME — name of the generated CMake target (default: clangformat), avoiding collisions when snmalloc is consumed as a subdirectory. Additionally, a -check target (e.g. clangformat-check) is now generated alongside the formatting target. It runs clang-format --dry-run --Werror, suitable for CI validation without modifying files. The existing call site clangformat_targets() is unchanged and behaves identically to before.
1 parent 139180c commit 4d232d5

File tree

3 files changed

+73
-18
lines changed

3 files changed

+73
-18
lines changed

.github/workflows/main.yml

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -373,10 +373,7 @@ jobs:
373373
# Run the clang-format check and error if it generates a diff
374374
- name: Run clang-format
375375
working-directory: ${{github.workspace}}/build
376-
run: |
377-
set -eo pipefail
378-
make clangformat
379-
git diff --exit-code
376+
run: make clangformat-check
380377
- name: Run clang-tidy
381378
run: |
382379
clang-tidy-15 src/snmalloc/override/malloc.cc -header-filter="`pwd`/*" -warnings-as-errors='*' -export-fixes=tidy.fail -- -std=c++17 -mcx16 -DSNMALLOC_USE_WAIT_ON_ADDRESS=1 -DSNMALLOC_PLATFORM_HAS_GETENTROPY=0 -Isrc

CMakeLists.txt

Lines changed: 70 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -213,30 +213,88 @@ function(warnings_high)
213213
endif()
214214
endfunction()
215215

216+
# clangformat_targets([VERSION <ver>]
217+
# [TARGET_NAME <name>]
218+
# [INCLUDE_PATTERNS <glob> ...]
219+
# [EXCLUDE_PATTERNS <regex> ...])
220+
#
221+
# Generate a CMake target that runs clang-format on a set of source files.
222+
# A companion <name>-check target is also created that verifies formatting
223+
# without modifying files (--dry-run --Werror), suitable for CI.
224+
#
225+
# Options:
226+
# VERSION - clang-format version to search for (default: 15).
227+
# Falls back to the unversioned 'clang-format' with a
228+
# warning if the requested version is not found.
229+
# TARGET_NAME - Name of the generated targets (default: clangformat).
230+
# INCLUDE_PATTERNS - Glob patterns passed to file(GLOB_RECURSE) to collect
231+
# source files (default: src/*.cc src/*.h src/*.hh).
232+
# EXCLUDE_PATTERNS - Regex patterns used to exclude files from the collected
233+
# list (default: src/[^/]*/[^/]*_concept\.h$).
216234
function(clangformat_targets)
235+
cmake_parse_arguments(
236+
CLANGFMT
237+
""
238+
"VERSION;TARGET_NAME"
239+
"INCLUDE_PATTERNS;EXCLUDE_PATTERNS"
240+
${ARGN}
241+
)
242+
243+
# Default inclusion globs match this project's source layout.
244+
if (NOT CLANGFMT_INCLUDE_PATTERNS)
245+
set(CLANGFMT_INCLUDE_PATTERNS src/*.cc src/*.h src/*.hh)
246+
endif()
247+
248+
# Default exclusion regexes skip concept headers that clang-format
249+
# does not yet handle well. See https://reviews.llvm.org/D79773
250+
if (NOT CLANGFMT_EXCLUDE_PATTERNS)
251+
set(CLANGFMT_EXCLUDE_PATTERNS "src/[^/]*/[^/]*_concept\\.h$")
252+
endif()
253+
254+
# Default clang-format version.
255+
if (NOT CLANGFMT_VERSION)
256+
set(CLANGFMT_VERSION 15)
257+
endif()
258+
259+
# Default target name.
260+
if (NOT CLANGFMT_TARGET_NAME)
261+
set(CLANGFMT_TARGET_NAME clangformat)
262+
endif()
263+
217264
# The clang-format tool is installed under a variety of different names. Try
218-
# to find a sensible one. Only look for versions 9 explicitly - we don't
219-
# know whether our clang-format file will work with newer versions of the
220-
# tool. It does not work with older versions as AfterCaseLabel is not supported
221-
# in earlier versions.
265+
# to find a sensible one. Look for the requested version first, then fall
266+
# back to the unversioned name.
222267
find_program(CLANG_FORMAT NAMES
223-
clang-format150 clang-format-15)
268+
clang-format${CLANGFMT_VERSION}0
269+
clang-format-${CLANGFMT_VERSION})
270+
271+
if (${CLANG_FORMAT} STREQUAL "CLANG_FORMAT-NOTFOUND")
272+
find_program(CLANG_FORMAT NAMES clang-format)
273+
if (NOT ${CLANG_FORMAT} STREQUAL "CLANG_FORMAT-NOTFOUND")
274+
message(WARNING "Could not find clang-format version ${CLANGFMT_VERSION}, falling back to ${CLANG_FORMAT}")
275+
endif()
276+
endif()
224277

225278
# If we've found a clang-format tool, generate a target for it, otherwise emit
226279
# a warning.
227280
if (${CLANG_FORMAT} STREQUAL "CLANG_FORMAT-NOTFOUND")
228-
message(WARNING "Not generating clangformat target, no clang-format tool found")
281+
message(WARNING "Not generating ${CLANGFMT_TARGET_NAME} target, no clang-format tool found")
229282
else ()
230-
message(STATUS "Generating clangformat target using ${CLANG_FORMAT}")
231-
file(GLOB_RECURSE ALL_SOURCE_FILES CONFIGURE_DEPENDS src/*.cc src/*.h src/*.hh)
232-
# clangformat does not yet understand concepts well; for the moment, don't
233-
# ask it to format them. See https://reviews.llvm.org/D79773
234-
list(FILTER ALL_SOURCE_FILES EXCLUDE REGEX "src/[^/]*/[^/]*_concept\.h$")
283+
message(STATUS "Generating ${CLANGFMT_TARGET_NAME} target using ${CLANG_FORMAT}")
284+
file(GLOB_RECURSE ALL_SOURCE_FILES CONFIGURE_DEPENDS ${CLANGFMT_INCLUDE_PATTERNS})
285+
foreach(pattern IN LISTS CLANGFMT_EXCLUDE_PATTERNS)
286+
list(FILTER ALL_SOURCE_FILES EXCLUDE REGEX "${pattern}")
287+
endforeach()
235288
add_custom_target(
236-
clangformat
289+
${CLANGFMT_TARGET_NAME}
237290
COMMAND ${CLANG_FORMAT}
238291
-i
239292
${ALL_SOURCE_FILES})
293+
add_custom_target(
294+
${CLANGFMT_TARGET_NAME}-check
295+
COMMAND ${CLANG_FORMAT}
296+
--dry-run --Werror
297+
${ALL_SOURCE_FILES})
240298
endif()
241299
endfunction()
242300

MODULE.bazel

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
module(name = "snmalloc")
22

3-
bazel_dep(name = "rules_cc", version = "0.1.1")
4-
bazel_dep(name = "rules_foreign_cc", version = "0.14.0")
3+
bazel_dep(name = "rules_cc", version = "0.2.17")
4+
bazel_dep(name = "rules_foreign_cc", version = "0.15.1")
55
bazel_dep(name = "fuzztest", version = "20250214.0")
66
bazel_dep(name = "googletest", version = "1.16.0")

0 commit comments

Comments
 (0)