[libc++] Remove [[nodiscard]] from map etc. operator[] #172444

zmodem · 2025-12-16T09:20:19Z

This was added in #169971 and related PRs. As commented there, discarding the return value on operator[] for these types does not typically indicate a bug since the operator can also be used to insert a default value.

llvmbot · 2025-12-16T09:20:40Z

@llvm/pr-subscribers-libcxx

Author: Hans Wennborg (zmodem)

Changes

This was added in #169971 and related PRs. As commented there, discarding the return value on operator[] for these types does not typically indicate a bug since the operator can also be used to insert a default value.

Full diff: https://github.com/llvm/llvm-project/pull/172444.diff

6 Files Affected:

(modified) libcxx/include/__flat_map/flat_map.h (+3-3)
(modified) libcxx/include/map (+2-2)
(modified) libcxx/include/unordered_map (+2-2)
(modified) libcxx/test/libcxx/diagnostics/flat_map.nodiscard.verify.cpp (+3-3)
(modified) libcxx/test/libcxx/diagnostics/map.nodiscard.verify.cpp (+2-2)
(modified) libcxx/test/libcxx/diagnostics/unordered_map.nodiscard.verify.cpp (+2-2)

diff --git a/libcxx/include/__flat_map/flat_map.h b/libcxx/include/__flat_map/flat_map.h
index 84b60cdc9ae27..4cd938b54cbf4 100644
--- a/libcxx/include/__flat_map/flat_map.h
+++ b/libcxx/include/__flat_map/flat_map.h
@@ -465,13 +465,13 @@ class flat_map {
   }
 
   // [flat.map.access], element access
-  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX26 mapped_type& operator[](const key_type& __x)
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX26 mapped_type& operator[](const key_type& __x)
     requires is_constructible_v<mapped_type>
   {
     return try_emplace(__x).first->second;
   }
 
-  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX26 mapped_type& operator[](key_type&& __x)
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX26 mapped_type& operator[](key_type&& __x)
     requires is_constructible_v<mapped_type>
   {
     return try_emplace(std::move(__x)).first->second;
@@ -480,7 +480,7 @@ class flat_map {
   template <class _Kp>
     requires(__is_compare_transparent && is_constructible_v<key_type, _Kp> && is_constructible_v<mapped_type> &&
              !is_convertible_v<_Kp &&, const_iterator> && !is_convertible_v<_Kp &&, iterator>)
-  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX26 mapped_type& operator[](_Kp&& __x) {
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX26 mapped_type& operator[](_Kp&& __x) {
     return try_emplace(std::forward<_Kp>(__x)).first->second;
   }
 
diff --git a/libcxx/include/map b/libcxx/include/map
index 6414c35a0799a..03c92e152e04f 100644
--- a/libcxx/include/map
+++ b/libcxx/include/map
@@ -1092,9 +1092,9 @@ public:
   [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI size_type size() const _NOEXCEPT { return __tree_.size(); }
   [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI size_type max_size() const _NOEXCEPT { return __tree_.max_size(); }
 
-  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI mapped_type& operator[](const key_type& __k);
+  _LIBCPP_HIDE_FROM_ABI mapped_type& operator[](const key_type& __k);
 #  ifndef _LIBCPP_CXX03_LANG
-  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI mapped_type& operator[](key_type&& __k);
+  _LIBCPP_HIDE_FROM_ABI mapped_type& operator[](key_type&& __k);
 #  endif
 
   template <class _Arg,
diff --git a/libcxx/include/unordered_map b/libcxx/include/unordered_map
index 5df57b5cedb1f..ca53348eb5e2a 100644
--- a/libcxx/include/unordered_map
+++ b/libcxx/include/unordered_map
@@ -1262,9 +1262,9 @@ public:
   }
 #  endif // _LIBCPP_STD_VER >= 20
 
-  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI mapped_type& operator[](const key_type& __k);
+  _LIBCPP_HIDE_FROM_ABI mapped_type& operator[](const key_type& __k);
 #  ifndef _LIBCPP_CXX03_LANG
-  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI mapped_type& operator[](key_type&& __k);
+  _LIBCPP_HIDE_FROM_ABI mapped_type& operator[](key_type&& __k);
 #  endif
 
   [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI mapped_type& at(const key_type& __k);
diff --git a/libcxx/test/libcxx/diagnostics/flat_map.nodiscard.verify.cpp b/libcxx/test/libcxx/diagnostics/flat_map.nodiscard.verify.cpp
index 7d75083157aef..d2000b66147db 100644
--- a/libcxx/test/libcxx/diagnostics/flat_map.nodiscard.verify.cpp
+++ b/libcxx/test/libcxx/diagnostics/flat_map.nodiscard.verify.cpp
@@ -66,9 +66,9 @@ void test() {
   TransparentKey<int> tkey;
 
   std::flat_map<int, int> nfm;
-  nfm[key];            // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
-  fm[std::move(key)];  // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
-  fm[std::move(tkey)]; // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  nfm[key];            // no-warning
+  fm[std::move(key)];  // no-warning
+  fm[std::move(tkey)]; // no-warning
 
   fm.at(key);   // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
   cfm.at(key);  // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
diff --git a/libcxx/test/libcxx/diagnostics/map.nodiscard.verify.cpp b/libcxx/test/libcxx/diagnostics/map.nodiscard.verify.cpp
index ea110c35c03dd..20218b50e6c60 100644
--- a/libcxx/test/libcxx/diagnostics/map.nodiscard.verify.cpp
+++ b/libcxx/test/libcxx/diagnostics/map.nodiscard.verify.cpp
@@ -55,8 +55,8 @@ void test() {
 
   int key = 0;
 
-  m[key];            // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
-  m[std::move(key)]; // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  m[key];            // no-warning
+  m[std::move(key)]; // no-warning
 
 #if TEST_STD_VER >= 14
   std::map<std::string, int, std::less<>> strMap;
diff --git a/libcxx/test/libcxx/diagnostics/unordered_map.nodiscard.verify.cpp b/libcxx/test/libcxx/diagnostics/unordered_map.nodiscard.verify.cpp
index 064b7a2fb444c..0747556905200 100644
--- a/libcxx/test/libcxx/diagnostics/unordered_map.nodiscard.verify.cpp
+++ b/libcxx/test/libcxx/diagnostics/unordered_map.nodiscard.verify.cpp
@@ -81,8 +81,8 @@ void test() {
   ctm.equal_range(tkey); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
 #endif
 
-  m[key];            // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
-  m[std::move(key)]; // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  m[key];            // no-warning
+  m[std::move(key)]; // no-warning
 
   m.at(key);  // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
   cm.at(key); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}

frederick-vs-ja · 2025-12-16T09:29:13Z

libcxx/include/__flat_map/flat_map.h


  // [flat.map.access], element access
-  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX26 mapped_type& operator[](const key_type& __x)
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX26 mapped_type& operator[](const key_type& __x)


I'm not sure whether value-discarding m[k] is intended. If one only wants the side effects, m.try_emplace(k) can be used instead to express this more clearly.

For map and unordered_map, m[k] is possibly a workaround because try_emplace wasn't present before C++17. So I just want to keep [[nodiscard]] for flat_map::operator[].

It sounds like we're on the same page about map and unordered_map.

I don't feel particularly strongly about flat_map since we don't have many uses, but I think we should strive for consistency between the interfaces.

https://libcxx.llvm.org/CodingGuidelines.html#apply-nodiscard-where-relevant says nodiscard should be used

where discarding the return value is most likely a correctness issue. For example a locking constructor in unique_lock.

I don't think nodiscard on these operator[]'s meets the criterion of "most likely a correctness issue". Of the instances I've seen so far, none have been a correctness issue. Do you have any data that suggests otherwise?

I think you could interpret where discarding the value is most likely a misuse of the function to apply here, but that's not exactly the spirit. I do think that there is value in pointing people to APIs which better suit their use-case, but I don't think that's currently captured in the guidelines.

I agree that there's value in pointing people to newer APIs, but I think that falls in a different category of warnings than what nodiscard is used for, which is to flag likely bugs.

philnik777

I'm definitely in favour of removing them from map/unordered_map. I think I agree with @frederick-vs-ja's assessment to keep the annotations on flat_map, since it is new. However, I do think that a message that try_emplace can be used instead would be good there.

We could consider adding [[nodiscard]] to map/unordered_map with a message later as well, but we'd definitely have to guard that behind C++17 and possibly make it opt-in, or at the very least easy to opt-out. Anyways, this isn't something we should do now but consider at some point in the future if at all.

TL;DR IMO we can keep the [[nodiscard]] in flat_map if we add a message, but I'm happy for that to not be this patch. LGTM.

zmodem · 2025-12-16T11:57:59Z

Thanks! I'll go ahead and land this for now.

Fixed in llvm/llvm-project#172444.

[libc++] Remove [[nodiscard]] from map etc. operator[]

7c5c0aa

zmodem requested review from Zingam and frederick-vs-ja December 16, 2025 09:20

zmodem requested a review from a team as a code owner December 16, 2025 09:20

zmodem added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Dec 16, 2025

zmodem mentioned this pull request Dec 16, 2025

[libc++][map] Applied [[nodiscard]] #169971

Merged

frederick-vs-ja reviewed Dec 16, 2025

View reviewed changes

philnik777 approved these changes Dec 16, 2025

View reviewed changes

zmodem merged commit c3ebcfe into llvm:main Dec 16, 2025
82 checks passed

jtstogel mentioned this pull request Dec 16, 2025

[RDF] Use try_emplace instead of operator[] to avoid nodiscard error. #172531

Closed

hebasto mentioned this pull request Dec 17, 2025

clang-snapshot: Add -Wno-error=unused-result when using libc++ hebasto/bitcoin-core-nightly#106

Merged

hebasto added a commit to hebasto/bitcoin-core-nightly that referenced this pull request Dec 18, 2025

clang-snapshot: Drop -Wno-error=unused-result when using libc++

0450ba1

Fixed in llvm/llvm-project#172444.

hebasto mentioned this pull request Dec 18, 2025

clang-snapshot: Drop -Wno-error=unused-result when using libc++ hebasto/bitcoin-core-nightly#113

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[libc++] Remove [[nodiscard]] from map etc. operator[] #172444

[libc++] Remove [[nodiscard]] from map etc. operator[] #172444

Uh oh!

zmodem commented Dec 16, 2025

Uh oh!

llvmbot commented Dec 16, 2025

Uh oh!

frederick-vs-ja Dec 16, 2025

Uh oh!

zmodem Dec 16, 2025

Uh oh!

philnik777 Dec 16, 2025

Uh oh!

zmodem Dec 16, 2025

Uh oh!

philnik777 left a comment

Uh oh!

zmodem commented Dec 16, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

[libc++] Remove [[nodiscard]] from map etc. operator[] #172444

[libc++] Remove [[nodiscard]] from map etc. operator[] #172444

Uh oh!

Conversation

zmodem commented Dec 16, 2025

Uh oh!

llvmbot commented Dec 16, 2025

Uh oh!

frederick-vs-ja Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

zmodem Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

philnik777 Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

zmodem Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

philnik777 left a comment

Choose a reason for hiding this comment

Uh oh!

zmodem commented Dec 16, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants