[libcxx-commits] [libcxx] [libc++] Mark string functions as [[nodiscard]] (PR #166524)

[Oliver Stöneberg via libcxx-commits]

================
@@ -1344,24 +1354,26 @@ public:
     return size() == 0;
   }
 
-  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 const_reference operator[](size_type __pos) const _NOEXCEPT {
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 const_reference
+  operator[](size_type __pos) const _NOEXCEPT {
     _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(__pos <= size(), "string index out of bounds");
     if (__builtin_constant_p(__pos) && !__fits_in_sso(__pos)) {
       return *(__get_long_pointer() + __pos);
     }
     return *(data() + __pos);
   }
 
-  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 reference operator[](size_type __pos) _NOEXCEPT {
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 reference
+  operator[](size_type __pos) _NOEXCEPT {
     _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(__pos <= size(), "string index out of bounds");
     if (__builtin_constant_p(__pos) && !__fits_in_sso(__pos)) {
       return *(__get_long_pointer() + __pos);
     }
     return *(__get_pointer() + __pos);
   }
 
-  _LIBCPP_CONSTEXPR_SINCE_CXX20 const_reference at(size_type __n) const;
-  _LIBCPP_CONSTEXPR_SINCE_CXX20 reference at(size_type __n);
+  [[__nodiscard__]] _LIBCPP_CONSTEXPR_SINCE_CXX20 const_reference at(size_type __n) const;
+  [[__nodiscard__]] _LIBCPP_CONSTEXPR_SINCE_CXX20 reference at(size_type __n);
----------------
firewave wrote:

Sorry for the late reply.

> Why would you call this if you don't care about the returned thing?

Because you can and it throws.

I have seen (and worked on code) which added such a plain call as a safe guard against OOB or leveraging the exception handling as an easy way to abort processing.

> There are easier ways to throw than calling `at` with something you know to be out of range.

Aren't you calling it because yo do *not* know it is out of range. And what is easier (not efficient) than a simple function call?

So far I interpreted `[[nodiscard]]` that if you do not use the result you can essentially remove the call (similar to a pure/const function). I think it is similar to the discussion in https://github.com/llvm/llvm-project/pull/172444#discussion_r2622490998.  

Functions with side effects which have been annotated might be worth a clang-tidy check.

https://github.com/llvm/llvm-project/pull/166524