Skip to content

clang-tidy: resolve readability-qualified-auto - #759

Merged
knoepfel merged 2 commits into
Framework-R-D:mainfrom
knoepfel:readability-qualified-auto
Jul 24, 2026
Merged

clang-tidy: resolve readability-qualified-auto#759
knoepfel merged 2 commits into
Framework-R-D:mainfrom
knoepfel:readability-qualified-auto

Conversation

@knoepfel

@knoepfel knoepfel commented Jul 22, 2026

Copy link
Copy Markdown
Member

Resolve readability-qualified-auto clang-tidy warnings.

  • Code quality: Resolved readability-qualified-auto clang-tidy warnings by making pointer and const-pointer type deductions explicit across ROOT storage, framework graph, computational edges, node catalog, and data-layer hierarchy code.
  • Tests: Updated expected error-message assertions in test/adjust_config.cpp for malformed configurations specifying both cpp and py.
  • Documentation: Marked the clang-tidy check as fixed and recorded PR #759 in docs/dev/clang-tidy-fixes-2026-04.md.

knoepfel added a commit to knoepfel/phlex that referenced this pull request Jul 22, 2026
@coderabbitai

coderabbitai Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: c9013979-bc94-4825-b691-ef040641d2aa

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

The change applies explicit pointer and const-qualified local declarations across ROOT storage and phlex graph code, updates three configuration-test expectations, and marks the corresponding clang-tidy check as fixed.

Changes

Clang-tidy pointer and constness cleanup

Layer / File(s) Summary
Explicit pointer and const-qualified locals
form/root_storage/*, phlex/core/*, phlex/model/data_layer_hierarchy.cpp, test/adjust_config.cpp, docs/dev/clang-tidy-fixes-2026-04.md
ROOT and phlex lookup variables use explicit pointer or const-qualified declarations; related error-message assertions and clang-tidy tracking are updated.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

Suggested reviewers: aolivier23

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
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.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: resolving the readability-qualified-auto clang-tidy warning.
✨ 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.

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@form/root_storage/root_tbranch_read_container.cpp`:
- Around line 63-70: Rename the touched local variables to lower_case names: use
dict_info at both sites in form/root_storage/root_tbranch_read_container.cpp
(lines 63-70 and 104), fund_info at line 114 in that file, and dict_info at line
76 in form/root_storage/root_tbranch_write_container.cpp. Update all references
within their respective scopes consistently.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 6a257ace-7606-4ef5-b037-88ff36cdc641

📥 Commits

Reviewing files that changed from the base of the PR and between f4f7e3c and 535379d.

📒 Files selected for processing (8)
  • docs/dev/clang-tidy-fixes-2026-04.md
  • form/root_storage/root_tbranch_read_container.cpp
  • form/root_storage/root_tbranch_write_container.cpp
  • phlex/core/framework_graph.cpp
  • phlex/core/make_computational_edges.cpp
  • phlex/core/node_catalog.cpp
  • phlex/model/data_layer_hierarchy.cpp
  • test/adjust_config.cpp
📜 Review details
⏰ Context from checks skipped due to timeout. (4)
  • GitHub Check: build (gcc, none)
  • GitHub Check: Analyze cpp with CodeQL
  • GitHub Check: coverage
  • GitHub Check: clang-tidy-check
🧰 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:

  • phlex/core/framework_graph.cpp
  • phlex/model/data_layer_hierarchy.cpp
  • phlex/core/node_catalog.cpp
  • form/root_storage/root_tbranch_write_container.cpp
  • phlex/core/make_computational_edges.cpp
  • form/root_storage/root_tbranch_read_container.cpp
  • test/adjust_config.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:

  • phlex/core/framework_graph.cpp
  • phlex/model/data_layer_hierarchy.cpp
  • phlex/core/node_catalog.cpp
  • form/root_storage/root_tbranch_write_container.cpp
  • phlex/core/make_computational_edges.cpp
  • form/root_storage/root_tbranch_read_container.cpp
  • test/adjust_config.cpp
🔇 Additional comments (7)
form/root_storage/root_tbranch_read_container.cpp (1)

163-163: LGTM!

phlex/core/framework_graph.cpp (1)

190-192: LGTM!

Also applies to: 194-194

phlex/core/make_computational_edges.cpp (1)

86-86: LGTM!

Also applies to: 150-150

phlex/core/node_catalog.cpp (1)

31-31: LGTM!

Also applies to: 34-34, 37-37, 40-40, 43-43, 46-46, 65-65

phlex/model/data_layer_hierarchy.cpp (1)

117-117: LGTM!

test/adjust_config.cpp (1)

32-32: LGTM!

Also applies to: 45-45, 57-57

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

160-161: LGTM!

Comment thread form/root_storage/root_tbranch_read_container.cpp
@codecov

codecov Bot commented Jul 22, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 89.47368% with 2 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
phlex/core/node_catalog.cpp 75.00% 0 Missing and 2 partials ⚠️
@@            Coverage Diff             @@
##             main     #759      +/-   ##
==========================================
- Coverage   83.86%   83.81%   -0.06%     
==========================================
  Files         172      172              
  Lines        7203     7203              
  Branches      845      845              
==========================================
- Hits         6041     6037       -4     
- Misses        883      884       +1     
- Partials      279      282       +3     
Flag Coverage Δ
scripts 80.09% <ø> (ø)
unittests 85.68% <89.47%> (-0.09%) ⬇️

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

Files with missing lines Coverage Δ
form/root_storage/root_tbranch_read_container.cpp 87.02% <100.00%> (ø)
form/root_storage/root_tbranch_write_container.cpp 81.96% <100.00%> (ø)
phlex/core/framework_graph.cpp 96.66% <100.00%> (ø)
phlex/core/make_computational_edges.cpp 100.00% <100.00%> (ø)
phlex/model/data_layer_hierarchy.cpp 100.00% <100.00%> (ø)
phlex/core/node_catalog.cpp 88.88% <75.00%> (ø)

... and 3 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 a2cca50...52eda47. Read the comment docs.

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

greenc-FNAL
greenc-FNAL previously approved these changes Jul 23, 2026
@knoepfel
knoepfel merged commit f24e70c into Framework-R-D:main Jul 24, 2026
37 checks passed
@knoepfel
knoepfel deleted the readability-qualified-auto branch July 24, 2026 20:46
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.

2 participants