-
Notifications
You must be signed in to change notification settings - Fork 0
build: add lint, CI, and build tooling #1
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
Changes from all commits
cbd88eb
bd8f9a9
509da7e
1b7d251
70a85b1
6420b49
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| BasedOnStyle: Google | ||
| ColumnLimit: 100 | ||
| DerivePointerAlignment: false | ||
| IncludeBlocks: Preserve |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| --- | ||
| # Google style plus correctness/concurrency checks. Naming follows the Google | ||
| # C++ Style Guide, with STL-style lower_case method names for container-like | ||
| # types (explicitly permitted by the guide for STL-consistent interfaces). | ||
| Checks: > | ||
| bugprone-*, | ||
| clang-analyzer-*, | ||
| concurrency-*, | ||
| google-*, | ||
| misc-*, | ||
| modernize-*, | ||
| performance-*, | ||
| readability-*, | ||
| -modernize-use-trailing-return-type, | ||
| -misc-include-cleaner | ||
| WarningsAsErrors: '*' | ||
| HeaderFilterRegex: 'include/cq/.*' | ||
| CheckOptions: | ||
| # Complexity contributed by macro expansion (GTest asserts, etc.) is not | ||
| # the author's complexity. | ||
| readability-function-cognitive-complexity.IgnoreMacros: 'true' | ||
| # `auto _` is the general placeholder idiom (a language feature in C++26). | ||
| readability-identifier-length.IgnoredVariableNames: '^_$' | ||
| readability-identifier-naming.ClassCase: CamelCase | ||
| readability-identifier-naming.StructCase: CamelCase | ||
| readability-identifier-naming.EnumCase: CamelCase | ||
| readability-identifier-naming.TypeAliasCase: CamelCase | ||
| readability-identifier-naming.FunctionCase: lower_case | ||
| readability-identifier-naming.VariableCase: lower_case | ||
| readability-identifier-naming.ParameterCase: lower_case | ||
| readability-identifier-naming.PrivateMemberSuffix: '_' | ||
| readability-identifier-naming.ConstexprVariablePrefix: 'k' | ||
| readability-identifier-naming.ConstexprVariableCase: CamelCase | ||
| readability-identifier-naming.NamespaceCase: lower_case |
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,64 @@ | ||||||||
| name: CI | ||||||||
|
|
||||||||
| on: | ||||||||
| push: | ||||||||
| branches: [main] | ||||||||
| pull_request: | ||||||||
|
|
||||||||
| concurrency: | ||||||||
| group: ${{ github.workflow }}-${{ github.ref }} | ||||||||
| cancel-in-progress: true | ||||||||
|
|
||||||||
| env: | ||||||||
| # Single parallelism knob: cmake --build and ctest read these from the | ||||||||
| # environment, and the lint job reuses it for xargs -P. | ||||||||
| CMAKE_BUILD_PARALLEL_LEVEL: 4 | ||||||||
| CTEST_PARALLEL_LEVEL: 4 | ||||||||
|
|
||||||||
| jobs: | ||||||||
| test-tsan: | ||||||||
| name: Tests (ThreadSanitizer, ${{ matrix.os }}) | ||||||||
| runs-on: ${{ matrix.os }} | ||||||||
| strategy: | ||||||||
| fail-fast: false | ||||||||
| matrix: | ||||||||
| os: [ubuntu-latest, macos-latest] | ||||||||
| steps: | ||||||||
| - uses: actions/checkout@v4 | ||||||||
| - name: Configure | ||||||||
| run: cmake -B build-tsan -DENABLE_TSAN=ON -DCMAKE_BUILD_TYPE=Debug | ||||||||
| - name: Build | ||||||||
| run: cmake --build build-tsan | ||||||||
| - name: Test | ||||||||
| # --no-tests=error arms itself once tests/ lands; until then the | ||||||||
| # scaffolding is allowed to no-op. | ||||||||
| run: ctest --test-dir build-tsan --output-on-failure --no-tests="$([ -d tests ] && echo error || echo ignore)" | ||||||||
|
|
||||||||
| bench-build: | ||||||||
| name: Benchmarks (Release, smoke-run) | ||||||||
| runs-on: ubuntu-latest | ||||||||
| steps: | ||||||||
| - uses: actions/checkout@v4 | ||||||||
| - name: Configure | ||||||||
| run: cmake -B build-rel -DCMAKE_BUILD_TYPE=Release -DCQ_BUILD_TESTS=OFF | ||||||||
| - name: Build | ||||||||
| run: cmake --build build-rel | ||||||||
| - name: Smoke-run benchmarks | ||||||||
| run: ctest --test-dir build-rel --output-on-failure --no-tests="$([ -d bench ] && echo error || echo ignore)" | ||||||||
|
|
||||||||
| lint: | ||||||||
| name: clang-format & clang-tidy | ||||||||
| runs-on: ubuntu-latest | ||||||||
| env: | ||||||||
| # Pinned so a runner-image rollover can't change the formatting | ||||||||
| # contract under us; bump deliberately. | ||||||||
| CLANG_VERSION: 18 | ||||||||
|
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The version pin isn't backed by an install step. |
||||||||
| steps: | ||||||||
| - uses: actions/checkout@v4 | ||||||||
| - name: clang-format | ||||||||
| run: git ls-files '*.hpp' '*.ipp' '*.cpp' | xargs -r "clang-format-$CLANG_VERSION" --dry-run --Werror | ||||||||
|
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
| - name: clang-tidy | ||||||||
| run: | | ||||||||
| cmake -B build-lint | ||||||||
| git ls-files '*.cpp' | | ||||||||
|
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. clang-tidy never covers the header-only product directly. |
||||||||
| xargs -r -P "$CMAKE_BUILD_PARALLEL_LEVEL" -n1 "clang-tidy-$CLANG_VERSION" -p build-lint | ||||||||
|
Comment on lines
+63
to
+64
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This hand-rolled pipeline re-implements
Suggested change
|
||||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,4 @@ | ||||||
| build*/ | ||||||
|
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unanchored pattern ignores nested source directories.
Suggested change
|
||||||
| .cache/ | ||||||
| compile_commands.json | ||||||
| .DS_Store | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| { | ||
| // Template implementation files are C++ (included from headers). | ||
| "files.associations": { | ||
| "*.ipp": "cpp" | ||
| }, | ||
| // Relayout on save via the repo's .clang-format (Prettier-style). | ||
| // Works with either the clangd extension or Microsoft's C/C++ extension. | ||
| "[cpp]": { | ||
| "editor.formatOnSave": true | ||
|
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. formatOnSave needs |
||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,66 @@ | ||||||
| cmake_minimum_required(VERSION 3.24) | ||||||
|
|
||||||
| project(concurrent_queue | ||||||
| VERSION 0.1.0 | ||||||
| DESCRIPTION "Concurrent queues in C++20, from locked baseline to lock-free" | ||||||
| LANGUAGES CXX) | ||||||
|
|
||||||
| option(ENABLE_TSAN "Build with ThreadSanitizer" OFF) | ||||||
| option(CQ_BUILD_TESTS "Build unit and stress tests" ON) | ||||||
| option(CQ_BUILD_BENCHMARKS "Build Google Benchmark targets" ON) | ||||||
|
|
||||||
| # Every build tree gets a compile_commands.json for clangd/clang-tidy. | ||||||
| set(CMAKE_EXPORT_COMPILE_COMMANDS ON) | ||||||
|
|
||||||
| # Header-only library; consumers inherit C++20 via the compile feature. | ||||||
| add_library(cq INTERFACE) | ||||||
| add_library(cq::cq ALIAS cq) | ||||||
| target_include_directories(cq INTERFACE ${CMAKE_CURRENT_SOURCE_DIR}/include) | ||||||
| target_compile_features(cq INTERFACE cxx_std_20) | ||||||
|
|
||||||
| add_library(cq_warnings INTERFACE) | ||||||
|
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||||||
| target_compile_options(cq_warnings INTERFACE | ||||||
| $<$<CXX_COMPILER_ID:GNU,Clang,AppleClang>:-Wall -Wextra -Wpedantic -Wconversion> | ||||||
| # Validates Doxygen comments against signatures (clang-only). | ||||||
| $<$<CXX_COMPILER_ID:Clang,AppleClang>:-Wdocumentation> | ||||||
| $<$<CXX_COMPILER_ID:MSVC>:/W4>) | ||||||
|
|
||||||
| # Applied only to our own targets: instrumenting the FetchContent deps just | ||||||
| # slows the build, and TSan still intercepts their pthread-level sync. | ||||||
| add_library(cq_sanitizers INTERFACE) | ||||||
| if(ENABLE_TSAN) | ||||||
| target_compile_options(cq_sanitizers INTERFACE | ||||||
| $<$<CXX_COMPILER_ID:GNU,Clang,AppleClang>:-fsanitize=thread -fno-omit-frame-pointer>) | ||||||
| target_link_options(cq_sanitizers INTERFACE | ||||||
| $<$<CXX_COMPILER_ID:GNU,Clang,AppleClang>:-fsanitize=thread>) | ||||||
| endif() | ||||||
|
|
||||||
| include(FetchContent) | ||||||
| enable_testing() | ||||||
|
|
||||||
| # The tests/ and bench/ sources land in follow-up PRs; each block is a no-op | ||||||
| # until its directory exists so the build scaffolding can merge first. | ||||||
| if(CQ_BUILD_TESTS AND EXISTS "${CMAKE_CURRENT_SOURCE_DIR}/tests/CMakeLists.txt") | ||||||
| FetchContent_Declare( | ||||||
| googletest | ||||||
| URL https://github.com/google/googletest/archive/refs/tags/v1.15.2.tar.gz | ||||||
| URL_HASH SHA256=7b42b4d6ed48810c5362c265a17faebe90dc2373c885e5216439d37927f02926 | ||||||
| DOWNLOAD_EXTRACT_TIMESTAMP TRUE) | ||||||
|
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| set(gtest_force_shared_crt ON CACHE BOOL "" FORCE) | ||||||
| set(INSTALL_GTEST OFF CACHE BOOL "" FORCE) | ||||||
| FetchContent_MakeAvailable(googletest) | ||||||
| add_subdirectory(tests) | ||||||
| endif() | ||||||
|
|
||||||
| if(CQ_BUILD_BENCHMARKS AND NOT ENABLE_TSAN | ||||||
| AND EXISTS "${CMAKE_CURRENT_SOURCE_DIR}/bench/CMakeLists.txt") | ||||||
| FetchContent_Declare( | ||||||
| benchmark | ||||||
| URL https://github.com/google/benchmark/archive/refs/tags/v1.9.1.tar.gz | ||||||
| URL_HASH SHA256=32131c08ee31eeff2c8968d7e874f3cb648034377dfc32a4c377fa8796d84981 | ||||||
| DOWNLOAD_EXTRACT_TIMESTAMP TRUE) | ||||||
| set(BENCHMARK_ENABLE_TESTING OFF CACHE BOOL "" FORCE) | ||||||
| set(BENCHMARK_ENABLE_INSTALL OFF CACHE BOOL "" FORCE) | ||||||
| FetchContent_MakeAvailable(benchmark) | ||||||
| add_subdirectory(bench) | ||||||
| endif() | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| # Style guide | ||
|
|
||
| This project follows the [Google C++ Style Guide](https://google.github.io/styleguide/cppguide.html) | ||
| for formatting and naming, enforced by `.clang-format` and `.clang-tidy` (both | ||
| checked in CI). | ||
|
|
||
| ## Comments | ||
|
|
||
| - **Public API** (everything under `include/cq/`): Doxygen `///` comments on | ||
| every public class and member. Use `@tparam`, `@param` / `@param[out]`, | ||
| `@return`, and `@throws` tags. State the contract: blocking behavior, | ||
| error/close semantics, ownership, and thread-safety. | ||
| - **Internal code** (tests, benchmarks, private members, function bodies): | ||
| plain `//` prose. Explain *why*, not *what*. | ||
| - Tag hygiene is compiler-enforced: clang builds compile with | ||
| `-Wdocumentation`, which rejects `@param` names that do not match the | ||
| signature. Keep comments in sync with code or the build fails. | ||
| - `TODO(username): description` for known follow-ups. | ||
|
|
||
| ## Layout | ||
|
|
||
| - Headers (`.hpp`) declare; template member definitions live in a matching | ||
| `.ipp` included at the bottom of the header. No function bodies in class | ||
| definitions. | ||
| - Special member functions (constructors, copy/move operations, destructor) | ||
| stay grouped at the top of the `public:` section, with a comment explaining | ||
| any deleted operations. | ||
|
|
||
| ## Tooling | ||
|
|
||
| - Format: `clang-format -i` (settings live in `.clang-format`). CI rejects | ||
| unformatted code; format-on-save settings are checked in | ||
| (`.vscode/settings.json`). CI pins clang-format/clang-tidy **18** — use the | ||
| same major version locally or formatting may not match. | ||
| - Lint: `clang-tidy -p <build-dir>` (settings live in `.clang-tidy`; every | ||
| build dir exports `compile_commands.json`). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In-flight CI on main gets cancelled by the next merge. Push events on main all share group
CI-refs/heads/main, so two quick merges cancel the first commit's run — its TSan result and the "last green main" bisect signal are lost.