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

scoped_lock constructor could be nodiscard like in libstdc++ #89388

Closed
steakhal opened this issue Apr 19, 2024 · 1 comment · Fixed by #89397
Closed

scoped_lock constructor could be nodiscard like in libstdc++ #89388

steakhal opened this issue Apr 19, 2024 · 1 comment · Fixed by #89397
Assignees
Labels
enhancement Improving things as opposed to bug fixing, e.g. new or missing feature libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.

Comments

@steakhal
Copy link
Contributor

Consider this code:

#include <mutex>

int num;
std::mutex m;
void top() {
    if (std::scoped_lock{m}; num > 10) { // no-warning :(
        num = 1;
    }
    if (std::lock_guard{m}; num > 11) { // warn: nodiscard
        num = 2;
    }
}

The constructor of std::scoped_lock should have the [[nodiscard]] attribute to catch mistakes like the one in the example, creating a temporary scoped_lock object and leaving the access of the global variable unguarded.

If we use libstdc++, we get the following warning ignoring temporary created by a constructor declared with 'nodiscard' attribute [-Wunused-value]. Maybe we could also have this nodiscard attribute too.

Note that for instance lock_guard works as expected, and has the ``nodiscard` attribute.

@steakhal steakhal added enhancement Improving things as opposed to bug fixing, e.g. new or missing feature libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. labels Apr 19, 2024
@ldionne
Copy link
Member

ldionne commented Apr 19, 2024

I'm surprised we don't already do this. This is definitely an oversight, I think.

@ldionne ldionne self-assigned this Apr 19, 2024
ldionne added a commit to ldionne/llvm-project that referenced this issue Apr 19, 2024
It's basically always a bug to discard a scoped_lock.

Fixes llvm#89388
ldionne added a commit to ldionne/llvm-project that referenced this issue Apr 25, 2024
It's basically always a bug to discard a scoped_lock.

Fixes llvm#89388
ldionne added a commit that referenced this issue Apr 29, 2024
…]] (#89397)

It's basically always a bug to discard a scoped_lock or a unique_lock.

Fixes #89388
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improving things as opposed to bug fixing, e.g. new or missing feature libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants