Fix a number of clang-tidy detected defects by jblomer · Pull Request #22416 · root-project/root

jblomer · 2026-05-28T12:14:19Z

A test run of clang-tidy on the RNTuple code with the following checks

Checks: 'bugprone-*,-bugprone-assignment-in-if-condition,-bugprone-branch-clone,-bugprone-derived-method-shadowing-base-method,-bugprone-easily-swappable-parameters,-bugprone-implicit-widening-of-multiplication-result,-bugprone-incorrect-enable-if,-bugprone-macro-parentheses,-bugprone-multi-level-implicit-pointer-conversion,-bugprone-narrowing-conversions,-bugprone-not-null-terminated-result,-bugprone-random-generator-seed,-bugprone-reserved-identifier,-bugprone-signed-char-misuse,-bugprone-std-namespace-modification,-bugprone-switch-missing-default-case,-bugprone-throwing-static-initialization,-bugprone-too-small-loop-variable,-bugprone-unchecked-optional-access,-bugprone-unhandled-self-assignment,-bugprone-unused-return-value,-bugprone-virtual-near-miss,cppcoreguidelines-virtual-class-destructor,misc-*,-misc-non-private-member-variables-in-classes,-misc-include-cleaner,-misc-misplaced-const,-misc-multiple-inheritance,-misc-no-recursion,misc-non-copyable-objects,-misc-non-private-member-variables-in-classes,-misc-predictable-rand,-misc-unconventional-assign-operator,-misc-use-anonymous-namespace,performance-*,-performance-enum-size,-performance-inefficient-string-concatenation,-performance-no-int-to-ptr,-performance-noexcept-swap,readability-duplicate-include,readability-delete-null-pointer,readability-misleading-indentation,readability-redundant-control-flow,readability-redundant-declaration,readability-redundant-function-ptr-dereference,readability-reference-to-constructed-temporary'

github-actions · 2026-05-28T16:08:11Z

Test Results

21 files 21 suites 3d 11h 4m 24s ⏱️
3 864 tests 3 566 ✅ 0 💤 298 ❌
73 365 runs 72 513 ✅ 259 💤 593 ❌

For more details on these failures, see this check.

Results for commit 0aaf399.

♻️ This comment has been updated with latest results.

With commit c5ee8b5 (enable unique_ptr support for nested STL collections), the method TClassEdit::GetUniquePtrType() became obsolete.

…rs()

dpiparo

Thanks for these changes! Would it be useful to expand this kind of campaigns to other portions of the codebase, e.g. roofit or RDF, or explore other checks?

pcanal · 2026-06-03T21:44:36Z

   {
      return 0 == name.compare(0, 17, "std::__pair_base<") || 0 == name.compare(0, 12, "__pair_base<");
   }
-   inline std::string GetUniquePtrType(std::string_view name)


TClassEdit is a public interface and thus this 'could' be used outside of ROOT. Should we deprecated it instead of removing it?

pcanal

LGTM.

ferdymercury · 2026-06-04T09:46:47Z

Thanks for this, it's great!

Thanks for these changes! Would it be useful to expand this kind of campaigns to other portions of the codebase, e.g. roofit or RDF, or explore other checks?

Yes. Maybe even sth like:
https://github.com/HorstBaerbel/action-clang-tidy
or
https://github.com/marketplace/actions/clang-tidy-review
https://github.com/marketplace/actions/c-c-linter

hahnjo · 2026-06-04T12:38:12Z

+   RException(const RException &other) noexcept : std::runtime_error(other)
+   {
+      try {
+         fError = other.fError;


This could throw because of std::string and std::vector in RError? Maybe add a comment, and a mention in the commit message why exceptions should be nothrow copy constructible.

Also, I think implementing the copy constructor deletes the move constructor and assignment operators, so they must be implemented explicitly (rule of five).

hahnjo · 2026-06-04T12:46:03Z

@@ -147,9 +147,9 @@ public:
   static RError ForwardError(RResultBase &&result, RError::RLocation &&sourceLocation)


Should these methods also take const ref?

hahnjo · 2026-06-04T12:46:23Z

 {
   // Avoid frequent reallocations as we move up the call stack
   fStackTrace.reserve(32);
   AddFrame(std::move(sourceLocation));


Hm, is this moving a const ref? 🤔

jblomer · 2026-06-05T12:02:38Z

Thanks for this, it's great!

Thanks for these changes! Would it be useful to expand this kind of campaigns to other portions of the codebase, e.g. roofit or RDF, or explore other checks?

Yes. Maybe even sth like: https://github.com/HorstBaerbel/action-clang-tidy or https://github.com/marketplace/actions/clang-tidy-review https://github.com/marketplace/actions/c-c-linter

Maybe. I think the next step would be a regular clang-tidy check for RNTuple as part of the CI. Looking into it.

jblomer self-assigned this May 28, 2026

jblomer requested review from dpiparo and pcanal as code owners May 28, 2026 12:14

jblomer force-pushed the tidy branch from 8b910fd to 0aaf399 Compare June 2, 2026 11:44

jblomer added 25 commits June 2, 2026 13:45

[ntuple] fix argument comment

e65b5ff

[io] fix casting through void

e6c96eb

[RVec] avoid unnecessary capacity check in internal set_size

3aaf0d1

[ntuple] avoid throwing impossible exception

2b12921

[ntuple] don't throw in RNTupleModel::~RUpdater()

54cdde1

[ntuple] safer use of RException

a816682

[core] make RException nothrow copy constructible

97682c7

[ntuple] fix throwing exception in RNTupleDescriptor

81f3304

[ntuple] remove wrong forward declaration of RFieldVisitor

9c70bae

[core] fix definition of kMaxULong

6135634

[ntuple] merger: minor improvement in optional assignment

76674f5

[meta] remove unused TClassEdit::GetUniquePtrType()

3a03db1

With commit c5ee8b5 (enable unique_ptr support for nested STL collections), the method TClassEdit::GetUniquePtrType() became obsolete.

[ntuple] minor readability improvement

b0cd742

[ntuple] minor improvement in RPageSourceFile::CreateFromAnchor()

907d881

[ntuple] remove unused variable in RFieldBase::Create()

ed040e0

[ntuple] fix use-after-move in RNTupleDescriptorBuilder::AddColumn()

7b2802a

[NFC][ntuple] suppress clang-tidy false positive

2c710b8

[ntuple] fix widening method visibility change

41b2b3a

[ntuple] remove unusued usings

b295c6f

[ntuple] move RAuxiliaryProcessorField to anonymous namespace

6cef18b

[ntuple] move RKeyBlob to anonymous namespace

fa17ecb

[ntuple] private linkage of stream operator in the merger

57d1eb5

[ntuple] replace std::endl by \n

7351ad8

[ntuple] minor performance improvement in RPageSourceFile::LoadCluste…

c888811

…rs()

[core] pass RError::RLocation by const ref

d946abb

jblomer added 12 commits June 2, 2026 13:45

[ntuple] use member move in RValue move ctor

ce0d076

[ntuple] minor perf improvement in type name normalization

d8b27c0

[ntuple] make moving RPage(Ref) noexcept

ee3279e

[ntuple] avoid some unnecessary value copies

643beb4

[core] remove unnecessary declaration in RResult

0891899

[ntuple] mark some move constructors noexcept

a1461a4

[core] make moving TTaskGroup noexcept

46c6e41

[RVec] improve noexcept specification

12a194f

[ntuple] fix virtual destructor visibility

3ee662f

[ntuple] fix up RNTupleDescriptorBuilder::SetNTuple() signature

18a1fc3

[ntuple] remove duplicate include

a60d5bc

[core] minor improvement in TIterator

0aaf399

dpiparo approved these changes Jun 3, 2026

View reviewed changes

pcanal reviewed Jun 3, 2026

View reviewed changes

pcanal approved these changes Jun 3, 2026

View reviewed changes

hahnjo self-requested a review June 4, 2026 11:32

hahnjo reviewed Jun 4, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix a number of clang-tidy detected defects#22416

Fix a number of clang-tidy detected defects#22416
jblomer wants to merge 37 commits into
root-project:masterfrom
jblomer:tidy

jblomer commented May 28, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented May 28, 2026 •

edited

Loading

Uh oh!

dpiparo left a comment

Uh oh!

pcanal Jun 3, 2026

Uh oh!

pcanal left a comment

Uh oh!

ferdymercury commented Jun 4, 2026

Uh oh!

hahnjo Jun 4, 2026

Uh oh!

hahnjo Jun 4, 2026

Uh oh!

hahnjo Jun 4, 2026

Uh oh!

jblomer commented Jun 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

		@@ -147,9 +147,9 @@ public:
		static RError ForwardError(RResultBase &&result, RError::RLocation &&sourceLocation)

Conversation

jblomer commented May 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented May 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results

Uh oh!

dpiparo left a comment

Choose a reason for hiding this comment

Uh oh!

pcanal Jun 3, 2026

Choose a reason for hiding this comment

Uh oh!

pcanal left a comment

Choose a reason for hiding this comment

Uh oh!

ferdymercury commented Jun 4, 2026

Uh oh!

hahnjo Jun 4, 2026

Choose a reason for hiding this comment

Uh oh!

hahnjo Jun 4, 2026

Choose a reason for hiding this comment

Uh oh!

hahnjo Jun 4, 2026

Choose a reason for hiding this comment

Uh oh!

jblomer commented Jun 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

jblomer commented May 28, 2026 •

edited

Loading

github-actions Bot commented May 28, 2026 •

edited

Loading