Add [[nodiscard]] to cpp #3279
Merged
Add [[nodiscard]] to cpp #3279
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
pepone
approved these changes
Dec 17, 2024
| @@ -43,24 +43,24 @@ namespace IceInternal | |||
| // This is called from the endpoint host resolver thread, so | |||
| // it's safe if this this method blocks. | |||
| // | |||
| virtual NetworkProxyPtr resolveHost(ProtocolSupport) const = 0; | |||
| [[nodiscard]] virtual NetworkProxyPtr resolveHost(ProtocolSupport) const = 0; | |||
There was a problem hiding this comment.
What is the rule for this attribute, we apply it to the initial declaration and to the overrides? It isn't enough to just apply it to the initial declaration?
externl
approved these changes
Dec 17, 2024
| @@ -63,7 +63,7 @@ ConnectionManager::remove(const Ice::ConnectionPtr& connection) | |||
| exception_ptr ex; | |||
| try | |||
| { | |||
| connection->getInfo(); | |||
| [[maybe_unused]] auto _ = connection->getInfo(); | |||
There was a problem hiding this comment.
I'm not following this code. Doesn't getInfo only throw if the connection is closed. Are we relying on this behavior to get the close exception?
There was a problem hiding this comment.
That's apparently what we do, yes. However, I am not following this code either.
InsertCreativityHere
pushed a commit
to InsertCreativityHere/compiler-comparison
that referenced
this pull request
Jan 1, 2025
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.
This PR adds [[nodiscard]] to many C++ API. It's largely an automatic update using clang-tidy -fix.
I disabled modernize-use-nodiscard for a single API: ice_getConnection on proxy, which is often used in our tests to establish a connection without actually retrieving the connection.
The addition of nodiscard also unveiled two bugs (in an IceGrid test and in IcePy). Both fixed by this PR.