Skip to content

Address several RooFit issues found by clang-tidy#7642

Merged
linev merged 9 commits intoroot-project:masterfrom
linev:roofit_tidy3
Mar 22, 2021
Merged

Address several RooFit issues found by clang-tidy#7642
linev merged 9 commits intoroot-project:masterfrom
linev:roofit_tidy3

Conversation

@linev
Copy link
Copy Markdown
Member

@linev linev commented Mar 22, 2021

Each commit address its own issue.
Please revise each commit separately and prove that my understanding of code correct.
There is at least one memory leak fix be2464c

linev added 9 commits March 22, 2021 16:24
Delete m_pimpl with non-empty reference counter
And (that is more probable) do not delete container when no other
reference were existing
Only if getRange return kFALSE (which is OK), one can assign
values for histogram binning
It will be nullptr or undefined.
Use value to which it was move
It is not legal syntax to declare array like:

RooLinkedListElem * arr[sz];

Instead use std::vector<RooLinkedListElem *> arr(sz, nullptr);
Have to work on all platforms
@phsft-bot
Copy link
Copy Markdown

Starting build on ROOT-debian10-i386/cxx14, ROOT-performance-centos8-multicore/default, ROOT-fedora30/cxx14, ROOT-fedora31/noimt, ROOT-ubuntu16/nortcxxmod, mac1014/python3, mac11.0/cxx17, windows10/cxx14
How to customize builds

@linev linev requested a review from guitargeek March 22, 2021 15:39
@linev linev linked an issue Mar 22, 2021 that may be closed by this pull request
@linev
Copy link
Copy Markdown
Member Author

linev commented Mar 22, 2021

Last PR, which closes for me #7536

Copy link
Copy Markdown
Contributor

@guitargeek guitargeek left a comment

Choose a reason for hiding this comment

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

Thanks for structuring your fixes in several commits! That made it easy to review and all your fixes look correct to me.

@linev linev merged commit 7fe5b35 into root-project:master Mar 22, 2021
@linev linev deleted the roofit_tidy3 branch March 22, 2021 20:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[RF] Clang-Tidy Clazy Warnings

3 participants