Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Backport][TMVA] Fix unused variable warnings #13836

Merged

Conversation

dpiparo
Copy link
Member

@dpiparo dpiparo commented Oct 10, 2023

BP of #13835 . The usage of the annotation is possible since for 6.30 cpp17 is enforced.

to fix warnings due to an unused variable in non-debug builds.
@dpiparo dpiparo self-assigned this Oct 10, 2023
@phsft-bot
Copy link
Collaborator

Starting build on ROOT-performance-centos8-multicore/soversion, ROOT-ubuntu2204/nortcxxmod, ROOT-ubuntu2004/python3, mac11/noimt, mac12arm/cxx20, windows10/default
How to customize builds

@dpiparo dpiparo changed the title [Backport] Fix tmva warnings v2 v630 [Backport][TMVA] Fix unused variable warnings Oct 10, 2023
@@ -196,7 +196,7 @@ class TCpuTensor : public TMVA::Experimental::RTensor<AFloat, TCpuBuffer<AFloat>
//this will be an unsafe view. Method exists for backwards compatibility only
TCpuMatrix<AFloat> GetMatrix() const
{
size_t ndims = 0;
[[maybe_unused]] size_t ndims = 0;
Copy link
Member

Choose a reason for hiding this comment

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

I think the warning is actually correct, running the loop be low is pointless if the assert is off. I would consider using:

assert( [&shape]() { int ndims; for (auto& shape_i : shape) { if (shape_i != 1) { ndims++; } }; return ndims; } () <= 2 && shape.size() > 1); // to support shape cases {n,1}

(alternative use a [[maybe_unused]] variable for the lambda).

Copy link
Member

@pcanal pcanal left a comment

Choose a reason for hiding this comment

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

see comment.

@github-actions
Copy link

Test Results

         9 files           9 suites   2d 2h 53m 7s ⏱️
  2 483 tests   2 482 ✔️ 0 💤 1
21 620 runs  21 618 ✔️ 0 💤 2

For more details on these failures, see this check.

Results for commit 9794fa3.

@dpiparo dpiparo merged commit 89ca26d into root-project:v6-30-00-patches Oct 10, 2023
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.

3 participants