From patchwork Sat Nov 11 00:44:41 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonathan Wakely X-Patchwork-Id: 164047 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:b129:0:b0:403:3b70:6f57 with SMTP id q9csp1499212vqs; Fri, 10 Nov 2023 17:58:22 -0800 (PST) X-Google-Smtp-Source: AGHT+IEH/6lW6XBiMXn8jmXWRLpmJcqU+E3VDaifLrlqNjqkYy1W8uWPaFh66PI8zv3CdUdH6IfW X-Received: by 2002:a0c:fe03:0:b0:647:406b:4b06 with SMTP id x3-20020a0cfe03000000b00647406b4b06mr932551qvr.57.1699667902205; Fri, 10 Nov 2023 17:58:22 -0800 (PST) ARC-Seal: i=2; a=rsa-sha256; t=1699667902; cv=pass; d=google.com; s=arc-20160816; b=dfmQOyZSPqzVpSy6txD2VirpLiWAROuCZErjAdwMFGCZNDQJOhMPypMFUMfmbkm4/y t9CGjWx0rb/xI4JNlY4xwkICeIfmIGvABiqOHbau6wYLU0t3ITL2AUMoVs8JzcS1u7xb csu1FiTxemi2yFTXJS8v5nna5/uwFtAijclb9Yy3Cn99jMl6N6B1UJhmgMnwZsbFoRhc 9lhBSwNiJOG3b3ixwFpfSkYSIB9nKnPhfiVdWwv7nbYXzs7cJpgsDFwlmKVIFMhw9t7T Muakft+XxY6sW2liDbEv4BWCsV5ch2cZRk/ZcrntE9sNsbJBrUta+Zk89Hr+VFnYBuBi ny4A== ARC-Message-Signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=errors-to:list-subscribe:list-help:list-post:list-archive :list-unsubscribe:list-id:precedence:content-transfer-encoding :mime-version:message-id:date:subject:to:from:dkim-signature :arc-filter:dmarc-filter:delivered-to; bh=JmFJl/K8NHJwhgkyThTarPsaS8qCU0putjh7dovWtGE=; fh=sJ+2/4g29YdyXkoRrFZSpsL2zxijepB7X/1rB0LDDh8=; b=VqaOke+XbSlq9ox+/FqD1KI8DhBOQWT+kxMJ7B+rFm4p9mf+Y0zY3T3E37o5svIGbI VthvqUL7GBEYIpJo8Shg1VlUUtVQZXr0lJvU1/CnIQAfbvKw50fMg/Kfj46syzRpDuUK z7wqor8L40ncyYoRDl3B8n5ahggQyNKj/Di8Ety09JBmfVpiHpHX1xonGap0nAfLp20a bKSALPnHWeMfYgaY64Ga+XEh10BjoJ35h3K9XDu7nIg2U/jlI73Fl7HfhxS+AoXKZtAG lsw/xU62Fwo8kKsT66VH6a8qZn620+rITyTImZYwTKNNx35RXyRMiHaHngVnpeQR4lBC Qi9g== ARC-Authentication-Results: i=2; mx.google.com; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b="DEDhn4X/"; arc=pass (i=1); spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 8.43.85.97 as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com Received: from server2.sourceware.org (server2.sourceware.org. [8.43.85.97]) by mx.google.com with ESMTPS id qr4-20020a05620a390400b0077a4af9271esi648403qkn.294.2023.11.10.17.58.22 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 10 Nov 2023 17:58:22 -0800 (PST) Received-SPF: pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 8.43.85.97 as permitted sender) client-ip=8.43.85.97; Authentication-Results: mx.google.com; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b="DEDhn4X/"; arc=pass (i=1); spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 8.43.85.97 as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 1220338A8152 for ; Sat, 11 Nov 2023 00:45:32 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 4B78C388883A for ; Sat, 11 Nov 2023 00:44:52 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 4B78C388883A Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 4B78C388883A Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.133.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1699663494; cv=none; b=I4qOSZr7oKX9DfKbDJYESyVGRmvKx/6lEBQyri39sRXG5fdBJ5q//FyMhLucggNvlwYfYfNCeLcN1P+KFlKcqF7PaIMo0AvVaTu43lXubJx6JMhZOXBr5sCwjSs65+SdvjIthd7eXM1E9NQ1JwYhqolSIzgXnrT5/JTe33bYx8E= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1699663494; c=relaxed/simple; bh=qXwRLU3/Lj/tRW6C6LTBt2lRRzl+C3/QBnYlByIpdpc=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=pvmg0c0BEFbl/5/OtktakSmDDZuJw/k9Ber93bXxz0A+cgKDl5JEogQVJ+rKmR6jJ8yTkkUd3Kph24YMydcNlyxChspj6sMklYuPVAjSRHEtQc3LdzBmZaXTKqjUIt3pv7oJlqff2wH8Q2PRhopvdYGnK9hXBeP+vDdbQinRoBI= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1699663492; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=JmFJl/K8NHJwhgkyThTarPsaS8qCU0putjh7dovWtGE=; b=DEDhn4X/UXNWzhZ6SQIl9dC7X6T+U/w6RTlctj5JhAG/OzmbquehA6Mgbm3qzyI4Xp9qO4 R93gXlN/ne9SSa5mwD5NRipah+jjDDf8wVh6v9beDBKvfn2jsN00wYktJmYLFfSBpmYmcx 16eHzbjhTnQ9W1aF3btMCs2XtNd1aGo= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-274-88bYzeytMeCol71XYPAxnQ-1; Fri, 10 Nov 2023 19:44:47 -0500 X-MC-Unique: 88bYzeytMeCol71XYPAxnQ-1 Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.rdu2.redhat.com [10.11.54.9]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 4E75E101A529; Sat, 11 Nov 2023 00:44:47 +0000 (UTC) Received: from localhost (unknown [10.42.28.3]) by smtp.corp.redhat.com (Postfix) with ESMTP id 182F6492BE8; Sat, 11 Nov 2023 00:44:46 +0000 (UTC) From: Jonathan Wakely To: libstdc++@gcc.gnu.org, gcc-patches@gcc.gnu.org Subject: [committed] libstdc++: Add [[nodiscard]] to lock types Date: Sat, 11 Nov 2023 00:44:41 +0000 Message-ID: <20231111004446.77907-1-jwakely@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.9 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-13.3 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1782230969991781019 X-GMAIL-MSGID: 1782230969991781019 Tested x86_64-linux. Pushed to trunk. -- >8 -- Adding this attribute means users get a warning when they accidentally create a temporary lock instead of creating an automatic variable with block scope. For std::lock_guard both constructors have side effects (they both take a mutex and so both cause it to be unlocked at the end of the full expression when a temporary is constructed). Ideally we would just put the attribute on the class instead of the constructors, but that doesn't work with GCC (PR c++/85973). For std::unique_lock the default constructor and std::defer_lock_t constructor do not cause any locking or unlocking, so do not need to give a warning. It might still be a mistake to create a temporary using those constructors, but it's harmless and seems unlikely anyway. For a lock object created with one of those constructors you would expect the lock object to be referred to later in the function, and that would not even compile if it was constructed as an unnamed temporary. std::scoped_lock gets the same treatment as std::lock_guard, except that the explicit specialization for zero lockables has no side effects so doesn't need to warn. libstdc++-v3/ChangeLog: * include/bits/std_mutex.h (lock_guard): Add [[nodiscard]] attribute to constructors. * include/bits/unique_lock.h (unique_lock): Likewise. * include/std/mutex (scoped_lock, scoped_lock): Likewise. * testsuite/30_threads/lock_guard/cons/nodiscard.cc: New test. * testsuite/30_threads/scoped_lock/cons/nodiscard.cc: New test. * testsuite/30_threads/unique_lock/cons/nodiscard.cc: New test. --- libstdc++-v3/include/bits/std_mutex.h | 2 + libstdc++-v3/include/bits/unique_lock.h | 5 +++ libstdc++-v3/include/std/mutex | 5 +++ .../30_threads/lock_guard/cons/nodiscard.cc | 20 ++++++++++ .../30_threads/scoped_lock/cons/nodiscard.cc | 29 ++++++++++++++ .../30_threads/unique_lock/cons/nodiscard.cc | 40 +++++++++++++++++++ 6 files changed, 101 insertions(+) create mode 100644 libstdc++-v3/testsuite/30_threads/lock_guard/cons/nodiscard.cc create mode 100644 libstdc++-v3/testsuite/30_threads/scoped_lock/cons/nodiscard.cc create mode 100644 libstdc++-v3/testsuite/30_threads/unique_lock/cons/nodiscard.cc diff --git a/libstdc++-v3/include/bits/std_mutex.h b/libstdc++-v3/include/bits/std_mutex.h index 4693055269d..9ac8c76c9fb 100644 --- a/libstdc++-v3/include/bits/std_mutex.h +++ b/libstdc++-v3/include/bits/std_mutex.h @@ -245,9 +245,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION public: typedef _Mutex mutex_type; + [[__nodiscard__]] explicit lock_guard(mutex_type& __m) : _M_device(__m) { _M_device.lock(); } + [[__nodiscard__]] lock_guard(mutex_type& __m, adopt_lock_t) noexcept : _M_device(__m) { } // calling thread owns mutex diff --git a/libstdc++-v3/include/bits/unique_lock.h b/libstdc++-v3/include/bits/unique_lock.h index c28e6456ad5..07474d26db5 100644 --- a/libstdc++-v3/include/bits/unique_lock.h +++ b/libstdc++-v3/include/bits/unique_lock.h @@ -66,6 +66,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION : _M_device(0), _M_owns(false) { } + [[__nodiscard__]] explicit unique_lock(mutex_type& __m) : _M_device(std::__addressof(__m)), _M_owns(false) { @@ -77,10 +78,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION : _M_device(std::__addressof(__m)), _M_owns(false) { } + [[__nodiscard__]] unique_lock(mutex_type& __m, try_to_lock_t) : _M_device(std::__addressof(__m)), _M_owns(_M_device->try_lock()) { } + [[__nodiscard__]] unique_lock(mutex_type& __m, adopt_lock_t) noexcept : _M_device(std::__addressof(__m)), _M_owns(true) { @@ -88,6 +91,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION } template + [[__nodiscard__]] unique_lock(mutex_type& __m, const chrono::time_point<_Clock, _Duration>& __atime) : _M_device(std::__addressof(__m)), @@ -95,6 +99,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { } template + [[__nodiscard__]] unique_lock(mutex_type& __m, const chrono::duration<_Rep, _Period>& __rtime) : _M_device(std::__addressof(__m)), diff --git a/libstdc++-v3/include/std/mutex b/libstdc++-v3/include/std/mutex index bd3a1cbd94d..9d22ce80045 100644 --- a/libstdc++-v3/include/std/mutex +++ b/libstdc++-v3/include/std/mutex @@ -744,9 +744,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION class scoped_lock { public: + + [[nodiscard]] explicit scoped_lock(_MutexTypes&... __m) : _M_devices(std::tie(__m...)) { std::lock(__m...); } + [[nodiscard]] explicit scoped_lock(adopt_lock_t, _MutexTypes&... __m) noexcept : _M_devices(std::tie(__m...)) { } // calling thread owns mutex @@ -779,9 +782,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION public: using mutex_type = _Mutex; + [[nodiscard]] explicit scoped_lock(mutex_type& __m) : _M_device(__m) { _M_device.lock(); } + [[nodiscard]] explicit scoped_lock(adopt_lock_t, mutex_type& __m) noexcept : _M_device(__m) { } // calling thread owns mutex diff --git a/libstdc++-v3/testsuite/30_threads/lock_guard/cons/nodiscard.cc b/libstdc++-v3/testsuite/30_threads/lock_guard/cons/nodiscard.cc new file mode 100644 index 00000000000..50137f93fde --- /dev/null +++ b/libstdc++-v3/testsuite/30_threads/lock_guard/cons/nodiscard.cc @@ -0,0 +1,20 @@ +// { dg-do compile { target c++11 } } + +#include + +struct Mutex +{ + void lock(); + void unlock(); + bool try_lock(); +}; + +using namespace std; + +void +test_nodiscard(Mutex& m) +{ + lock_guard{m}; // { dg-warning "ignoring return value" } + lock_guard{m, adopt_lock}; // { dg-warning "ignoring return value" } + lock_guard(m, adopt_lock); // { dg-warning "ignoring return value" } +} diff --git a/libstdc++-v3/testsuite/30_threads/scoped_lock/cons/nodiscard.cc b/libstdc++-v3/testsuite/30_threads/scoped_lock/cons/nodiscard.cc new file mode 100644 index 00000000000..ca673e29bda --- /dev/null +++ b/libstdc++-v3/testsuite/30_threads/scoped_lock/cons/nodiscard.cc @@ -0,0 +1,29 @@ +// { dg-do compile { target c++17 } } +// { dg-require-gthreads "" } +// { dg-additional-options "-pthread" { target pthread } } + +#include + +struct Mutex +{ + void lock(); + void unlock(); + bool try_lock(); +}; + +using namespace std; + +void +test_nodiscard(Mutex& m, mutex& m2) +{ + scoped_lock<>{}; // no warning + scoped_lock<>(); // no warning + + scoped_lock{m}; // { dg-warning "ignoring return value" } + scoped_lock{adopt_lock, m}; // { dg-warning "ignoring return value" } + scoped_lock(adopt_lock, m); // { dg-warning "ignoring return value" } + scoped_lock(m, m2); // { dg-warning "ignoring return value" } + scoped_lock{m, m2}; // { dg-warning "ignoring return value" } + scoped_lock{adopt_lock, m, m2}; // { dg-warning "ignoring return value" } + scoped_lock(adopt_lock, m, m2); // { dg-warning "ignoring return value" } +} diff --git a/libstdc++-v3/testsuite/30_threads/unique_lock/cons/nodiscard.cc b/libstdc++-v3/testsuite/30_threads/unique_lock/cons/nodiscard.cc new file mode 100644 index 00000000000..79e347e17d5 --- /dev/null +++ b/libstdc++-v3/testsuite/30_threads/unique_lock/cons/nodiscard.cc @@ -0,0 +1,40 @@ +// { dg-do compile { target c++11 } } + +#include +#include + +using namespace std; + +struct Mutex +{ + void lock(); + void unlock(); + bool try_lock(); + + bool try_lock_for(chrono::seconds); + bool try_lock_until(chrono::system_clock::time_point); +}; + + +void +test_nodiscard(Mutex& m) +{ + unique_lock(); // no warning + unique_lock{}; // no warning + unique_lock(m, defer_lock); // no warning + unique_lock{m, defer_lock}; // no warning + + unique_lock{m}; // { dg-warning "ignoring return value" } + unique_lock{m, try_to_lock}; // { dg-warning "ignoring return value" } + unique_lock(m, try_to_lock); // { dg-warning "ignoring return value" } + unique_lock{m, adopt_lock}; // { dg-warning "ignoring return value" } + unique_lock(m, adopt_lock); // { dg-warning "ignoring return value" } + + chrono::seconds reltime(1); + unique_lock{m, reltime}; // { dg-warning "ignoring return value" } + unique_lock(m, reltime); // { dg-warning "ignoring return value" } + chrono::system_clock::time_point abstime(reltime); + unique_lock{m, abstime}; // { dg-warning "ignoring return value" } + unique_lock(m, abstime); // { dg-warning "ignoring return value" } +} +