STYLE: Prefer c++17 [[maybe_unused]] attribute over (void) #4581
+249
−316
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
hjmjohnson
added
the
type:Style
github-actions
bot
added
type:Infrastructure
hjmjohnson
force-pushed
the
prefer-maybe-unused
branch
from
7f8732a
to
017193a
Compare
N-Dekker
requested changes
Apr 12, 2024
Modules/Compatibility/Deprecated/include/itkTreeIteratorBase.hxx
Outdated
Show resolved
Hide resolved
blowekamp
reviewed
Apr 12, 2024
hjmjohnson
force-pushed
the
prefer-maybe-unused
branch
from
017193a
to
d69c780
Compare
github-actions
bot
removed
area:Segmentation
hjmjohnson
force-pushed
the
prefer-maybe-unused
branch
from
d69c780
to
e773540
Compare
hjmjohnson
force-pushed
the
prefer-maybe-unused
branch
from
e749cae
to
1938221
Compare
thewtex
approved these changes
Apr 13, 2024
N-Dekker
requested changes
Apr 15, 2024
Modules/Core/QuadEdgeMesh/test/itkQuadEdgeMeshEulerOperatorCreateCenterVertexTest.cxx
Outdated
Show resolved
Hide resolved
N-Dekker
reviewed
Apr 15, 2024
N-Dekker
reviewed
Apr 15, 2024
N-Dekker
reviewed
Apr 15, 2024
hjmjohnson
force-pushed
the
prefer-maybe-unused
branch
from
1938221
to
3856b89
Compare
N-Dekker
reviewed
Apr 17, 2024
hjmjohnson
force-pushed
the
prefer-maybe-unused
branch
from
3856b89
to
dbeb0f5
Compare
|
@thewtex -- This PR is getting old, and is one that will likely have merge-conflicts. Is this one that can be merged at this point? |
blowekamp
requested changes
Apr 30, 2024
There was a problem hiding this comment.
I noted the two cases I saw, where the void maybe used to prevent a variable from being optimized away. Although I was not able reproduce this scenario, I am hesitant to merged those changes right before the release.
If those two cases were separated into another PR, I think this PR is OK, if @thewtex is OK with it and the release schedual.
|
@blowekamp I'll wait until next month after the release. Hopefully, there won't be too many merge conflicts. |
|
Making a draft until until after 5.4 release is complete. |
|
Thank you for saving this improvement for after v5.4.0 👍 |
|
Isn't |
|
@hjmjohnson v5.4.0 has been tagged -- please merge when you see fit. |
hjmjohnson
force-pushed
the
prefer-maybe-unused
branch
from
dbeb0f5
to
02c188a
Compare
The use of (void)varname was a mechanism to silence compiler warnings prior to universal language support in c++17 for the [[maybe_unused]] attribute specifier.
Function signature variable names are preserved using itkNotUsed macro for documentation purposes.
Prefer to test input values are correct rather than ignore the unused variable.
hjmjohnson
force-pushed
the
prefer-maybe-unused
branch
from
02c188a
to
6582d65
Compare
hjmjohnson
changed the title
Updated comments. |
blowekamp
approved these changes
May 31, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The use of (void)varname was a mechanism to silence compiler warnings prior to universal language support in c++17 for the [[maybe_unused]] attribute specifier.
PR Checklist