Skip to content

readability-else-after-return - #12439

Merged
Jacobfaib merged 4 commits into
NVIDIA:mainfrom
Jacobfaib:jacobf/2026-11-04/readability-else-after-return
Nov 10, 2026
Merged

Jacobfaib merged 4 commits into
NVIDIA:mainfrom
Jacobfaib:jacobf/2026-11-04/readability-else-after-return

Conversation

@Jacobfaib

Copy link
Copy Markdown
Contributor

Fixes all clang-tidy warnings for readability-else-after-return

@Jacobfaib Jacobfaib self-assigned this Nov 4, 2026
@github-project-automation github-project-automation Bot moved this to Todo in CCCL Nov 4, 2026
@copy-pr-bot

copy-pr-bot Bot commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@cccl-authenticator-app cccl-authenticator-app Bot moved this from Todo to In Progress in CCCL Nov 4, 2026
@Jacobfaib

Copy link
Copy Markdown
Contributor Author

/ok to test

@github-actions

This comment has been minimized.

@Jacobfaib

Copy link
Copy Markdown
Contributor Author

/ok to test

@github-actions

This comment has been minimized.

@Jacobfaib

Copy link
Copy Markdown
Contributor Author

/ok to test

@github-actions

This comment has been minimized.

@Jacobfaib
Jacobfaib marked this pull request as ready for review November 6, 2026 17:44
@Jacobfaib
Jacobfaib requested review from a team as code owners November 6, 2026 17:44
@cccl-authenticator-app cccl-authenticator-app Bot moved this from In Progress to In Review in CCCL Nov 6, 2026
@coderabbitai

coderabbitai Bot commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

Explain Change →

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • Resume reviews
  • Trigger review

No actionable comments were generated in the recent review.

Summary

Summary by CodeRabbit

  • Bug Fixes

    • Corrected sorting behavior when insertion sorting completes successfully on a partition.
    • Improved handling of empty parallel reductions and zero-item test cases.
    • Standardized NaN results in floating-point rounding and refined floating-point boundary handling.
    • Updated floating-point boundary test cases.
  • Refactor

    • Simplified conditional control flow across algorithms, examples, and utilities; most changes preserve existing behavior.
    • Enabled an additional readability check, with targeted suppressions where needed.

The PR enables readability-else-after-return and updates control flow across CCCL. Most edits remove redundant else branches or add targeted lint suppressions. The vsmem empty-input path and the __introsort partition path also change behavior.

Changes: Control-flow readability updates

Layer / File(s) Summary
Enable the check and retain selected else branches
.clang-tidy, libcudacxx/include/cuda/__cmath/ceil_div.h, libcudacxx/include/cuda/__memory/ptr_in_range.h, libcudacxx/include/cuda/__memory/ranges_overlap.h, libcudacxx/include/cuda/std/__memory/uninitialized_algorithms.h
The configuration enables readability-else-after-return. Selected existing else branches receive suppression comments.
Update CUB algorithms, dispatch, and tests
c/parallel/test/*, c2h/include/c2h/*, cub/benchmarks/*, cub/cub/*, cub/test/*
Branch and return structures are simplified across CUB algorithms, dispatch policies, and test helpers. The vsmem test dispatch now returns success with zero temporary-storage bytes for zero items, whether or not the storage pointer is null.
Update libcudacxx control flow
libcudacxx/include/cuda/*, libcudacxx/include/cuda/std/*
Branch and fallback paths are simplified in libcudacxx. In __introsort, the left-partition success path now advances past the pivot and continues when the right-partition check succeeds.
Update Thrust numerical and algorithm implementations
nvbench_helper/*, thrust/thrust/detail/complex/*, thrust/thrust/system/*
Control flow is simplified in numeric routines and system algorithms. The TBB reduction returns init for empty input before creating a reduction body.
Update examples and test support
cudax/examples/stf/*, cudax/include/cuda/experimental/__stf/*, cudax/test/*, libcudacxx/benchmarks/*, libcudacxx/test/*, thrust/benchmarks/*, thrust/examples/*, thrust/testing/*
Examples, benchmark helpers, and tests replace redundant else branches with direct returns or sequential checks.

Change: Refactor

Merge Risk: Minimal · previous review 525e2

The inspected control-flow changes preserve the affected behavior. No issue identified here needs to be resolved before merging.

Architecture risk: Low · up to 525e2

The change affects 7 systems.

Changed systems: libcudacxx, thrust, cub, cudax, c, c2h, nvbench_helper

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — libcudacxx (service) was modified; 34 changed files map to changed impact.
  • observed — thrust (service) was modified; 24 changed files map to changed impact.
  • observed — cub (service) was modified; 19 changed files map to changed impact.
  • observed — cudax (service) was modified; 8 changed files map to changed impact.

Before / after behavior

  • observed — Modified behavior in c/parallel/test/test_reduce.cpp: reduce_run::operator() retains the nondeterministic call for CCCL_NOT_GUARANTEED and moves the deterministic cccl_device_reduce call outside the else branch, so it runs when that condition is false.
  • observed — Modified behavior in c/parallel/test/test_scan.cpp: scan_run::operator() now directly returns the exclusive scan call when inclusive is false; the removed else branch previously contained the same call.
  • observed — Modified behavior in c/parallel/test/test_scan.cpp: scan_run_future_value::operator() now directly returns the future-value exclusive scan call when inclusive is false; the removed else branch previously contained the same call.
  • observed — Modified behavior in c2h/include/c2h/test_util_vec.h: In the 2-component vector operator>, the second-component comparison is now an independent if rather than an else if.
- [ ] 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: 5060e9b
  • Layers: Repository YAML
  • Review profile: CHILL
  • Plan: Enterprise
  • Run ID: 2d820f0a-872a-44c5-b517-a67afbb08942
Commits

Reviewing files that changed from the base of the PR and between ad7d09c and 525e299.

Files selected for processing: 20

  • .clang-tidy
  • cub/benchmarks/bench/transform/applications/P1/pytorch.cu
  • cub/cub/block/block_reduce.cuh
  • cub/test/test_util.h
  • cudax/examples/stf/graph_algorithms/jaccard.cu
  • cudax/include/cuda/experimental/__stf/utility/memory.cuh
  • libcudacxx/include/cuda/__container/simple_vector.h
  • libcudacxx/include/cuda/__event/event_ref.h
  • libcudacxx/include/cuda/std/__chrono/hh_mm_ss.h
  • libcudacxx/include/cuda/std/__expected/expected.h
  • libcudacxx/include/cuda/std/__mdspan/layout_stride.h
  • libcudacxx/include/cuda/std/__optional/optional_ref.h
  • thrust/thrust/detail/complex/ccosh.h
  • thrust/thrust/detail/complex/ccoshf.h
  • thrust/thrust/detail/complex/cexp.h
  • thrust/thrust/detail/complex/cexpf.h
  • thrust/thrust/detail/complex/csinh.h
  • thrust/thrust/detail/complex/csinhf.h
  • thrust/thrust/detail/complex/csqrt.h
  • thrust/thrust/detail/complex/csqrtf.h
Files with no reviewable changes: 1

  • .clang-tidy

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

@fbusato fbusato left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm slightly in favor of that because it reduces the LoC. On the other hand, we need to add a comment for every if constexpr, _CCCL_IF_CONSTEVAL

@Jacobfaib

Copy link
Copy Markdown
Contributor Author

we need to add a comment for every if constexpr, _CCCL_IF_CONSTEVAL

if constexpr no, clang-tidy specifically knows about this and won't emit diagnostics. _CCCL_IF_CONSTEVAL, currently yes, but I think I can work around it by defining it in terms of if constexpr until we require C++20.

@Jacobfaib
Jacobfaib force-pushed the jacobf/2026-11-04/readability-else-after-return branch from 6736fe8 to cc7239e Compare November 6, 2026 18:45
@Jacobfaib
Jacobfaib force-pushed the jacobf/2026-11-04/readability-else-after-return branch from cc7239e to ad7d09c Compare November 9, 2026 13:15
@Jacobfaib
Jacobfaib enabled auto-merge (squash) November 9, 2026 13:32
@Jacobfaib

Copy link
Copy Markdown
Contributor Author

/ok to test

@github-actions

This comment has been minimized.

@Jacobfaib
Jacobfaib force-pushed the jacobf/2026-11-04/readability-else-after-return branch 2 times, most recently from 525e299 to c315139 Compare November 10, 2026 14:57
@Jacobfaib
Jacobfaib force-pushed the jacobf/2026-11-04/readability-else-after-return branch from c315139 to 013e788 Compare November 10, 2026 17:38
@Jacobfaib
Jacobfaib merged commit ea5cfb8 into NVIDIA:main Nov 10, 2026
479 checks passed
@github-project-automation github-project-automation Bot moved this from In Review to Done in CCCL Nov 10, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🥳 CI Workflow Results

🟩 Finished in 4h 02m: Pass: 100%/459 | Total: 22d 19h | Max: 4h 02m | Hits: 23%/5618413

See results here.

@Jacobfaib
Jacobfaib deleted the jacobf/2026-11-04/readability-else-after-return branch November 10, 2026 21:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

5 participants