Skip to content

COMP: Silence nodiscard warnings for Future.get() in PoolMultiThreader#5945

Closed
hjmjohnson wants to merge 1 commit intoInsightSoftwareConsortium:mainfrom
hjmjohnson:fix-nodiscard-pool-multithreader
Closed

COMP: Silence nodiscard warnings for Future.get() in PoolMultiThreader#5945
hjmjohnson wants to merge 1 commit intoInsightSoftwareConsortium:mainfrom
hjmjohnson:fix-nodiscard-pool-multithreader

Conversation

@hjmjohnson
Copy link
Copy Markdown
Member

@hjmjohnson hjmjohnson commented Mar 13, 2026

Summary

  • Cast Future.get() return values to (void) in itkPoolMultiThreader.cxx where the call is only used for its side effect of waiting for thread completion and propagating exceptions

Fixes:

ITK/Modules/Core/Common/src/itkPoolMultiThreader.cxx:148:55: warning:
  ignoring return value of function declared with 'nodiscard'
  attribute [-Wunused-result]
    exceptionHandler.TryAndCatch([this, threadLoop] { m_ThreadInfoArray[threadLoop].Future.get(); });
                                                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
ITK/Modules/Core/Common/src/itkPoolMultiThreader.cxx:303:11: warning:
  ignoring return value of function declared with 'nodiscard'
  attribute [-Wunused-result]
          m_ThreadInfoArray[i].Future.get();
          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Test plan

  • CI builds clean with no -Wunused-result warnings
  • No functional change — (void) cast only suppresses the warning

🤖 Generated with Claude Code

@github-actions github-actions bot added type:Compiler Compiler support or related warnings area:Core Issues affecting the Core module labels Mar 13, 2026
@hjmjohnson hjmjohnson force-pushed the fix-nodiscard-pool-multithreader branch from 82e1b97 to fcbbe4e Compare March 13, 2026 18:48
@hjmjohnson hjmjohnson requested a review from dzenanz March 13, 2026 18:49
@hjmjohnson hjmjohnson force-pushed the fix-nodiscard-pool-multithreader branch from fcbbe4e to 2f3d4e1 Compare March 13, 2026 22:35
@hjmjohnson @claude
COMP: Silence nodiscard warnings for Future.get() in PoolMultiThreader
32c6b1d 
Cast Future.get() return values to void where the call is only used
for its side effect of waiting for thread completion and propagating
exceptions.

Fixes the following warnings:
```
ITK/Modules/Core/Common/src/itkPoolMultiThreader.cxx:148:55: warning:
  ignoring return value of function declared with 'nodiscard'
  attribute [-Wunused-result]
ITK/Modules/Core/Common/src/itkPoolMultiThreader.cxx:303:11: warning:
  ignoring return value of function declared with 'nodiscard'
  attribute [-Wunused-result]

This is for clang-22 compilers.
```

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@hjmjohnson hjmjohnson force-pushed the fix-nodiscard-pool-multithreader branch from 2f3d4e1 to 32c6b1d Compare March 13, 2026 23:48
N-Dekker
N-Dekker reviewed Mar 14, 2026
View reviewed changes
Modules/Core/Common/src/itkPoolMultiThreader.cxx
for (threadLoop = 1; threadLoop < m_NumberOfWorkUnits; ++threadLoop)
{
exceptionHandler.TryAndCatch([this, threadLoop] { m_ThreadInfoArray[threadLoop].Future.get(); });
exceptionHandler.TryAndCatch([this, threadLoop] { (void)m_ThreadInfoArray[threadLoop].Future.get(); });
Copy link
Copy Markdown
Contributor

@N-Dekker N-Dekker Mar 14, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

What does Future.get() return? And why is it OK to ignore its return value?

I'm asking, because maybe we should use the std::future<void> specialization, for Future: https://en.cppreference.com/w/cpp/thread/future/get.html
std::future<void>::get() returns void. It looks like that's what we want, right?

In general, I think it's preferable to avoid unnecessary casting 🤷

Copy link
Copy Markdown
Member Author

@hjmjohnson hjmjohnson Mar 14, 2026

Choose a reason for hiding this comment

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

std::future<ITK_THREAD_RETURN_TYPE> Future;

ITK_THREAD_RETURN_TYPE depends on the platform.

I don't understand what you are suggesting. Could you please make an alternate proposal?

Copy link
Copy Markdown
Contributor

@N-Dekker N-Dekker Mar 15, 2026

Choose a reason for hiding this comment

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

Ah, I see:

ITK_THREAD_RETURN_TYPE is a pointer type when using Pthreads:

ITK/Modules/Core/Common/include/itkThreadSupport.h

Line 47 in badec43

using ITK_THREAD_RETURN_TYPE = void *;

And it's unsigned int when using WIN32 threads:

ITK/Modules/Core/Common/include/itkThreadSupport.h

Line 62 in badec43

using ITK_THREAD_RETURN_TYPE = unsigned int;

But "normally" it's just void:

ITK/Modules/Core/Common/include/itkThreadSupport.h

Line 77 in badec43

using ITK_THREAD_RETURN_TYPE = void;

Are Pthreads and WIN32 threads both still actively supported nowadays?

N-Dekker
N-Dekker approved these changes Mar 15, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@N-Dekker N-Dekker left a comment

Choose a reason for hiding this comment

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

Could you please make an alternate proposal?

Please wait, I'll give it a try.

@N-Dekker N-Dekker self-requested a review March 15, 2026 10:39
N-Dekker
N-Dekker requested changes Mar 15, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@N-Dekker N-Dekker left a comment

Choose a reason for hiding this comment

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

Please wait, I'll try an alternative.

N-Dekker added a commit to N-Dekker/ITK that referenced this pull request Mar 15, 2026
@N-Dekker
STYLE: Let Future.get() from PoolMultiThreader always return void
1dabb9f 
`Future` was declared as `std::future<ITK_THREAD_RETURN_TYPE>`, which is
platform specific, and caused warnings on some platforms, as reported by Hans
Johnson at InsightSoftwareConsortium#5945 :

    itkPoolMultiThreader.cxx:148:55: warning:
      ignoring return value of function declared with 'nodiscard'
      attribute [-Wunused-result]
        exceptionHandler.TryAndCatch([this, threadLoop] { m_ThreadInfoArray[threadLoop].Future.get(); });
                                                          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Addressed by using `std::future<void>` instead.
N-Dekker added a commit to N-Dekker/ITK that referenced this pull request Mar 15, 2026
@N-Dekker
COMP: Let Future.get() calls in PoolMultiThreader always return void
a22aedb 
`Future` was declared as `std::future<ITK_THREAD_RETURN_TYPE>`, which is
platform specific, and caused warnings on some platforms, as reported by Hans
Johnson at InsightSoftwareConsortium#5945 :

    itkPoolMultiThreader.cxx:148:55: warning:
      ignoring return value of function declared with 'nodiscard'
      attribute [-Wunused-result]
        exceptionHandler.TryAndCatch([this, threadLoop] { m_ThreadInfoArray[threadLoop].Future.get(); });
                                                          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Addressed by using `std::future<void>` instead.
@N-Dekker N-Dekker mentioned this pull request Mar 15, 2026
Merged
N-Dekker added a commit to N-Dekker/ITK that referenced this pull request Mar 15, 2026
@N-Dekker
COMP: Let Future.get() calls in PoolMultiThreader always return void
af0c4e3 
`Future` was declared as `std::future<ITK_THREAD_RETURN_TYPE>`, which is
platform specific, and caused warnings on some platforms, as reported by Hans
Johnson at InsightSoftwareConsortium#5945 :

    itkPoolMultiThreader.cxx:148:55: warning:
      ignoring return value of function declared with 'nodiscard'
      attribute [-Wunused-result]
        exceptionHandler.TryAndCatch([this, threadLoop] { m_ThreadInfoArray[threadLoop].Future.get(); });
                                                          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Addressed by replacing the public `ThreadPoolInfoStruct` with a private
`InternalWorkUnitInfo` struct, which has its `Future` declared as
`std::future<void>`.
N-Dekker added a commit to N-Dekker/ITK that referenced this pull request Mar 15, 2026
@N-Dekker
COMP: Let Future.get() calls in PoolMultiThreader always return void
fd7c78b 
`Future` was declared as `std::future<ITK_THREAD_RETURN_TYPE>`, which is
platform specific, and caused warnings on some platforms, as reported by Hans
Johnson at InsightSoftwareConsortium#5945 :

    itkPoolMultiThreader.cxx:148:55: warning:
      ignoring return value of function declared with 'nodiscard'
      attribute [-Wunused-result]
        exceptionHandler.TryAndCatch([this, threadLoop] { m_ThreadInfoArray[threadLoop].Future.get(); });
                                                          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Addressed by replacing the public `ThreadPoolInfoStruct` with a private
`InternalWorkUnitInfo` struct, which has its `Future` declared as
`std::future<void>`.
@hjmjohnson
Copy link
Copy Markdown
Member Author

hjmjohnson commented Mar 16, 2026

Superceeded by

@hjmjohnson hjmjohnson closed this Mar 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@N-Dekker N-Dekker N-Dekker requested changes

@dzenanz dzenanz dzenanz approved these changes

Assignees

No one assigned

Labels

area:Core Issues affecting the Core module type:Compiler Compiler support or related warnings

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@hjmjohnson @dzenanz @N-Dekker