Skip to content

Conversation

@philnik777
Copy link
Contributor

@philnik777 philnik777 commented Nov 4, 2024

We're checking quite a few things that are either trivially true or trivially false. These cases became trivial when we changed reserve() to never shrink.

@philnik777 philnik777 marked this pull request as ready for review November 5, 2024 12:54
@philnik777 philnik777 requested a review from a team as a code owner November 5, 2024 12:54
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Nov 5, 2024
@llvmbot
Copy link
Member

llvmbot commented Nov 5, 2024

@llvm/pr-subscribers-libcxx

Author: Nikolas Klauser (philnik777)

Changes

We're checking quite a few things that are either trivially true or trivially false. These cases became trivial when we changed reserve() to never shrink.


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

1 Files Affected:

  • (modified) libcxx/include/string (+3-6)
diff --git a/libcxx/include/string b/libcxx/include/string
index 20e44eaca2ac7a..e4b2d7a8d964c1 100644
--- a/libcxx/include/string
+++ b/libcxx/include/string
@@ -2083,6 +2083,8 @@ private:
     size_type __guess          = __align_it<__boundary>(__s + 1) - 1;
     if (__guess == __min_cap)
       __guess += __endian_factor;
+
+    _LIBCPP_ASSERT_INTERNAL(__guess >= __s, "recommendation is below the requested size");
     return __guess;
   }
 
@@ -3346,12 +3348,7 @@ _LIBCPP_CONSTEXPR_SINCE_CXX20 void basic_string<_CharT, _Traits, _Allocator>::re
   if (__requested_capacity <= capacity())
     return;
 
-  size_type __target_capacity = std::max(__requested_capacity, size());
-  __target_capacity           = __recommend(__target_capacity);
-  if (__target_capacity == capacity())
-    return;
-
-  __shrink_or_extend(__target_capacity);
+  __shrink_or_extend(__recommend(__requested_capacity));
 }
 
 template <class _CharT, class _Traits, class _Allocator>

Copy link
Member

@ldionne ldionne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is NFC, and there's no way to test this, so LGTM if CI passes.

@philnik777 philnik777 merged commit b8f9063 into llvm:main Nov 5, 2024
66 checks passed
@philnik777 philnik777 deleted the simplify_string_reserve branch November 5, 2024 15:05
PhilippRados pushed a commit to PhilippRados/llvm-project that referenced this pull request Nov 6, 2024
We're checking quite a few things that are either trivially true or
trivially false. These cases became trivial when we changed `reserve()`
to never shrink.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants