Skip to content

Force enable assertions for clang-tidy - #12635

Open
Jacobfaib wants to merge 1 commit into
NVIDIA:mainfrom
Jacobfaib:jacobf/2026-11-13/clang-tidy-enable-assertions
Open

Jacobfaib wants to merge 1 commit into
NVIDIA:mainfrom
Jacobfaib:jacobf/2026-11-13/clang-tidy-enable-assertions

Conversation

@Jacobfaib

@Jacobfaib Jacobfaib commented Nov 13, 2026 •

Copy link
Copy Markdown
Contributor

Description

Even though we run clang-tidy in the debug preset, some targets (like benchmarks) force-enable optimization flags or otherwise disable assertions. Since clang-tidy isn't running any code, we should make sure it sees all code regardless of optimization level. This should help reduce false positives around _CCCL_ASSERT() and friends.

Checklist

  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@Jacobfaib Jacobfaib self-assigned this Nov 13, 2026
@Jacobfaib
Jacobfaib requested a review from a team as a code owner November 13, 2026 16:12
@Jacobfaib
Jacobfaib requested a review from davebayer November 13, 2026 16:12
@github-project-automation github-project-automation Bot moved this to Todo in CCCL Nov 13, 2026
@cccl-authenticator-app cccl-authenticator-app Bot moved this from Todo to In Review in CCCL Nov 13, 2026
@coderabbitai

coderabbitai Bot commented Nov 13, 2026

Copy link
Copy Markdown
Contributor

Explain Change →

No actionable comments were generated in the recent review.

Summary

Summary by CodeRabbit

  • Chores
    • Updated static analysis to run with assertions enabled, so its checks now account for assertion-related code paths. This change affects development checks only; it does not alter application behavior or add user-facing features.

The .clang-tidy configuration and CMake clang-tidy invocation now pass CCCL_ENABLE_ASSERTIONS=1 as a preprocessor definition.

Changes: Clang-tidy assertions

Layer / File(s) Summary
Add the assertions definition
.clang-tidy, cmake/run_clang_tidy.sh.in
Both clang-tidy configurations pass CCCL_ENABLE_ASSERTIONS=1 as a preprocessor definition.

Suggested reviewers: bernhardmgruber

Change: Other

Merge Risk: Minimal · up to c6bf2

This change only affects static analysis configuration and does not alter runtime behavior of the library. It is low risk to merge, though CI clang-tidy results should be watched in case the newly enabled assertion code surfaces warnings.

Architecture risk: Low · up to c6bf2

The change affects 1 system.

Changed systems: cmake

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — cmake (service) was modified; 1 changed file maps to changed impact.

Before / after behavior

  • observed — Modified behavior in cmake/run_clang_tidy.sh.in: The clang-tidy invocation adds the CCCL_ENABLE_ASSERTIONS=1 preprocessor definition.
  • observed — Modified behavior in .clang-tidy: Adds CCCL_ENABLE_ASSERTIONS=1 as a preprocessor definition in ExtraArgsBefore.
- [ ] Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Recent review info

Run configuration

  • Source mode: Auto
  • Repository: NVIDIA/cccl
  • Branch: main
  • File: .coderabbit.yaml
  • Commit: 038308b
  • Layers: Repository YAML
  • Review profile: CHILL
  • Plan: Enterprise
  • Run ID: aebecca9-7826-4187-8915-3c3163ade357
Commits

Reviewing files that changed from the base of the PR and between 038308b and c6bf265.

Files selected for processing: 2

  • .clang-tidy
  • cmake/run_clang_tidy.sh.in

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 10 remain after this review.

@github-actions

This comment has been minimized.

@Jacobfaib
Jacobfaib enabled auto-merge (squash) November 13, 2026 18:26
@github-actions

Copy link
Copy Markdown
Contributor

🥳 CI Workflow Results

🟩 Finished in 2h 19m: Pass: 100%/502 | Total: 4d 09h | Max: 1h 22m | Hits: 95%/968775

See results here.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In Review

Development

Successfully merging this pull request may close these issues.

2 participants