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

[onert] Apply nodiscard attribute #14711

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ragmani
Copy link
Contributor

@ragmani ragmani commented Feb 19, 2025

This commit applies nodiscard attribute.

ONE-DCO-1.0-Signed-off-by: ragmani [email protected]

This commit applies nodiscard attribute.

ONE-DCO-1.0-Signed-off-by: ragmani <[email protected]>
@ragmani ragmani added the PR/ready for review It is ready to review. Please review it. label Feb 19, 2025
@ragmani ragmani requested a review from a team February 19, 2025 05:53
@glistening
Copy link
Contributor

glistening commented Feb 24, 2025

@ragmani

  1. I don't like [[nodiscard]] appears in almost all (even in APIs). I don't think it is the first-citizen or the most important information in our function.

  2. I searched what is [[nodiscard]]. it seems a helper to give hint to compiler, maybe to get more warnings? I don't know the differences or benefits compare to as-it-is. We already get warnings if we ignores the return values with -Wall or static analyzer. Could you explain what is the benefit of this change?

@ragmani
Copy link
Contributor Author

ragmani commented Feb 24, 2025

@glistening

Thank you for your feedback. Here are some points addressing your concerns:

  1. Why [[nodiscard]]?
    The idea behind [[nodiscard]] is not to make the return value “first-citizen” in every function but to protect against inadvertent mistakes. For functions that return important status codes, pointers, or computed values that could lead to resource leaks or logic errors if ignored, marking them as [[nodiscard]] enforces that the caller must explicitly consider the return value. This is particularly helpful when the function’s result isn’t automatically checked by the caller.

  2. Benefits over –Wall or static analysis:
    While it’s true that some compilers (with -Wall) or static analyzers may warn about unused return values, these warnings are not always issued for all user-defined functions. [[nodiscard]] is a standardized attribute that guarantees a warning in any compliant compiler, making the intent explicit in the code. This attribute improves documentation and communication of function contracts—anyone reading the header immediately sees that the return value should not be discarded.

In summary, the use of [[nodiscard]] improves code safety and clarity by ensuring that critical return values aren’t silently ignored. It serves as an additional layer of defense on top of existing compiler warnings and static analysis tools.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR/ready for review It is ready to review. Please review it.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants