[libcxx-commits] [libcxx] Suppress a redundant hardening check in basic_string_view::substr (PR #91804)

[Konstantin Varlamov via libcxx-commits]

https://github.com/var-const updated https://github.com/llvm/llvm-project/pull/91804

>From f71a3d95657673fa0353b955dc20dca000c16ea7 Mon Sep 17 00:00:00 2001
From: David Benjamin <davidben at google.com>
Date: Fri, 10 May 2024 16:31:11 -0400
Subject: [PATCH 1/4] Suppress a redundant hardening check in
 basic_string_view::substr

Fixes #91634.

This could alternatively be done with an _LIBCPP_ASSUME, after
https://github.com/llvm/llvm-project/pull/91801 lands, but would also
require https://github.com/llvm/llvm-project/issues/91619 be fixed
first. Given the dependencies, it seemed simplest to just make a private
ctor.
---
 libcxx/include/string_view | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/libcxx/include/string_view b/libcxx/include/string_view
index 5c4bec742bafa..2c9703cedfd2a 100644
--- a/libcxx/include/string_view
+++ b/libcxx/include/string_view
@@ -449,8 +449,11 @@ public:
   }
 
   _LIBCPP_CONSTEXPR _LIBCPP_HIDE_FROM_ABI basic_string_view substr(size_type __pos = 0, size_type __n = npos) const {
+    // Use the `__assume_valid` form of the constructor to avoid an unnecessary check. Any substring of a view is a
+    // valid view. In particular, `size()` is known to be smaller than `numeric_limits<difference_type>::max()`, so the
+    // new size is also smaller. See also https://github.com/llvm/llvm-project/issues/91634.
     return __pos > size() ? (__throw_out_of_range("string_view::substr"), basic_string_view())
-                          : basic_string_view(data() + __pos, std::min(__n, size() - __pos));
+                          : basic_string_view(data() + __pos, std::min(__n, size() - __pos), __assume_valid{});
   }
 
   _LIBCPP_CONSTEXPR_SINCE_CXX14 int compare(basic_string_view __sv) const _NOEXCEPT {
@@ -675,6 +678,16 @@ public:
 #endif
 
 private:
+  struct __assume_valid {};
+
+  // This is the same as the pointer and length constructor, but it does not include additionally hardening checks. It
+  // is intended for use within the class, when the class invariants already imply the resulting object is valid. The
+  // compiler usually cannot apply this optimization itself, because it does not know class invariants.
+  _LIBCPP_CONSTEXPR _LIBCPP_HIDE_FROM_ABI
+  basic_string_view(const _CharT* __s, size_type __len, __assume_valid) _NOEXCEPT
+      : __data_(__s),
+        __size_(__len) {}
+
   const value_type* __data_;
   size_type __size_;
 };

>From 350b01e242e7540c7b170920a43294ea51e24fb9 Mon Sep 17 00:00:00 2001
From: Konstantin Varlamov <varconst at apple.com>
Date: Mon, 22 Jul 2024 17:43:13 -0700
Subject: [PATCH 2/4] Slightly tweak a comment

---
 libcxx/include/string_view | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/libcxx/include/string_view b/libcxx/include/string_view
index 2c9703cedfd2a..590271a002bf5 100644
--- a/libcxx/include/string_view
+++ b/libcxx/include/string_view
@@ -680,9 +680,9 @@ public:
 private:
   struct __assume_valid {};
 
-  // This is the same as the pointer and length constructor, but it does not include additionally hardening checks. It
-  // is intended for use within the class, when the class invariants already imply the resulting object is valid. The
-  // compiler usually cannot apply this optimization itself, because it does not know class invariants.
+  // This is the same as the pointer and length constructor, but without the additional hardening checks. It is intended
+  // for use within the class, when the class invariants already guarantee the resulting object is valid. The compiler
+  // usually cannot eliminate the redundant checks because it does not know class invariants.
   _LIBCPP_CONSTEXPR _LIBCPP_HIDE_FROM_ABI
   basic_string_view(const _CharT* __s, size_type __len, __assume_valid) _NOEXCEPT
       : __data_(__s),

>From aafa186173db7e82e2c370ad321cfc7bca0f5db1 Mon Sep 17 00:00:00 2001
From: Konstantin Varlamov <varconst at apple.com>
Date: Mon, 22 Jul 2024 17:44:55 -0700
Subject: [PATCH 3/4] Don't use list initialization to hopefully fix the C++03
 mode

---
 libcxx/include/string_view | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libcxx/include/string_view b/libcxx/include/string_view
index 590271a002bf5..2c2898c170986 100644
--- a/libcxx/include/string_view
+++ b/libcxx/include/string_view
@@ -453,7 +453,7 @@ public:
     // valid view. In particular, `size()` is known to be smaller than `numeric_limits<difference_type>::max()`, so the
     // new size is also smaller. See also https://github.com/llvm/llvm-project/issues/91634.
     return __pos > size() ? (__throw_out_of_range("string_view::substr"), basic_string_view())
-                          : basic_string_view(data() + __pos, std::min(__n, size() - __pos), __assume_valid{});
+                          : basic_string_view(data() + __pos, std::min(__n, size() - __pos), __assume_valid());
   }
 
   _LIBCPP_CONSTEXPR_SINCE_CXX14 int compare(basic_string_view __sv) const _NOEXCEPT {

>From 15e7af78c4b347bc7043975b686c90dceb03ae9b Mon Sep 17 00:00:00 2001
From: Konstantin Varlamov <varconst at apple.com>
Date: Tue, 23 Jul 2024 12:25:50 -0700
Subject: [PATCH 4/4] Feedback

---
 libcxx/include/string_view | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libcxx/include/string_view b/libcxx/include/string_view
index a748bf39150d2..2a03ee99e9ab5 100644
--- a/libcxx/include/string_view
+++ b/libcxx/include/string_view
@@ -452,7 +452,7 @@ public:
     // valid view. In particular, `size()` is known to be smaller than `numeric_limits<difference_type>::max()`, so the
     // new size is also smaller. See also https://github.com/llvm/llvm-project/issues/91634.
     return __pos > size() ? (__throw_out_of_range("string_view::substr"), basic_string_view())
-                          : basic_string_view(data() + __pos, std::min(__n, size() - __pos), __assume_valid());
+                          : basic_string_view(__assume_valid(), data() + __pos, std::min(__n, size() - __pos));
   }
 
   _LIBCPP_CONSTEXPR_SINCE_CXX14 int compare(basic_string_view __sv) const _NOEXCEPT {
@@ -683,7 +683,7 @@ private:
   // for use within the class, when the class invariants already guarantee the resulting object is valid. The compiler
   // usually cannot eliminate the redundant checks because it does not know class invariants.
   _LIBCPP_CONSTEXPR _LIBCPP_HIDE_FROM_ABI
-  basic_string_view(const _CharT* __s, size_type __len, __assume_valid) _NOEXCEPT
+  basic_string_view(__assume_valid, const _CharT* __s, size_type __len) _NOEXCEPT
       : __data_(__s),
         __size_(__len) {}