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

Mark expected as [[nodiscard]] #74

Closed
wants to merge 1 commit into from

Conversation

Quuxplusone
Copy link
Contributor

Since it holds either a result or an error (that is, it always might hold an error), an expected return value should never be blindly discarded.

At least that's my take on it: https://quuxplusone.github.io/blog/2024/12/08/should-expected-be-nodiscard/

boost::outcome_v2::result<T> and llvm::Expected<T> are already marked nodiscard. No vendor's std::expected is marked yet, but the /r/cpp chatter re my above post makes it sound like both STL and jwakely are at least considering it now.

IMHO this is a Very Good Idea, but if you as the project owner think it's not (or not yet), then feel free to close it.

Since it holds either a result or an error (that is, it always might hold
an error), an `expected` return value should never be blindly discarded.
martinmoene added a commit that referenced this pull request Dec 10, 2024
martinmoene added a commit that referenced this pull request Dec 10, 2024
expected.t.cpp(1347): warning C4834: discarding return value of function with 'nodiscard' attribute
@Quuxplusone
Copy link
Contributor Author

Awesome, looks like I can close this. :) FYI, Microsoft STL marked theirs last week in microsoft/STL@7643c27 .

@Quuxplusone Quuxplusone deleted the nodiscard branch December 15, 2024 18:30
@martinmoene
Copy link
Owner

martinmoene commented Dec 17, 2024

Thanks, see that exceptions such as bad_any_cast also are candidates to mark as [[nodiscard]] in the nonstd lite series.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants