[libc++] Introduce a standalone __scope_guard and use it in <string> #114867
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
the
libc++
|
@llvm/pr-subscribers-libcxx Author: Nikolas Klauser (philnik777) ChangesThis introduces a new Full diff: https://github.com/llvm/llvm-project/pull/114867.diff 4 Files Affected:
diff --git a/libcxx/include/__flat_map/flat_map.h b/libcxx/include/__flat_map/flat_map.h
index 5c14c0ac693b08..076c14f64018af 100644
--- a/libcxx/include/__flat_map/flat_map.h
+++ b/libcxx/include/__flat_map/flat_map.h
@@ -50,7 +50,7 @@
#include <__type_traits/is_nothrow_constructible.h>
#include <__type_traits/is_same.h>
#include <__type_traits/maybe_const.h>
-#include <__utility/exception_guard.h>
+#include <__utility/scope_guard.h>
#include <__utility/pair.h>
#include <initializer_list>
#include <stdexcept>
diff --git a/libcxx/include/__utility/exception_guard.h b/libcxx/include/__utility/exception_guard.h
index 00b835d3e2a2fc..a03bd7e8f35227 100644
--- a/libcxx/include/__utility/exception_guard.h
+++ b/libcxx/include/__utility/exception_guard.h
@@ -137,12 +137,6 @@ _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR __exception_guard<_Rollback> __make_exce
return __exception_guard<_Rollback>(std::move(__rollback));
}
-template <class _Rollback>
-_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 __exception_guard_exceptions<_Rollback>
-__make_scope_guard(_Rollback __rollback) {
- return __exception_guard_exceptions<_Rollback>(std::move(__rollback));
-}
-
_LIBCPP_END_NAMESPACE_STD
_LIBCPP_POP_MACROS
diff --git a/libcxx/include/__utility/scope_guard.h b/libcxx/include/__utility/scope_guard.h
new file mode 100644
index 00000000000000..865ced71ffc44c
--- /dev/null
+++ b/libcxx/include/__utility/scope_guard.h
@@ -0,0 +1,46 @@
+// -*- C++ -*-
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef _LIBCPP___UTILITY_SCOPE_GUARD_H
+#define _LIBCPP___UTILITY_SCOPE_GUARD_H
+
+#include <__config>
+#include <__utility/move.h>
+
+#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
+# pragma GCC system_header
+#endif
+
+_LIBCPP_PUSH_MACROS
+#include <__undef_macros>
+
+_LIBCPP_BEGIN_NAMESPACE_STD
+
+template <class _Func>
+class __scope_guard {
+ _Func __func_;
+
+public:
+ _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR __scope_guard(_Func __func) : __func_(std::move(__func)) {}
+ _LIBCPP_HIDE_FROM_ABI ~__scope_guard() { __func_(); }
+
+ __scope_guard(const __scope_guard&) = delete;
+ __scope_guard& operator=(const __scope_guard&) = delete;
+};
+
+template <class _Func>
+__scope_guard<_Func> __make_scope_guard(_Func __func) {
+ return __scope_guard<_Func>(std::move(__func));
+}
+
+_LIBCPP_END_NAMESPACE_STD
+
+_LIBCPP_POP_MACROS
+
+#endif // _LIBCPP___UTILITY_SCOPE_GUARD_H
diff --git a/libcxx/include/string b/libcxx/include/string
index 20e44eaca2ac7a..caea2707fbd479 100644
--- a/libcxx/include/string
+++ b/libcxx/include/string
@@ -638,6 +638,7 @@ basic_string<char32_t> operator""s( const char32_t *str, size_t len );
#include <__utility/forward.h>
#include <__utility/is_pointer_in_range.h>
#include <__utility/move.h>
+#include <__utility/scope_guard.h>
#include <__utility/swap.h>
#include <__utility/unreachable.h>
#include <climits>
@@ -929,6 +930,13 @@ private:
_LIBCPP_COMPRESSED_PAIR(__rep, __rep_, allocator_type, __alloc_);
+ // annotate the string with its size() at scope exit. The string has to be in a valid state at that point.
+ struct __annotate_new_size {
+ basic_string& __str_;
+
+ void operator()() { __str_.__annotate_new(__str_.size()); }
+ };
+
// Construct a string with the given allocator and enough storage to hold `__size` characters, but
// don't initialize the characters. The contents of the string, including the null terminator, must be
// initialized separately.
@@ -2169,6 +2177,7 @@ private:
__alloc_ = __str.__alloc_;
} else {
__annotate_delete();
+ auto __guard = std::__make_scope_guard(__annotate_new_size(*this));
allocator_type __a = __str.__alloc_;
auto __allocation = std::__allocate_at_least(__a, __str.__get_long_cap());
__begin_lifetime(__allocation.ptr, __allocation.count);
@@ -2178,7 +2187,6 @@ private:
__set_long_pointer(__allocation.ptr);
__set_long_cap(__allocation.count);
__set_long_size(__str.size());
- __annotate_new(__get_long_size());
}
}
}
@@ -2506,6 +2514,7 @@ _LIBCPP_CONSTEXPR_SINCE_CXX20 void basic_string<_CharT, _Traits, _Allocator>::__
size_type __cap =
__old_cap < __ms / 2 - __alignment ? __recommend(std::max(__old_cap + __delta_cap, 2 * __old_cap)) : __ms - 1;
__annotate_delete();
+ auto __guard = std::__make_scope_guard(__annotate_new_size(*this));
auto __allocation = std::__allocate_at_least(__alloc_, __cap + 1);
pointer __p = __allocation.ptr;
__begin_lifetime(__p, __allocation.count);
@@ -2524,7 +2533,6 @@ _LIBCPP_CONSTEXPR_SINCE_CXX20 void basic_string<_CharT, _Traits, _Allocator>::__
__old_sz = __n_copy + __n_add + __sec_cp_sz;
__set_long_size(__old_sz);
traits_type::assign(__p[__old_sz], value_type());
- __annotate_new(__old_sz);
}
// __grow_by is deprecated because it does not set the size. It may not update the size when the size is changed, and it
@@ -2548,7 +2556,6 @@ _LIBCPP_DEPRECATED_("use __grow_by_without_replace") basic_string<_CharT, _Trait
pointer __old_p = __get_pointer();
size_type __cap =
__old_cap < __ms / 2 - __alignment ? __recommend(std::max(__old_cap + __delta_cap, 2 * __old_cap)) : __ms - 1;
- __annotate_delete();
auto __allocation = std::__allocate_at_least(__alloc_, __cap + 1);
pointer __p = __allocation.ptr;
__begin_lifetime(__p, __allocation.count);
@@ -2573,11 +2580,12 @@ basic_string<_CharT, _Traits, _Allocator>::__grow_by_without_replace(
size_type __n_copy,
size_type __n_del,
size_type __n_add) {
+ __annotate_delete();
+ auto __guard = std::__make_scope_guard(__annotate_new_size(*this));
_LIBCPP_SUPPRESS_DEPRECATED_PUSH
__grow_by(__old_cap, __delta_cap, __old_sz, __n_copy, __n_del, __n_add);
_LIBCPP_SUPPRESS_DEPRECATED_POP
__set_long_size(__old_sz - __n_del + __n_add);
- __annotate_new(__old_sz - __n_del + __n_add);
}
// assign
@@ -3367,6 +3375,7 @@ template <class _CharT, class _Traits, class _Allocator>
inline _LIBCPP_CONSTEXPR_SINCE_CXX20 void
basic_string<_CharT, _Traits, _Allocator>::__shrink_or_extend(size_type __target_capacity) {
__annotate_delete();
+ auto __guard = std::__make_scope_guard(__annotate_new_size(*this));
size_type __cap = capacity();
size_type __sz = size();
@@ -3398,7 +3407,6 @@ basic_string<_CharT, _Traits, _Allocator>::__shrink_or_extend(size_type __target
// due to swapping the elements.
if (__allocation.count - 1 > __target_capacity) {
__alloc_traits::deallocate(__alloc_, __allocation.ptr, __allocation.count);
- __annotate_new(__sz); // Undoes the __annotate_delete()
return;
}
__new_data = __allocation.ptr;
@@ -3423,7 +3431,6 @@ basic_string<_CharT, _Traits, _Allocator>::__shrink_or_extend(size_type __target
__set_long_pointer(__new_data);
} else
__set_short_size(__sz);
- __annotate_new(__sz);
}
template <class _CharT, class _Traits, class _Allocator>
|
You can test this locally with the following command:git-clang-format --diff 4c4db3c943d686ff7c1fcf2dbc975e8462497efe 2eed91b18b058bd0f2904777030ffe3ebc649dc3 --extensions ,h -- libcxx/include/__utility/scope_guard.h libcxx/include/__flat_map/flat_map.h libcxx/include/__utility/exception_guard.h libcxx/include/stringView the diff from clang-format here.diff --git a/libcxx/include/__utility/scope_guard.h b/libcxx/include/__utility/scope_guard.h
index 3685c531fd..133e54212e 100644
--- a/libcxx/include/__utility/scope_guard.h
+++ b/libcxx/include/__utility/scope_guard.h
@@ -32,7 +32,7 @@ public:
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR __scope_guard(_Func __func) : __func_(std::move(__func)) {}
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 ~__scope_guard() { __func_(); }
- __scope_guard(const __scope_guard&) = delete;
+ __scope_guard(const __scope_guard&) = delete;
// C++17 has mandatory RVO, so we don't need the move constructor anymore to make __make_scope_guard work.
#if _LIBCPP_STD_VER <= 14
@@ -45,7 +45,7 @@ public:
#endif
__scope_guard& operator=(const __scope_guard&) = delete;
- __scope_guard& operator=(__scope_guard&&) = delete;
+ __scope_guard& operator=(__scope_guard&&) = delete;
};
template <class _Func>
diff --git a/libcxx/include/string b/libcxx/include/string
index 3480bb7e03..88fe3e35a3 100644
--- a/libcxx/include/string
+++ b/libcxx/include/string
@@ -2181,7 +2181,7 @@ private:
__alloc_ = __str.__alloc_;
} else {
__annotate_delete();
- auto __guard = std::__make_scope_guard(__annotate_new_size(*this));
+ auto __guard = std::__make_scope_guard(__annotate_new_size(*this));
allocator_type __a = __str.__alloc_;
auto __allocation = std::__allocate_at_least(__a, __str.__get_long_cap());
__begin_lifetime(__allocation.ptr, __allocation.count);
@@ -3374,7 +3374,7 @@ template <class _CharT, class _Traits, class _Allocator>
inline _LIBCPP_CONSTEXPR_SINCE_CXX20 void
basic_string<_CharT, _Traits, _Allocator>::__shrink_or_extend(size_type __target_capacity) {
__annotate_delete();
- auto __guard = std::__make_scope_guard(__annotate_new_size(*this));
+ auto __guard = std::__make_scope_guard(__annotate_new_size(*this));
size_type __cap = capacity();
size_type __sz = size();
|
ldionne
reviewed
Nov 4, 2024
ldionne
reviewed
Nov 4, 2024
philnik777
force-pushed
the
introduce_scope_guard
branch
from
bb0e22a to
d95605c
Compare
ldionne
approved these changes
Nov 11, 2024
philnik777
force-pushed
the
introduce_scope_guard
branch
2 times, most recently
from
60d418f to
2eed91b
Compare
philnik777
force-pushed
the
introduce_scope_guard
branch
from
2eed91b to
7b015a0
Compare
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/52/builds/3649 Here is the relevant piece of the build log for the reference |
arichardson
reviewed
Nov 14, 2024
| template <class _Func> | ||
| class __scope_guard { | ||
| _Func __func_; | ||
| bool __moved_from_; |
There was a problem hiding this comment.
Isn't this used uninitialized for C++14?
There was a problem hiding this comment.
Yes, you are correct. @philnik777 Can you open a PR to fix that and #116204 ?
Groverkss
pushed a commit
to iree-org/llvm-project
that referenced
this pull request
Nov 15, 2024
…lvm#114867) This introduces a new `__scope_guard` without any fancy features. The scope guard is used in `<string>` to simplify some of the ASan annotations (especially by making it harder to forget them where exceptions are thrown).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This introduces a new
__scope_guardwithout any fancy features. The scope guard is used in<string>to simplify some of the ASan annotations (especially by making it harder to forget them where exceptions are thrown).