Skip to content

clang-tidy: resolve readability-redundant-control-flow - #743

Merged
knoepfel merged 2 commits into
Framework-R-D:mainfrom
knoepfel:readability-redundant-control-flow
Jul 22, 2026
Merged

clang-tidy: resolve readability-redundant-control-flow#743
knoepfel merged 2 commits into
Framework-R-D:mainfrom
knoepfel:readability-redundant-control-flow

Conversation

@knoepfel

@knoepfel knoepfel commented Jul 21, 2026

Copy link
Copy Markdown
Member
  • Code quality: Resolved readability-redundant-control-flow clang-tidy findings by removing redundant and unreachable return statements across persistence, storage, and ROOT storage writers/readers.
  • Behavior: Converted stub methods in Storage_Write_Container to explicit no-op implementations without changing signatures or exception behavior.
  • Documentation: Marked the clang-tidy finding as fixed and linked PR #743 in the developer checklist.
  • API/tests: No public API declarations or tests were changed.

@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 2cadaf2f-1d63-4951-b5e7-f093d3e7f982

📥 Commits

Reviewing files that changed from the base of the PR and between 799bb5a and 680553a.

📒 Files selected for processing (8)
  • docs/dev/clang-tidy-fixes-2026-04.md
  • form/persistence/persistence_reader.cpp
  • form/persistence/persistence_writer.cpp
  • form/root_storage/root_tbranch_read_container.cpp
  • form/root_storage/root_tbranch_write_container.cpp
  • form/root_storage/root_ttree_write_container.cpp
  • form/storage/storage_write_container.cpp
  • form/storage/storage_writer.cpp
💤 Files with no reviewable changes (6)
  • form/root_storage/root_tbranch_read_container.cpp
  • form/storage/storage_writer.cpp
  • form/persistence/persistence_writer.cpp
  • form/persistence/persistence_reader.cpp
  • form/root_storage/root_tbranch_write_container.cpp
  • form/root_storage/root_ttree_write_container.cpp
📜 Recent review details
🧰 Additional context used
📓 Path-based instructions (3)
**/*.md

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.md: All Markdown files must follow markdownlint rule MD012: no multiple consecutive blank lines (never more than one blank line in a row)
All Markdown files must follow markdownlint rule MD022: headings must be surrounded by exactly one blank line before and after
All Markdown files must follow markdownlint rule MD031: fenced code blocks must be surrounded by exactly one blank line before and after
All Markdown files must follow markdownlint rule MD032: lists must be surrounded by exactly one blank line before and after (including after headings and code blocks)
All Markdown files must follow markdownlint rule MD034: no bare URLs (use markdown link syntax like [text](destination) instead of plain URLs)
All Markdown files must follow markdownlint rule MD036: use # headings for titles, not Bold:
All Markdown files must follow markdownlint rule MD040: always specify code block language (for example, use 'bash', 'python', '```text', etc.)

**/*.md: Do not use multiple consecutive blank lines in Markdown (MD012)
Surround Markdown headings with exactly one blank line (MD022)
Surround Markdown fenced code blocks with exactly one blank line (MD031)
Surround Markdown lists with exactly one blank line (MD032)
Do not use bare URLs in Markdown; use [text](url) syntax instead (MD034)
Use # headings in Markdown, not **Bold** for section titles (MD036)
Always specify language on fenced code blocks in Markdown (MD040)

Files:

  • docs/dev/clang-tidy-fixes-2026-04.md
**/*.{cpp,cc,cxx,h,hpp}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.{cpp,cc,cxx,h,hpp}: Use clang-format tool for all C++ code formatting (VS Code auto-formats on save); configuration defined in .clang-format with 100-character line limit and 2-space indentation
Follow clang-tidy recommendations defined in .clang-tidy

Files:

  • form/storage/storage_write_container.cpp
**/*.{hpp,cpp}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{hpp,cpp}: Use .hpp for header files, .cpp for implementation, and *_test.cpp for test files in C++
Enforce 100-character line limit and 2-space indentation in C++ code via .clang-format
Use QualifierAlignment: Right (east-const) style: int const x not const int x in C++
Use PointerAlignment: Left in C++ (pointer * attached to type, not variable name)
All C++ identifiers must use lower_case naming: namespaces, classes, structs, enums, functions, variables, parameters, members, and constants
Exception to C++ naming: template parameters use CamelCase
Exception to C++ naming: macros use UPPER_CASE
Private, protected, and constant members in C++ must have a trailing underscore (_), no trailing underscore on anything else
Use enum class preferred over plain enum in C++
Use std::shared_ptr for shared ownership, std::unique_ptr for exclusive ownership, raw pointers for non-owning references only in C++
Use functors with agent-noun pattern: ModelEvaluator evaluate_model(...) in C++
Apply .clang-tidy checks for bugprone, cert, clang-analyzer, concurrency, cppcoreguidelines, misc, modernize, performance, portability, and readability as defined in the .clang-tidy configuration file
Use phlex:: namespace for core code, phlex::experimental:: for experimental features in C++

Files:

  • form/storage/storage_write_container.cpp
🔇 Additional comments (2)
form/storage/storage_write_container.cpp (1)

17-21: LGTM!

docs/dev/clang-tidy-fixes-2026-04.md (1)

157-158: LGTM!


📝 Walkthrough

Walkthrough

Changes

Redundant control-flow cleanup

Layer / File(s) Summary
Persistence and storage exit cleanup
form/persistence/*, form/storage/*
Removed redundant trailing returns and converted storage stubs to explicit no-op bodies without changing call behavior.
ROOT container exit cleanup
form/root_storage/*
Removed redundant or unreachable returns from ROOT branch and tree container methods.
Clang-tidy completion tracking
docs/dev/clang-tidy-fixes-2026-04.md
Marked the readability-redundant-control-flow checklist item complete and added the PR reference.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Suggested reviewers: wwuoneway, greenc-fnal

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the PR’s main change: resolving the clang-tidy readability-redundant-control-flow warning.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@codecov

codecov Bot commented Jul 21, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

@@            Coverage Diff             @@
##             main     #743      +/-   ##
==========================================
- Coverage   83.85%   83.80%   -0.05%     
==========================================
  Files         172      172              
  Lines        7198     7183      -15     
  Branches      845      845              
==========================================
- Hits         6036     6020      -16     
- Misses        882      883       +1     
  Partials      280      280              
Flag Coverage Δ
scripts 80.13% <ø> (ø)
unittests 85.66% <100.00%> (-0.07%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
form/persistence/persistence_reader.cpp 90.62% <ø> (-0.29%) ⬇️
form/persistence/persistence_writer.cpp 100.00% <ø> (ø)
form/root_storage/root_tbranch_read_container.cpp 87.02% <ø> (-0.10%) ⬇️
form/root_storage/root_tbranch_write_container.cpp 81.96% <ø> (-1.37%) ⬇️
form/root_storage/root_ttree_write_container.cpp 54.54% <ø> (-3.79%) ⬇️
form/storage/storage_write_container.cpp 100.00% <100.00%> (ø)
form/storage/storage_writer.cpp 87.93% <ø> (-0.60%) ⬇️

... and 4 files with indirect coverage changes


Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 799bb5a...680553a. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@knoepfel

Copy link
Copy Markdown
Member Author

@CodeRabbit review

@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@aolivier23

Copy link
Copy Markdown
Contributor

Found this when poking around to see what's new. I'd be happy to write an approving review as a FORM developer when you're ready :)

@knoepfel

Copy link
Copy Markdown
Member Author

Found this when poking around to see what's new. I'd be happy to write an approving review as a FORM developer when you're ready :)

Thanks, @aolivier23. I'm ready :)

@aolivier23 aolivier23 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.

Looks good to me. Thank @knoepfel !

@knoepfel
knoepfel merged commit 62bf9f2 into Framework-R-D:main Jul 22, 2026
37 checks passed
@knoepfel
knoepfel deleted the readability-redundant-control-flow branch July 22, 2026 13:23
Comment thread form/storage/storage_write_container.cpp
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants