From 89f1f531d373f5e4eab77960d3b0e8c05d879c7c Mon Sep 17 00:00:00 2001 From: Bronek Kozicki Date: Sun, 1 Oct 2017 13:14:12 +0100 Subject: [PATCH 01/12] Make any_internal::FastTypeId() and IdForType() constexpr This means removing all side effects from FastTypeId(). So rather than instantiate dummy_var in the first call to FastTypeId(), move this responsibility to the linker, and only read its address during execution - guaranteed to never change. This allows for more optimization opportunities, with more explicit uses of constexpr --- absl/types/any.h | 50 ++++++++++++++++++++++++++++-------------------- 1 file changed, 29 insertions(+), 21 deletions(-) diff --git a/absl/types/any.h b/absl/types/any.h index a51dea11..fe104818 100644 --- a/absl/types/any.h +++ b/absl/types/any.h @@ -94,23 +94,20 @@ namespace absl { namespace any_internal { -// FastTypeId() evaluates at compile/link-time to a unique integer for the -// passed in type. Their values are neither contiguous nor small, making them -// unfit for using as an index into a vector, but a good match for keys into -// maps or straight up comparisons. -// Note that on 64-bit (unix) systems size_t is 64-bit while int is 32-bit and -// the compiler will happily and quietly assign such a 64-bit value to a -// 32-bit integer. While a client should never do that it SHOULD still be safe, -// assuming the BSS segment doesn't span more than 4GiB. +template +struct TypeTag { + constexpr static char dummy_var = 0; +}; + +template +constexpr char TypeTag::dummy_var; + +// FastTypeId() evaluates at compile/link-time to a unique pointer for the +// passed in type. These are meant to be good match for keys into maps or straight +// up comparisons. template -inline size_t FastTypeId() { - static_assert(sizeof(char*) <= sizeof(size_t), - "ptr size too large for size_t"); - - // This static variable isn't actually used, only its address, so there are - // no concurrency issues. - static char dummy_var; - return reinterpret_cast(&dummy_var); +constexpr inline const void* FastTypeId() { + return &TypeTag::dummy_var; } } // namespace any_internal @@ -382,10 +379,20 @@ class any { public: virtual ~ObjInterface() = default; virtual std::unique_ptr Clone() const = 0; - virtual size_t type_id() const noexcept = 0; + virtual const void* ObjTypeId() const noexcept = 0; #if ABSL_ANY_DETAIL_HAS_RTTI virtual const std::type_info& Type() const noexcept = 0; #endif // ABSL_ANY_DETAIL_HAS_RTTI + + // Note that on 64-bit (unix) systems size_t is 64-bit while int is 32-bit and + // the compiler will happily and quietly assign such a 64-bit value to a + // 32-bit integer. While a client should never do that it SHOULD still be safe, + // assuming the BSS segment doesn't span more than 4GiB. + size_t type_id() const noexcept { + static_assert(sizeof(void*) <= sizeof(size_t), + "ptr size too large for size_t"); + return reinterpret_cast(ObjTypeId()); + } }; // Hold a value of some queryable type, with an ability to Clone it. @@ -400,7 +407,7 @@ class any { return std::unique_ptr(new Obj(in_place, value)); } - size_t type_id() const noexcept final { return IdForType(); } + const void* ObjTypeId() const noexcept final { return IdForType(); } #if ABSL_ANY_DETAIL_HAS_RTTI const std::type_info& Type() const noexcept final { return typeid(T); } @@ -415,7 +422,7 @@ class any { } template - static size_t IdForType() { + constexpr static const void* IdForType() { // Note: This type dance is to make the behavior consistent with typeid. using NormalizedType = typename std::remove_cv::type>::type; @@ -423,8 +430,9 @@ class any { return any_internal::FastTypeId(); } - size_t GetObjTypeId() const { - return obj_ == nullptr ? any_internal::FastTypeId() : obj_->type_id(); + const void* GetObjTypeId() const { + return obj_ == nullptr ? any_internal::FastTypeId() + : obj_->ObjTypeId(); } // `absl::any` nonmember functions // From 0848aecd435389f9600249241467b74ee46f7942 Mon Sep 17 00:00:00 2001 From: misterg Date: Fri, 6 Oct 2017 15:20:20 -0400 Subject: [PATCH 02/12] Removing Windows Bazel-Ci nodes --- .ci/abseil-cpp.json | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.ci/abseil-cpp.json b/.ci/abseil-cpp.json index a90cc171..b21e68d1 100644 --- a/.ci/abseil-cpp.json +++ b/.ci/abseil-cpp.json @@ -3,6 +3,5 @@ [ {"node": "linux-x86_64"}, {"node": "ubuntu_16.04-x86_64"}, - {"node": "darwin-x86_64"}, - {"node": "windows-x86_64"} + {"node": "darwin-x86_64"} ] From b7bdd3a63d78da1cd4e537cf52cfe08068d2a399 Mon Sep 17 00:00:00 2001 From: Bronek Kozicki Date: Tue, 10 Oct 2017 22:07:10 +0100 Subject: [PATCH 03/12] Removed unused type_id() function --- absl/types/any.h | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/absl/types/any.h b/absl/types/any.h index fe104818..f29de037 100644 --- a/absl/types/any.h +++ b/absl/types/any.h @@ -383,16 +383,6 @@ class any { #if ABSL_ANY_DETAIL_HAS_RTTI virtual const std::type_info& Type() const noexcept = 0; #endif // ABSL_ANY_DETAIL_HAS_RTTI - - // Note that on 64-bit (unix) systems size_t is 64-bit while int is 32-bit and - // the compiler will happily and quietly assign such a 64-bit value to a - // 32-bit integer. While a client should never do that it SHOULD still be safe, - // assuming the BSS segment doesn't span more than 4GiB. - size_t type_id() const noexcept { - static_assert(sizeof(void*) <= sizeof(size_t), - "ptr size too large for size_t"); - return reinterpret_cast(ObjTypeId()); - } }; // Hold a value of some queryable type, with an ability to Clone it. @@ -431,8 +421,7 @@ class any { } const void* GetObjTypeId() const { - return obj_ == nullptr ? any_internal::FastTypeId() - : obj_->ObjTypeId(); + return obj_ ? obj_->ObjTypeId() : any_internal::FastTypeId(); } // `absl::any` nonmember functions // From 029795a9b5281379f892fbbe3f9a400d5a33f5cc Mon Sep 17 00:00:00 2001 From: Abseil Team Date: Tue, 10 Oct 2017 17:07:46 -0700 Subject: [PATCH 04/12] Changes imported from Abseil "staging" branch: - 3e05f2c346a9faf07088c49d590d49a9199e7edd Simplify Duration's operator-() by Jorg Brown - 3c4942375a6d17e887bb6ab7cf2d0e763d58a511 Rewrite `noexcept(noexcept(allocator_type()))` to instead... by Matt Calabrese - 02f35a684201a6aa9f70e8b0a041993676f2d230 Fix comment on remove_prefix since the function is not re... by Abseil Team - ceb40aba8031e0ccec9cd49da844882df100c56f Fix mutex_test under TSAN. by Derek Mauro - 7bd12e7ddc5d074e1b9c9f037879211fa1d81f8c Slight wording tweaks for "adopting" wrappers by Abseil Team - c3580afe092e0357d40b1769314f36da1b887c65 Internal cleanup. by Greg Miller GitOrigin-RevId: 3e05f2c346a9faf07088c49d590d49a9199e7edd Change-Id: If3df72fba3803398cfcbb323fb4cb84ec55511aa --- .ci/abseil-cpp.json | 8 --- absl/container/inlined_vector.h | 3 +- absl/memory/memory.h | 21 +++--- absl/strings/string_view.h | 5 +- absl/synchronization/mutex_test.cc | 103 +++++++++++++++-------------- absl/time/internal/test_util.cc | 2 +- absl/time/time.h | 37 +++++------ 7 files changed, 88 insertions(+), 91 deletions(-) delete mode 100644 .ci/abseil-cpp.json diff --git a/.ci/abseil-cpp.json b/.ci/abseil-cpp.json deleted file mode 100644 index a90cc171..00000000 --- a/.ci/abseil-cpp.json +++ /dev/null @@ -1,8 +0,0 @@ -// This is a relaxed JSON format, you can have comments in it. -// This is a list of configuration for the job that does not specify a configuration. -[ - {"node": "linux-x86_64"}, - {"node": "ubuntu_16.04-x86_64"}, - {"node": "darwin-x86_64"}, - {"node": "windows-x86_64"} -] diff --git a/absl/container/inlined_vector.h b/absl/container/inlined_vector.h index f68ca507..96a9d001 100644 --- a/absl/container/inlined_vector.h +++ b/absl/container/inlined_vector.h @@ -82,7 +82,8 @@ class InlinedVector { using reverse_iterator = std::reverse_iterator; using const_reverse_iterator = std::reverse_iterator; - InlinedVector() noexcept(noexcept(allocator_type())) + InlinedVector() noexcept( + std::is_nothrow_default_constructible::value) : allocator_and_tag_(allocator_type()) {} explicit InlinedVector(const allocator_type& alloc) noexcept diff --git a/absl/memory/memory.h b/absl/memory/memory.h index c6799608..95909138 100644 --- a/absl/memory/memory.h +++ b/absl/memory/memory.h @@ -38,8 +38,8 @@ namespace absl { // Function Template: WrapUnique() // ----------------------------------------------------------------------------- // -// Transfers ownership of a raw pointer to a `std::unique_ptr`. The returned -// value is a `std::unique_ptr` of deduced type. +// Adopts ownership from a raw pointer and transfers it to the returned +// `std::unique_ptr`, whose type is deduced. // // Example: // X* NewX(int, int); @@ -169,8 +169,8 @@ typename memory_internal::MakeUniqueResult::invalid make_unique( // Function Template: RawPtr() // ----------------------------------------------------------------------------- // -// Extracts the raw pointer from a pointer-like 'ptr'. `absl::RawPtr` is useful -// within templates that need to handle a complement of raw pointers, +// Extracts the raw pointer from a pointer-like value `ptr`. `absl::RawPtr` is +// useful within templates that need to handle a complement of raw pointers, // `std::nullptr_t`, and smart pointers. template auto RawPtr(T&& ptr) -> decltype(&*ptr) { @@ -183,9 +183,9 @@ inline std::nullptr_t RawPtr(std::nullptr_t) { return nullptr; } // Function Template: ShareUniquePtr() // ----------------------------------------------------------------------------- // -// Transforms a `std::unique_ptr` rvalue into a `std::shared_ptr`. The returned -// value is a `std::shared_ptr` of deduced type and ownership is transferred to -// the shared pointer. +// Adopts a `std::unique_ptr` rvalue and returns a `std::shared_ptr` of deduced +// type. Ownership (if any) of the held value is transferred to the returned +// shared pointer. // // Example: // @@ -194,8 +194,11 @@ inline std::nullptr_t RawPtr(std::nullptr_t) { return nullptr; } // CHECK_EQ(*sp, 10); // CHECK(up == nullptr); // -// Note that this conversion is correct even when T is an array type, although -// the resulting shared pointer may not be very useful. +// Note that this conversion is correct even when T is an array type, and more +// generally it works for *any* deleter of the `unique_ptr` (single-object +// deleter, array deleter, or any custom deleter), since the deleter is adopted +// by the shared pointer as well. The deleter is copied (unless it is a +// reference). // // Implements the resolution of [LWG 2415](http://wg21.link/lwg2415), by which a // null shared pointer does not attempt to call the deleter. diff --git a/absl/strings/string_view.h b/absl/strings/string_view.h index 951e9cbc..c3acd729 100644 --- a/absl/strings/string_view.h +++ b/absl/strings/string_view.h @@ -295,9 +295,8 @@ class string_view { // string_view::remove_prefix() // - // Removes the first `n` characters from the `string_view`, returning a - // pointer to the new first character. Note that the underlying std::string is not - // changed, only the view. + // Removes the first `n` characters from the `string_view`. Note that the + // underlying std::string is not changed, only the view. void remove_prefix(size_type n) { assert(n <= length_); ptr_ += n; diff --git a/absl/synchronization/mutex_test.cc b/absl/synchronization/mutex_test.cc index cfe81096..5a5874de 100644 --- a/absl/synchronization/mutex_test.cc +++ b/absl/synchronization/mutex_test.cc @@ -89,8 +89,6 @@ static void CheckSumG0G1(void *v) { } static void TestMu(TestContext *cxt, int c) { - SetInvariantChecked(false); - cxt->mu.EnableInvariantDebugging(CheckSumG0G1, cxt); for (int i = 0; i != cxt->iterations; i++) { absl::MutexLock l(&cxt->mu); int a = cxt->g0 + 1; @@ -100,8 +98,6 @@ static void TestMu(TestContext *cxt, int c) { } static void TestTry(TestContext *cxt, int c) { - SetInvariantChecked(false); - cxt->mu.EnableInvariantDebugging(CheckSumG0G1, cxt); for (int i = 0; i != cxt->iterations; i++) { do { std::this_thread::yield(); @@ -122,8 +118,6 @@ static void TestR20ms(TestContext *cxt, int c) { } static void TestRW(TestContext *cxt, int c) { - SetInvariantChecked(false); - cxt->mu.EnableInvariantDebugging(CheckSumG0G1, cxt); if ((c & 1) == 0) { for (int i = 0; i != cxt->iterations; i++) { absl::WriterMutexLock l(&cxt->mu); @@ -356,67 +350,57 @@ static void EndTest(int *c0, int *c1, absl::Mutex *mu, absl::CondVar *cv, cv->Signal(); } -// Basis for the parameterized tests configured below. -static int RunTest(void (*test)(TestContext *cxt, int), int threads, - int iterations, int operations) { - TestContext cxt; +// Code common to RunTest() and RunTestWithInvariantDebugging(). +static int RunTestCommon(TestContext *cxt, void (*test)(TestContext *cxt, int), + int threads, int iterations, int operations) { absl::Mutex mu2; absl::CondVar cv2; - int c0; - int c1; - - // run with large thread count for full test and to get timing - -#if !defined(ABSL_MUTEX_ENABLE_INVARIANT_DEBUGGING_NOT_IMPLEMENTED) - absl::EnableMutexInvariantDebugging(false); -#endif - c0 = 0; - c1 = 0; - cxt.g0 = 0; - cxt.g1 = 0; - cxt.iterations = iterations; - cxt.threads = threads; + int c0 = 0; + int c1 = 0; + cxt->g0 = 0; + cxt->g1 = 0; + cxt->iterations = iterations; + cxt->threads = threads; absl::synchronization_internal::ThreadPool tp(threads); for (int i = 0; i != threads; i++) { tp.Schedule(std::bind(&EndTest, &c0, &c1, &mu2, &cv2, std::function( - std::bind(test, &cxt, std::placeholders::_1)))); + std::bind(test, cxt, std::placeholders::_1)))); } mu2.Lock(); while (c1 != threads) { cv2.Wait(&mu2); } mu2.Unlock(); - int saved_g0 = cxt.g0; + return cxt->g0; +} - // run again with small number of iterations to test invariant checking +// Basis for the parameterized tests configured below. +static int RunTest(void (*test)(TestContext *cxt, int), int threads, + int iterations, int operations) { + TestContext cxt; + return RunTestCommon(&cxt, test, threads, iterations, operations); +} +// Like RunTest(), but sets an invariant on the tested Mutex and +// verifies that the invariant check happened. The invariant function +// will be passed the TestContext* as its arg and must call +// SetInvariantChecked(true); #if !defined(ABSL_MUTEX_ENABLE_INVARIANT_DEBUGGING_NOT_IMPLEMENTED) +static int RunTestWithInvariantDebugging(void (*test)(TestContext *cxt, int), + int threads, int iterations, + int operations, + void (*invariant)(void *)) { absl::EnableMutexInvariantDebugging(true); -#endif - SetInvariantChecked(true); - c0 = 0; - c1 = 0; - cxt.g0 = 0; - cxt.g1 = 0; - cxt.iterations = (iterations > 10 ? 10 : iterations); - cxt.threads = threads; - for (int i = 0; i != threads; i++) { - tp.Schedule(std::bind(&EndTest, &c0, &c1, &mu2, &cv2, - std::function( - std::bind(test, &cxt, std::placeholders::_1)))); - } - mu2.Lock(); - while (c1 != threads) { - cv2.Wait(&mu2); - } - mu2.Unlock(); -#if !defined(ABSL_MUTEX_ENABLE_INVARIANT_DEBUGGING_NOT_IMPLEMENTED) + SetInvariantChecked(false); + TestContext cxt; + cxt.mu.EnableInvariantDebugging(invariant, &cxt); + int ret = RunTestCommon(&cxt, test, threads, iterations, operations); ABSL_RAW_CHECK(GetInvariantChecked(), "Invariant not checked"); -#endif - - return saved_g0; + absl::EnableMutexInvariantDebugging(false); // Restore. + return ret; } +#endif // -------------------------------------------------------- // Test for fix of bug in TryRemove() @@ -1463,6 +1447,13 @@ TEST_P(MutexVariableThreadCountTest, Mutex) { int iterations = ScaleIterations(10000000) / threads; int operations = threads * iterations; EXPECT_EQ(RunTest(&TestMu, threads, iterations, operations), operations); +#if !defined(ABSL_MUTEX_ENABLE_INVARIANT_DEBUGGING_NOT_IMPLEMENTED) + iterations = std::min(iterations, 10); + operations = threads * iterations; + EXPECT_EQ(RunTestWithInvariantDebugging(&TestMu, threads, iterations, + operations, CheckSumG0G1), + operations); +#endif } TEST_P(MutexVariableThreadCountTest, Try) { @@ -1470,6 +1461,13 @@ TEST_P(MutexVariableThreadCountTest, Try) { int iterations = 1000000 / threads; int operations = iterations * threads; EXPECT_EQ(RunTest(&TestTry, threads, iterations, operations), operations); +#if !defined(ABSL_MUTEX_ENABLE_INVARIANT_DEBUGGING_NOT_IMPLEMENTED) + iterations = std::min(iterations, 10); + operations = threads * iterations; + EXPECT_EQ(RunTestWithInvariantDebugging(&TestTry, threads, iterations, + operations, CheckSumG0G1), + operations); +#endif } TEST_P(MutexVariableThreadCountTest, R20ms) { @@ -1484,6 +1482,13 @@ TEST_P(MutexVariableThreadCountTest, RW) { int iterations = ScaleIterations(20000000) / threads; int operations = iterations * threads; EXPECT_EQ(RunTest(&TestRW, threads, iterations, operations), operations / 2); +#if !defined(ABSL_MUTEX_ENABLE_INVARIANT_DEBUGGING_NOT_IMPLEMENTED) + iterations = std::min(iterations, 10); + operations = threads * iterations; + EXPECT_EQ(RunTestWithInvariantDebugging(&TestRW, threads, iterations, + operations, CheckSumG0G1), + operations / 2); +#endif } TEST_P(MutexVariableThreadCountTest, Await) { diff --git a/absl/time/internal/test_util.cc b/absl/time/internal/test_util.cc index 1a415f89..8bb27a8f 100644 --- a/absl/time/internal/test_util.cc +++ b/absl/time/internal/test_util.cc @@ -63,7 +63,7 @@ const struct ZoneInfo { {"US/Pacific", // reinterpret_cast(America_Los_Angeles), America_Los_Angeles_len}, - // Allows use of the local time zone from a common system-specific location. + // Allows use of the local time zone from a system-specific location. #ifdef _MSC_VER {"localtime", // reinterpret_cast(America_Los_Angeles), America_Los_Angeles_len}, diff --git a/absl/time/time.h b/absl/time/time.h index 093f168d..c01977b0 100644 --- a/absl/time/time.h +++ b/absl/time/time.h @@ -1126,8 +1126,10 @@ constexpr Duration OppositeInfinity(Duration d) { : MakeDuration(std::numeric_limits::min(), ~0U); } -// Returns (-n)-1 (equivalently -(n+1)) without overflowing on any input value. +// Returns (-n)-1 (equivalently -(n+1)) without avoidable overflow. constexpr int64_t NegateAndSubtractOne(int64_t n) { + // Note: Good compilers will optimize this expression to ~n when using + // a two's-complement representation (which is required for int64_t). return (n < 0) ? -(n + 1) : (-n) - 1; } @@ -1232,31 +1234,26 @@ constexpr bool operator==(Duration lhs, Duration rhs) { constexpr Duration operator-(Duration d) { // This is a little interesting because of the special cases. // - // Infinities stay infinite, and just change direction. + // If rep_lo_ is zero, we have it easy; it's safe to negate rep_hi_, we're + // dealing with an integral number of seconds, and the only special case is + // the maximum negative finite duration, which can't be negated. // - // The maximum negative finite duration can't be negated (at least, not - // on a two's complement machine), so we return infinity for that case. - // Next we dispatch the case where rep_lo_ is zero, observing that it's - // safe to negate rep_hi_ in this case because it's not int64_t-min (or - // else we'd have handled it above, returning InfiniteDuration()). + // Infinities stay infinite, and just change direction. // // Finally we're in the case where rep_lo_ is non-zero, and we can borrow // a second's worth of ticks and avoid overflow (as negating int64_t-min + 1 // is safe). - return time_internal::IsInfiniteDuration(d) - ? time_internal::OppositeInfinity(d) - : (time_internal::GetRepHi(d) == - std::numeric_limits::min() && - time_internal::GetRepLo(d) == 0) + return time_internal::GetRepLo(d) == 0 + ? time_internal::GetRepHi(d) == std::numeric_limits::min() ? InfiniteDuration() - : (time_internal::GetRepLo(d) == 0) - ? time_internal::MakeDuration( - -time_internal::GetRepHi(d)) - : time_internal::MakeDuration( - time_internal::NegateAndSubtractOne( - time_internal::GetRepHi(d)), - time_internal::kTicksPerSecond - - time_internal::GetRepLo(d)); + : time_internal::MakeDuration(-time_internal::GetRepHi(d)) + : time_internal::IsInfiniteDuration(d) + ? time_internal::OppositeInfinity(d) + : time_internal::MakeDuration( + time_internal::NegateAndSubtractOne( + time_internal::GetRepHi(d)), + time_internal::kTicksPerSecond - + time_internal::GetRepLo(d)); } constexpr Duration Nanoseconds(int64_t n) { From bbf83057e5f611a66ba9dfeabb14ccfd5cf08ac7 Mon Sep 17 00:00:00 2001 From: Bronek Kozicki Date: Wed, 11 Oct 2017 22:11:13 +0100 Subject: [PATCH 05/12] Wrap comment at 80 cols --- absl/types/any.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/absl/types/any.h b/absl/types/any.h index f29de037..2e7bf21f 100644 --- a/absl/types/any.h +++ b/absl/types/any.h @@ -103,8 +103,8 @@ template constexpr char TypeTag::dummy_var; // FastTypeId() evaluates at compile/link-time to a unique pointer for the -// passed in type. These are meant to be good match for keys into maps or straight -// up comparisons. +// passed in type. These are meant to be good match for keys into maps or +// straight up comparisons. template constexpr inline const void* FastTypeId() { return &TypeTag::dummy_var; From 894a869e7b5d7c1c04b9583e706e21cef6a1d6d6 Mon Sep 17 00:00:00 2001 From: Eric Fiselier Date: Wed, 11 Oct 2017 17:38:16 -0600 Subject: [PATCH 06/12] Add Google-style .clang-format file. It would be nice if the repository contained a clang-format config file so that PR submitters could use tooling to correctly format their code match Googles/abseils style. This also makes it easier for reviewers to specify what coding style should be used. ie the one the tooling produces. The clang-format file is the default Google configuration as dumped by `clang-format`. I'm not sure abseil will want the exact same options, but it seemed like the right starting place. --- .clang-format | 4 ++++ CONTRIBUTING.md | 3 ++- 2 files changed, 6 insertions(+), 1 deletion(-) create mode 100644 .clang-format diff --git a/.clang-format b/.clang-format new file mode 100644 index 00000000..06ea346a --- /dev/null +++ b/.clang-format @@ -0,0 +1,4 @@ +--- +Language: Cpp +BasedOnStyle: Google +... diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index c9c89bbc..865da86d 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -47,7 +47,8 @@ will be expected to conform to the style outlined made and **why** it was made. Link to a GitHub issue if it exists. * Don't fix code style and formatting unless you are already changing that - line to address an issue. PRs with irrelevant changes won't be merged. If + line to address an issue. Formatting of modified lines may be done using + `git clang-format`. PRs with irrelevant changes won't be merged. If you do want to fix formatting or style, do that in a separate PR. * Unless your PR is trivial, you should expect there will be reviewer comments From 6de53819a7173bd446156237a37f53464b7732cc Mon Sep 17 00:00:00 2001 From: Abseil Team Date: Fri, 13 Oct 2017 10:21:40 -0700 Subject: [PATCH 07/12] Changes imported from Abseil "staging" branch: - 432508bf64998983b3c194d5f164872ce3c2e573 Put visibility tags into absl external build files by Jon Cohen - 25d59d11e7b833fe632cddb5bf4d76075ae6282b Use ABSL_PREDICT_TRUE instead of *FALSE for the range che... by Jon Cohen - 8d8a5890a55ddd19aac849748441eeb57c684f10 Better detection for MSVC support on std::optional. by Xiaoyi Zhang - c1b31e4a97939885c3bbc23ecb093e9619e73ad1 Internal cleanup by Gennadiy Rozental - 4f56ad20c4eeccc6f5fb21ec6c7191233d34a090 Internal change. by Matt Calabrese - d2a02b52c75c295708170f4d17b7ff442c8d6a97 Fixed a minor typo in the SimpleAtob() function comment. by Abseil Team - 5adbff5c23a45278d06de2ef3a29ea51b0d1269e Internal cleanup by Gennadiy Rozental GitOrigin-RevId: 432508bf64998983b3c194d5f164872ce3c2e573 Change-Id: I32ddd151d3350b96a22e8f1830f19b59374953ad --- .ci/abseil-cpp.json | 7 ------- absl/BUILD.bazel | 4 ++++ absl/base/BUILD.bazel | 18 ++++++++++++++++-- absl/base/config.h | 15 +++++++++++++++ absl/container/BUILD.bazel | 3 +++ absl/strings/BUILD.bazel | 15 +++++++++++++++ absl/strings/internal/utf8.h | 1 - absl/strings/match.h | 5 +++-- absl/strings/numbers.h | 6 +++--- absl/synchronization/BUILD.bazel | 3 +++ absl/time/BUILD.bazel | 3 +++ absl/types/optional_test.cc | 9 +++++++++ absl/types/span.h | 2 +- 13 files changed, 75 insertions(+), 16 deletions(-) delete mode 100644 .ci/abseil-cpp.json diff --git a/.ci/abseil-cpp.json b/.ci/abseil-cpp.json deleted file mode 100644 index b21e68d1..00000000 --- a/.ci/abseil-cpp.json +++ /dev/null @@ -1,7 +0,0 @@ -// This is a relaxed JSON format, you can have comments in it. -// This is a list of configuration for the job that does not specify a configuration. -[ - {"node": "linux-x86_64"}, - {"node": "ubuntu_16.04-x86_64"}, - {"node": "darwin-x86_64"} -] diff --git a/absl/BUILD.bazel b/absl/BUILD.bazel index 403a35c3..439addbf 100644 --- a/absl/BUILD.bazel +++ b/absl/BUILD.bazel @@ -23,6 +23,7 @@ config_setting( values = { "compiler": "llvm", }, + visibility = [":__subpackages__"], ) # following configs are based on mapping defined in: https://git.io/v5Ijz @@ -31,6 +32,7 @@ config_setting( values = { "cpu": "darwin", }, + visibility = [":__subpackages__"], ) config_setting( @@ -38,6 +40,7 @@ config_setting( values = { "cpu": "x64_windows", }, + visibility = [":__subpackages__"], ) config_setting( @@ -45,4 +48,5 @@ config_setting( values = { "cpu": "ppc", }, + visibility = [":__subpackages__"], ) diff --git a/absl/base/BUILD.bazel b/absl/base/BUILD.bazel index 23439a09..d68448da 100644 --- a/absl/base/BUILD.bazel +++ b/absl/base/BUILD.bazel @@ -25,8 +25,6 @@ package(default_visibility = ["//visibility:public"]) licenses(["notice"]) # Apache 2.0 -exports_files(["thread_annotations.h"]) - cc_library( name = "spinlock_wait", srcs = [ @@ -39,6 +37,9 @@ cc_library( "internal/spinlock_wait.h", ], copts = ABSL_DEFAULT_COPTS, + visibility = [ + "//absl/base:__pkg__", + ], deps = [":core_headers"], ) @@ -83,6 +84,9 @@ cc_library( "internal/malloc_extension_c.h", ], copts = ABSL_DEFAULT_COPTS, + visibility = [ + "//absl:__subpackages__", + ], deps = [ ":core_headers", ":dynamic_annotations", @@ -108,6 +112,9 @@ cc_library( textual_hdrs = [ "internal/malloc_hook_invoke.h", ], + visibility = [ + "//absl:__subpackages__", + ], deps = [ ":base", ":config", @@ -124,6 +131,9 @@ cc_library( "internal/invoke.h", ], copts = ABSL_DEFAULT_COPTS, + visibility = [ + "//absl:__subpackages__", + ], ) cc_library( @@ -183,6 +193,9 @@ cc_library( features = [ "-use_header_modules", ], + visibility = [ + "//absl:__subpackages__", + ], deps = [ ":base", ":config", @@ -205,6 +218,7 @@ cc_library( testonly = 1, hdrs = ["internal/exception_testing.h"], copts = ABSL_TEST_COPTS, + visibility = ["//absl:__subpackages__"], deps = [ ":config", "@com_google_googletest//:gtest", diff --git a/absl/base/config.h b/absl/base/config.h index 5f0dd04c..495811bd 100644 --- a/absl/base/config.h +++ b/absl/base/config.h @@ -372,4 +372,19 @@ #endif #endif +// For MSVC, `__has_include` is supported in VS 2017 15.3, which is later than +// the support for , , . So we use _MSC_VER to check +// whether we have VS 2017 RTM (when , , is +// implemented) or higher. +// Also, `__cplusplus` is not correctly set by MSVC, so we use `_MSVC_LANG` to +// check the language version. +// TODO(zhangxy): fix tests before enabling aliasing for `std::any`, +// `std::string_view`. +#if defined(_MSC_VER) && _MSC_VER >= 1910 && \ + ((defined(_MSVC_LANG) && _MSVC_LANG > 201402) || __cplusplus > 201402) +// #define ABSL_HAVE_STD_ANY 1 +#define ABSL_HAVE_STD_OPTIONAL 1 +// #define ABSL_HAVE_STD_STRING_VIEW 1 +#endif + #endif // ABSL_BASE_CONFIG_H_ diff --git a/absl/container/BUILD.bazel b/absl/container/BUILD.bazel index ee017431..7d550cb1 100644 --- a/absl/container/BUILD.bazel +++ b/absl/container/BUILD.bazel @@ -112,6 +112,9 @@ cc_library( srcs = ["internal/test_instance_tracker.cc"], hdrs = ["internal/test_instance_tracker.h"], copts = ABSL_DEFAULT_COPTS, + visibility = [ + "//absl:__subpackages__", + ], ) cc_test( diff --git a/absl/strings/BUILD.bazel b/absl/strings/BUILD.bazel index b2610663..49f49abd 100644 --- a/absl/strings/BUILD.bazel +++ b/absl/strings/BUILD.bazel @@ -103,6 +103,7 @@ cc_test( size = "small", srcs = ["match_test.cc"], copts = ABSL_TEST_COPTS, + visibility = ["//visibility:private"], deps = [ ":strings", "@com_google_googletest//:gtest_main", @@ -117,6 +118,7 @@ cc_test( "internal/escaping_test_common.inc", ], copts = ABSL_TEST_COPTS, + visibility = ["//visibility:private"], deps = [ ":strings", "//absl/base:core_headers", @@ -130,6 +132,7 @@ cc_test( size = "small", srcs = ["ascii_test.cc"], copts = ABSL_TEST_COPTS, + visibility = ["//visibility:private"], deps = [ ":strings", "//absl/base:core_headers", @@ -145,6 +148,7 @@ cc_test( "internal/memutil_test.cc", ], copts = ABSL_TEST_COPTS, + visibility = ["//visibility:private"], deps = [ ":strings", "//absl/base:core_headers", @@ -159,6 +163,7 @@ cc_test( "internal/utf8_test.cc", ], copts = ABSL_TEST_COPTS, + visibility = ["//visibility:private"], deps = [ ":internal", ":strings", @@ -172,6 +177,7 @@ cc_test( size = "small", srcs = ["string_view_test.cc"], copts = ABSL_TEST_COPTS + ABSL_EXCEPTIONS_FLAG, + visibility = ["//visibility:private"], deps = [ ":strings", "//absl/base:config", @@ -186,6 +192,7 @@ cc_test( size = "small", srcs = ["substitute_test.cc"], copts = ABSL_TEST_COPTS, + visibility = ["//visibility:private"], deps = [ ":strings", "//absl/base:core_headers", @@ -198,6 +205,7 @@ cc_test( size = "small", srcs = ["str_replace_test.cc"], copts = ABSL_TEST_COPTS, + visibility = ["//visibility:private"], deps = [ ":strings", "@com_google_googletest//:gtest_main", @@ -208,6 +216,7 @@ cc_test( name = "str_split_test", srcs = ["str_split_test.cc"], copts = ABSL_TEST_COPTS, + visibility = ["//visibility:private"], deps = [ ":strings", "//absl/base:core_headers", @@ -221,6 +230,7 @@ cc_test( size = "small", srcs = ["internal/ostringstream_test.cc"], copts = ABSL_TEST_COPTS, + visibility = ["//visibility:private"], deps = [ ":internal", "@com_google_googletest//:gtest_main", @@ -235,6 +245,7 @@ cc_test( "internal/resize_uninitialized_test.cc", ], copts = ABSL_TEST_COPTS, + visibility = ["//visibility:private"], deps = [ "//absl/base:core_headers", "//absl/meta:type_traits", @@ -247,6 +258,7 @@ cc_test( size = "small", srcs = ["str_join_test.cc"], copts = ABSL_TEST_COPTS, + visibility = ["//visibility:private"], deps = [ ":strings", "//absl/base:core_headers", @@ -260,6 +272,7 @@ cc_test( size = "small", srcs = ["str_cat_test.cc"], copts = ABSL_TEST_COPTS, + visibility = ["//visibility:private"], deps = [ ":strings", "//absl/base:core_headers", @@ -278,6 +291,7 @@ cc_test( tags = [ "no_test_loonix", ], + visibility = ["//visibility:private"], deps = [ ":strings", "//absl/base", @@ -291,6 +305,7 @@ cc_test( size = "small", srcs = ["strip_test.cc"], copts = ABSL_TEST_COPTS, + visibility = ["//visibility:private"], deps = [ ":strings", "@com_google_googletest//:gtest_main", diff --git a/absl/strings/internal/utf8.h b/absl/strings/internal/utf8.h index 705eea7f..5bd82e84 100644 --- a/absl/strings/internal/utf8.h +++ b/absl/strings/internal/utf8.h @@ -25,7 +25,6 @@ #include #include - namespace absl { namespace strings_internal { diff --git a/absl/strings/match.h b/absl/strings/match.h index 4ac35f19..3d54da81 100644 --- a/absl/strings/match.h +++ b/absl/strings/match.h @@ -53,7 +53,7 @@ inline bool StrContains(absl::string_view haystack, absl::string_view needle) { inline bool StartsWith(absl::string_view text, absl::string_view prefix) { return prefix.empty() || (text.size() >= prefix.size() && - memcmp(text.data(), prefix.data(), prefix.size()) == 0); + memcmp(text.data(), prefix.data(), prefix.size()) == 0); } // EndsWith() @@ -63,7 +63,8 @@ inline bool EndsWith(absl::string_view text, absl::string_view suffix) { return suffix.empty() || (text.size() >= suffix.size() && memcmp(text.data() + (text.size() - suffix.size()), suffix.data(), - suffix.size()) == 0); + suffix.size()) == 0 + ); } // StartsWithIgnoreCase() diff --git a/absl/strings/numbers.h b/absl/strings/numbers.h index 74aebc80..1f3bbcfa 100644 --- a/absl/strings/numbers.h +++ b/absl/strings/numbers.h @@ -62,9 +62,9 @@ ABSL_MUST_USE_RESULT bool SimpleAtod(absl::string_view str, double* value); // SimpleAtob() // -// Converts the given std::string into into a boolean, returning `true` if -// successful. The following case-insensitive strings are interpreted as boolean -// `true`: "true", "t", "yes", "y", "1". The following case-insensitive strings +// Converts the given std::string into a boolean, returning `true` if successful. +// The following case-insensitive strings are interpreted as boolean `true`: +// "true", "t", "yes", "y", "1". The following case-insensitive strings // are interpreted as boolean `false`: "false", "f", "no", "n", "0". ABSL_MUST_USE_RESULT bool SimpleAtob(absl::string_view str, bool* value); diff --git a/absl/synchronization/BUILD.bazel b/absl/synchronization/BUILD.bazel index cc8cecf9..4faf62de 100644 --- a/absl/synchronization/BUILD.bazel +++ b/absl/synchronization/BUILD.bazel @@ -34,6 +34,9 @@ cc_library( "internal/graphcycles.h", ], copts = ABSL_DEFAULT_COPTS, + visibility = [ + "//absl:__subpackages__", + ], deps = [ "//absl/base", "//absl/base:core_headers", diff --git a/absl/time/BUILD.bazel b/absl/time/BUILD.bazel index c34f5248..3d1d2df5 100644 --- a/absl/time/BUILD.bazel +++ b/absl/time/BUILD.bazel @@ -57,6 +57,9 @@ cc_library( ], hdrs = ["internal/test_util.h"], copts = ABSL_DEFAULT_COPTS, + visibility = [ + "//absl/time:__pkg__", + ], deps = [ ":time", "//absl/base", diff --git a/absl/types/optional_test.cc b/absl/types/optional_test.cc index 645f5b93..5eedfcfd 100644 --- a/absl/types/optional_test.cc +++ b/absl/types/optional_test.cc @@ -270,8 +270,17 @@ TEST(optionalTest, CopyConstructor) { EXPECT_TRUE(absl::is_trivially_copy_constructible< absl::optional>::value); #endif + // When testing with VS 2017 15.3, there seems to be a bug in MSVC + // std::optional when T is volatile-qualified. So skipping this test. + // Bug report: + // https://connect.microsoft.com/VisualStudio/feedback/details/3142534 +#if defined(ABSL_HAVE_STD_OPTIONAL) && defined(_MSC_VER) && _MSC_VER >= 1911 +#define ABSL_MSVC_OPTIONAL_VOLATILE_COPY_BUG 1 +#endif +#ifndef ABSL_MSVC_OPTIONAL_VOLATILE_COPY_BUG EXPECT_FALSE(std::is_copy_constructible< absl::optional>::value); +#endif } } diff --git a/absl/types/span.h b/absl/types/span.h index e1f006ad..f4738153 100644 --- a/absl/types/span.h +++ b/absl/types/span.h @@ -378,7 +378,7 @@ class Span { // // Returns a reference to the i'th element of this span. constexpr reference at(size_type i) const { - return ABSL_PREDICT_FALSE(i < size()) + return ABSL_PREDICT_TRUE(i < size()) ? ptr_[i] : (base_internal::ThrowStdOutOfRange( "Span::at failed bounds check"), From d8ac7afc105255e451b0a560f7d7fe30a26898cf Mon Sep 17 00:00:00 2001 From: Abseil Team Date: Tue, 17 Oct 2017 16:29:27 -0400 Subject: [PATCH 08/12] Changes imported from Abseil "staging" branch: - 1434dc58cc24e004531a09bcf1491773d9bf789e Copybara clang-format by Juemin Yang - 6296f0f69b23d406a275b7ce2669ea3b18149bb7 Internal change for git pull request #31 by Juemin Yang - 539940d88cfdf172b4b916d44225cc42839eeee7 Add internal-only ABSL_INTERNAL_HAVE_TSAN_INTERFACE macro. by Daniel Katz - bf85dda4ffdb4dd15084fb8b8db00281481dee90 Add missing pthread.h include to low_level_alloc.cc. by Derek Mauro GitOrigin-RevId: 1434dc58cc24e004531a09bcf1491773d9bf789e Change-Id: I68d6957b0cac32020e4e34dca3243f2f270a4b9c --- CONTRIBUTING.md | 2 +- absl/base/internal/low_level_alloc.cc | 1 + absl/base/internal/tsan_mutex_interface.h | 17 ++++++++++++++++- 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 865da86d..40351ddc 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -48,7 +48,7 @@ will be expected to conform to the style outlined * Don't fix code style and formatting unless you are already changing that line to address an issue. Formatting of modified lines may be done using - `git clang-format`. PRs with irrelevant changes won't be merged. If + `git clang-format`. PRs with irrelevant changes won't be merged. If you do want to fix formatting or style, do that in a separate PR. * Unless your PR is trivial, you should expect there will be reviewer comments diff --git a/absl/base/internal/low_level_alloc.cc b/absl/base/internal/low_level_alloc.cc index 08f89ea9..8e2f9c98 100644 --- a/absl/base/internal/low_level_alloc.cc +++ b/absl/base/internal/low_level_alloc.cc @@ -30,6 +30,7 @@ #ifndef ABSL_LOW_LEVEL_ALLOC_MISSING #ifndef _WIN32 +#include #include #include #include diff --git a/absl/base/internal/tsan_mutex_interface.h b/absl/base/internal/tsan_mutex_interface.h index a1303e67..6bb4faed 100644 --- a/absl/base/internal/tsan_mutex_interface.h +++ b/absl/base/internal/tsan_mutex_interface.h @@ -19,7 +19,22 @@ #ifndef ABSL_BASE_INTERNAL_TSAN_MUTEX_INTERFACE_H_ #define ABSL_BASE_INTERNAL_TSAN_MUTEX_INTERFACE_H_ -#ifdef THREAD_SANITIZER +// ABSL_INTERNAL_HAVE_TSAN_INTERFACE +// Macro intended only for internal use. +// +// Checks whether LLVM Thread Sanitizer interfaces are available. +// First made available in LLVM 5.0 (Sep 2017). +#ifdef ABSL_INTERNAL_HAVE_TSAN_INTERFACE +#error "ABSL_INTERNAL_HAVE_TSAN_INTERFACE cannot be directly set." +#endif + +#if defined(THREAD_SANITIZER) && defined(__has_include) +#if __has_include() +#define ABSL_INTERNAL_HAVE_TSAN_INTERFACE 1 +#endif +#endif + +#ifdef ABSL_INTERNAL_HAVE_TSAN_INTERFACE #include #define ABSL_TSAN_MUTEX_CREATE __tsan_mutex_create From 6cf9c731027f4d8aebe3c60df8e64317e6870870 Mon Sep 17 00:00:00 2001 From: Abseil Team Date: Tue, 17 Oct 2017 17:29:27 -0700 Subject: [PATCH 09/12] Changes imported from Abseil "staging" branch: - 2620e69367081a44678a4550dff050cf9bef9106 Avoid unused variable warnings. by Abseil Team GitOrigin-RevId: 2620e69367081a44678a4550dff050cf9bef9106 Change-Id: I0acf38c122e5bbb0864a8ee851ec72dea6c2cca0 --- absl/strings/string_view_test.cc | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/absl/strings/string_view_test.cc b/absl/strings/string_view_test.cc index 6be6f3b8..13fc214b 100644 --- a/absl/strings/string_view_test.cc +++ b/absl/strings/string_view_test.cc @@ -922,6 +922,10 @@ TEST(StringViewTest, ConstexprCompiles) { constexpr absl::string_view::iterator const_begin_empty = sp.begin(); constexpr absl::string_view::iterator const_end_empty = sp.end(); EXPECT_EQ(const_begin_empty, const_end_empty); + + constexpr absl::string_view::iterator const_begin_nullptr = cstr.begin(); + constexpr absl::string_view::iterator const_end_nullptr = cstr.end(); + EXPECT_EQ(const_begin_nullptr, const_end_nullptr); #endif constexpr absl::string_view::iterator const_begin = cstr_len.begin(); From 5fcbe86e7ba65b6457d98764aa511c4f05c9435b Mon Sep 17 00:00:00 2001 From: Abseil Team Date: Mon, 23 Oct 2017 11:40:35 -0700 Subject: [PATCH 10/12] Changes imported from Abseil "staging" branch: - 989557e6b443a81b5ad9bd0d0c704edbe96c09c9 Make InlinedVector::ShiftRight update the vector's size -... by Jon Cohen - ffc2e2a6f169bbfa823890f21d13e16110cd0206 Fix issues when passing references aliasing into an Inlin... by Jon Cohen - 2fce2f87043f8c044889b4aab828e6edc20da0d9 In C++14 or later, alias absl::make_unique to std::make_u... by Abseil Team - cb83e95b486c59fd6acfa956e97f42253dd158bd Roll back change to avoid weak virtual table warnings (-W... by Abseil Team - fb4ea46062895cb9340166c9dcc61ec4467bd834 Avoid weak virtual table warnings (-Wweak-vtables) and re... by Abseil Team GitOrigin-RevId: 989557e6b443a81b5ad9bd0d0c704edbe96c09c9 Change-Id: I6b8119c3f16e9d0cb9b5fd6e750502c9dad8e257 --- absl/container/inlined_vector.h | 42 +++++++------- absl/container/inlined_vector_test.cc | 83 +++++++++++++++++++++++++++ absl/memory/memory.h | 4 ++ absl/memory/memory_test.cc | 10 ++++ 4 files changed, 117 insertions(+), 22 deletions(-) diff --git a/absl/container/inlined_vector.h b/absl/container/inlined_vector.h index 96a9d001..e0bb900c 100644 --- a/absl/container/inlined_vector.h +++ b/absl/container/inlined_vector.h @@ -149,6 +149,9 @@ class InlinedVector { ~InlinedVector() { clear(); } InlinedVector& operator=(const InlinedVector& v) { + if (this == &v) { + return *this; + } // Optimized to avoid reallocation. // Prefer reassignment to copy construction for elements. if (size() < v.size()) { // grow @@ -681,6 +684,8 @@ class InlinedVector { // portion and the start of the uninitialized portion of the created gap. // The number of initialized spots is pair.second - pair.first; // the number of raw spots is n - (pair.second - pair.first). + // + // Updates the size of the InlinedVector internally. std::pair ShiftRight(const_iterator position, size_type n); @@ -1014,28 +1019,19 @@ typename InlinedVector::iterator InlinedVector::emplace( emplace_back(std::forward(args)...); return end() - 1; } - size_type s = size(); - size_type idx = std::distance(cbegin(), position); - if (s == capacity()) { - EnlargeBy(1); - } - assert(s < capacity()); - iterator pos = begin() + idx; // Set 'pos' to a post-enlarge iterator. - pointer space; - if (allocated()) { - tag().set_allocated_size(s + 1); - space = allocated_space(); + T new_t = T(std::forward(args)...); + + auto range = ShiftRight(position, 1); + if (range.first == range.second) { + // constructing into uninitialized memory + Construct(range.first, std::move(new_t)); } else { - tag().set_inline_size(s + 1); - space = inlined_space(); + // assigning into moved-from object + *range.first = T(std::move(new_t)); } - Construct(space + s, std::move(space[s - 1])); - std::move_backward(pos, space + s - 1, space + s); - Destroy(pos, pos + 1); - Construct(pos, std::forward(args)...); - return pos; + return range.first; } template @@ -1220,6 +1216,7 @@ auto InlinedVector::ShiftRight(const_iterator position, size_type n) start_used = pos; start_raw = pos + new_elements_in_used_space; } + tag().add_size(n); return std::make_pair(start_used, start_raw); } @@ -1298,10 +1295,12 @@ auto InlinedVector::InsertWithCount(const_iterator position, -> iterator { assert(position >= begin() && position <= end()); if (n == 0) return const_cast(position); + + value_type copy = v; std::pair it_pair = ShiftRight(position, n); - std::fill(it_pair.first, it_pair.second, v); - UninitializedFill(it_pair.second, it_pair.first + n, v); - tag().add_size(n); + std::fill(it_pair.first, it_pair.second, copy); + UninitializedFill(it_pair.second, it_pair.first + n, copy); + return it_pair.first; } @@ -1337,7 +1336,6 @@ auto InlinedVector::InsertWithRange(const_iterator position, ForwardIter open_spot = std::next(first, used_spots); std::copy(first, open_spot, it_pair.first); UninitializedCopy(open_spot, last, it_pair.second); - tag().add_size(n); return it_pair.first; } diff --git a/absl/container/inlined_vector_test.cc b/absl/container/inlined_vector_test.cc index c559a9a1..055bca98 100644 --- a/absl/container/inlined_vector_test.cc +++ b/absl/container/inlined_vector_test.cc @@ -14,6 +14,7 @@ #include "absl/container/inlined_vector.h" +#include #include #include #include @@ -569,6 +570,16 @@ TEST(IntVec, CopyConstructorAndAssignment) { } } +TEST(IntVec, AliasingCopyAssignment) { + for (int len = 0; len < 20; ++len) { + IntVec original; + Fill(&original, len); + IntVec dup = original; + dup = dup; + EXPECT_EQ(dup, original); + } +} + TEST(IntVec, MoveConstructorAndAssignment) { for (int len = 0; len < 20; len++) { IntVec v_in; @@ -606,6 +617,78 @@ TEST(IntVec, MoveConstructorAndAssignment) { } } +class NotTriviallyDestructible { + public: + NotTriviallyDestructible() : p_(new int(1)) {} + explicit NotTriviallyDestructible(int i) : p_(new int(i)) {} + + NotTriviallyDestructible(const NotTriviallyDestructible& other) + : p_(new int(*other.p_)) {} + + NotTriviallyDestructible& operator=(const NotTriviallyDestructible& other) { + p_ = absl::make_unique(*other.p_); + return *this; + } + + bool operator==(const NotTriviallyDestructible& other) const { + return *p_ == *other.p_; + } + + private: + std::unique_ptr p_; +}; + +TEST(AliasingTest, Emplace) { + for (int i = 2; i < 20; ++i) { + absl::InlinedVector vec; + for (int j = 0; j < i; ++j) { + vec.push_back(NotTriviallyDestructible(j)); + } + vec.emplace(vec.begin(), vec[0]); + EXPECT_EQ(vec[0], vec[1]); + vec.emplace(vec.begin() + i / 2, vec[i / 2]); + EXPECT_EQ(vec[i / 2], vec[i / 2 + 1]); + vec.emplace(vec.end() - 1, vec.back()); + EXPECT_EQ(vec[vec.size() - 2], vec.back()); + } +} + +TEST(AliasingTest, InsertWithCount) { + for (int i = 1; i < 20; ++i) { + absl::InlinedVector vec; + for (int j = 0; j < i; ++j) { + vec.push_back(NotTriviallyDestructible(j)); + } + for (int n = 0; n < 5; ++n) { + // We use back where we can because it's guaranteed to become invalidated + vec.insert(vec.begin(), n, vec.back()); + auto b = vec.begin(); + EXPECT_TRUE( + std::all_of(b, b + n, [&vec](const NotTriviallyDestructible& x) { + return x == vec.back(); + })); + + auto m_idx = vec.size() / 2; + vec.insert(vec.begin() + m_idx, n, vec.back()); + auto m = vec.begin() + m_idx; + EXPECT_TRUE( + std::all_of(m, m + n, [&vec](const NotTriviallyDestructible& x) { + return x == vec.back(); + })); + + // We want distinct values so the equality test is meaningful, + // vec[vec.size() - 1] is also almost always invalidated. + auto old_e = vec.size() - 1; + auto val = vec[old_e]; + vec.insert(vec.end(), n, vec[old_e]); + auto e = vec.begin() + old_e; + EXPECT_TRUE(std::all_of( + e, e + n, + [&val](const NotTriviallyDestructible& x) { return x == val; })); + } + } +} + TEST(OverheadTest, Storage) { // Check for size overhead. // In particular, ensure that std::allocator doesn't cost anything to store. diff --git a/absl/memory/memory.h b/absl/memory/memory.h index 95909138..15cd85f4 100644 --- a/absl/memory/memory.h +++ b/absl/memory/memory.h @@ -81,6 +81,9 @@ struct MakeUniqueResult { } // namespace memory_internal +#if __cplusplus >= 201402L || defined(_MSC_VER) +using std::make_unique; +#else // ----------------------------------------------------------------------------- // Function Template: make_unique() // ----------------------------------------------------------------------------- @@ -164,6 +167,7 @@ typename memory_internal::MakeUniqueResult::array make_unique(size_t n) { template typename memory_internal::MakeUniqueResult::invalid make_unique( Args&&... /* args */) = delete; +#endif // ----------------------------------------------------------------------------- // Function Template: RawPtr() diff --git a/absl/memory/memory_test.cc b/absl/memory/memory_test.cc index 8a5f5522..7d047ca0 100644 --- a/absl/memory/memory_test.cc +++ b/absl/memory/memory_test.cc @@ -138,6 +138,16 @@ TEST(Make_UniqueTest, Array) { EXPECT_THAT(ArrayWatch::allocs(), ElementsAre(5 * sizeof(ArrayWatch))); } +TEST(Make_UniqueTest, NotAmbiguousWithStdMakeUnique) { + // Ensure that absl::make_unique is not ambiguous with std::make_unique. + // In C++14 mode, the below call to make_unique has both types as candidates. + struct TakesStdType { + explicit TakesStdType(const std::vector &vec) {} + }; + using absl::make_unique; + make_unique(std::vector()); +} + #if 0 // TODO(billydonahue): Make a proper NC test. // These tests shouldn't compile. From dedb4eec6cf0addc26cc27b67c270aa5a478fcc5 Mon Sep 17 00:00:00 2001 From: Abseil Team Date: Tue, 24 Oct 2017 10:37:49 -0700 Subject: [PATCH 11/12] Changes imported from Abseil "staging" branch: - fbff677ef850865ea67ed6771a8ed348be181e8e Modify sysinfo.cc to support GetTID on Akaros. by Abseil Team - f1c2929e08a3d4181e08cb5014c4a569306fd922 Two functions that did not refer to their arguments unles... by Abseil Team - ee43cc3bfdb4d84d40eee31fb25ecdc1aa060f47 Support Akaros (https://akaros.org) in the ABSL spinlock_... by Abseil Team - 6869c8c5253126459d6c7f0aa708d8612c8e5963 Make sure vdso_base_ is constant-intialized. by Abseil Team - d54e0366efc8d44cd5da5fd157734da966dc45e8 Add missing include for assert used by ABSL_ASSERT. by Derek Mauro - a5139775f3917bb5201e7fc838135766daa05b8d When building against GLIBC-2.16 or newer, use getauxval(... by Abseil Team GitOrigin-RevId: fbff677ef850865ea67ed6771a8ed348be181e8e Change-Id: Ie3549f6ef054783dd104304d2faf8d9800c16b83 --- absl/base/BUILD.bazel | 1 + absl/base/internal/spinlock_akaros.inc | 35 +++++++++ absl/base/internal/spinlock_wait.cc | 2 + absl/base/internal/sysinfo.cc | 24 ++++++ absl/base/macros.h | 1 + absl/debugging/internal/elf_mem_image.cc | 5 +- absl/debugging/internal/elf_mem_image.h | 7 +- .../debugging/internal/stacktrace_x86-inl.inc | 4 +- absl/debugging/internal/vdso_support.cc | 73 +++++++++++-------- 9 files changed, 119 insertions(+), 33 deletions(-) create mode 100644 absl/base/internal/spinlock_akaros.inc diff --git a/absl/base/BUILD.bazel b/absl/base/BUILD.bazel index d68448da..e68c4500 100644 --- a/absl/base/BUILD.bazel +++ b/absl/base/BUILD.bazel @@ -28,6 +28,7 @@ licenses(["notice"]) # Apache 2.0 cc_library( name = "spinlock_wait", srcs = [ + "internal/spinlock_akaros.inc", "internal/spinlock_posix.inc", "internal/spinlock_wait.cc", "internal/spinlock_win32.inc", diff --git a/absl/base/internal/spinlock_akaros.inc b/absl/base/internal/spinlock_akaros.inc new file mode 100644 index 00000000..051c8cf8 --- /dev/null +++ b/absl/base/internal/spinlock_akaros.inc @@ -0,0 +1,35 @@ +// Copyright 2017 The Abseil Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// This file is an Akaros-specific part of spinlock_wait.cc + +#include + +#include "absl/base/internal/scheduling_mode.h" + +extern "C" { + +ABSL_ATTRIBUTE_WEAK void AbslInternalSpinLockDelay( + std::atomic* /* lock_word */, uint32_t /* value */, + int /* loop */, absl::base_internal::SchedulingMode /* mode */) { + // In Akaros, one must take care not to call anything that could cause a + // malloc(), a blocking system call, or a uthread_yield() while holding a + // spinlock. Our callers assume will not call into libraries or other + // arbitrary code. +} + +ABSL_ATTRIBUTE_WEAK void AbslInternalSpinLockWake( + std::atomic* /* lock_word */, bool /* all */) {} + +} // extern "C" diff --git a/absl/base/internal/spinlock_wait.cc b/absl/base/internal/spinlock_wait.cc index 0fd36286..8f951b66 100644 --- a/absl/base/internal/spinlock_wait.cc +++ b/absl/base/internal/spinlock_wait.cc @@ -23,6 +23,8 @@ #if defined(_WIN32) #include "absl/base/internal/spinlock_win32.inc" +#elif defined(__akaros__) +#include "absl/base/internal/spinlock_akaros.inc" #else #include "absl/base/internal/spinlock_posix.inc" #endif diff --git a/absl/base/internal/sysinfo.cc b/absl/base/internal/sysinfo.cc index 9e0140fa..00e98b66 100644 --- a/absl/base/internal/sysinfo.cc +++ b/absl/base/internal/sysinfo.cc @@ -284,6 +284,30 @@ pid_t GetTID() { return syscall(SYS_gettid); } +#elif defined(__akaros__) + +pid_t GetTID() { + // Akaros has a concept of "vcore context", which is the state the program + // is forced into when we need to make a user-level scheduling decision, or + // run a signal handler. This is analogous to the interrupt context that a + // CPU might enter if it encounters some kind of exception. + // + // There is no current thread context in vcore context, but we need to give + // a reasonable answer if asked for a thread ID (e.g., in a signal handler). + // Thread 0 always exists, so if we are in vcore context, we return that. + // + // Otherwise, we know (since we are using pthreads) that the uthread struct + // current_uthread is pointing to is the first element of a + // struct pthread_tcb, so we extract and return the thread ID from that. + // + // TODO(dcross): Akaros anticipates moving the thread ID to the uthread + // structure at some point. We should modify this code to remove the cast + // when that happens. + if (in_vcore_context()) + return 0; + return reinterpret_cast(current_uthread)->id; +} + #else // Fallback implementation of GetTID using pthread_getspecific. diff --git a/absl/base/macros.h b/absl/base/macros.h index 31d1c02e..d4140872 100644 --- a/absl/base/macros.h +++ b/absl/base/macros.h @@ -29,6 +29,7 @@ #ifndef ABSL_BASE_MACROS_H_ #define ABSL_BASE_MACROS_H_ +#include #include #include "absl/base/port.h" diff --git a/absl/debugging/internal/elf_mem_image.cc b/absl/debugging/internal/elf_mem_image.cc index f6c6bc07..3dfef5e8 100644 --- a/absl/debugging/internal/elf_mem_image.cc +++ b/absl/debugging/internal/elf_mem_image.cc @@ -75,8 +75,9 @@ const T *GetTableElement(const ElfW(Ehdr) * ehdr, ElfW(Off) table_offset, } // namespace -const void *const ElfMemImage::kInvalidBase = - reinterpret_cast(~0L); +// The value of this variable doesn't matter; it's used only for its +// unique address. +const int ElfMemImage::kInvalidBaseSentinel = 0; ElfMemImage::ElfMemImage(const void *base) { ABSL_RAW_CHECK(base != kInvalidBase, "bad pointer"); diff --git a/absl/debugging/internal/elf_mem_image.h b/absl/debugging/internal/elf_mem_image.h index 7f3dbb97..20a32a49 100644 --- a/absl/debugging/internal/elf_mem_image.h +++ b/absl/debugging/internal/elf_mem_image.h @@ -43,9 +43,14 @@ namespace debug_internal { // An in-memory ELF image (may not exist on disk). class ElfMemImage { + private: + // Sentinel: there could never be an elf image at &kInvalidBaseSentinel. + static const int kInvalidBaseSentinel; + public: // Sentinel: there could never be an elf image at this address. - static const void *const kInvalidBase; + static constexpr const void *const kInvalidBase = + static_cast(&kInvalidBaseSentinel); // Information about a single vdso symbol. // All pointers are into .dynsym, .dynstr, or .text of the VDSO. diff --git a/absl/debugging/internal/stacktrace_x86-inl.inc b/absl/debugging/internal/stacktrace_x86-inl.inc index 6e1af017..9bdaa542 100644 --- a/absl/debugging/internal/stacktrace_x86-inl.inc +++ b/absl/debugging/internal/stacktrace_x86-inl.inc @@ -114,7 +114,9 @@ static const int kMaxFrameBytes = 100000; // vuc is a ucontext_t *. We use void* to avoid the use // of ucontext_t on non-POSIX systems. static uintptr_t GetFP(const void *vuc) { -#if defined(__linux__) +#if !defined(__linux__) + static_cast(vuc); // Avoid an unused argument compiler warning. +#else if (vuc != nullptr) { auto *uc = reinterpret_cast(vuc); #if defined(__i386__) diff --git a/absl/debugging/internal/vdso_support.cc b/absl/debugging/internal/vdso_support.cc index 5026e1c1..815e702f 100644 --- a/absl/debugging/internal/vdso_support.cc +++ b/absl/debugging/internal/vdso_support.cc @@ -20,10 +20,15 @@ #ifdef ABSL_HAVE_VDSO_SUPPORT // defined in vdso_support.h +#include #include #include #include +#if __GLIBC_PREREQ(2, 16) // GLIBC-2.16 implements getauxval. +#include +#endif + #include "absl/base/dynamic_annotations.h" #include "absl/base/internal/raw_logging.h" #include "absl/base/port.h" @@ -35,8 +40,10 @@ namespace absl { namespace debug_internal { +ABSL_CONST_INIT std::atomic VDSOSupport::vdso_base_( debug_internal::ElfMemImage::kInvalidBase); + std::atomic VDSOSupport::getcpu_fn_(&InitAndGetCPU); VDSOSupport::VDSOSupport() // If vdso_base_ is still set to kInvalidBase, we got here @@ -56,37 +63,44 @@ VDSOSupport::VDSOSupport() // Finally, even if there is a race here, it is harmless, because // the operation should be idempotent. const void *VDSOSupport::Init() { - if (vdso_base_.load(std::memory_order_relaxed) == - debug_internal::ElfMemImage::kInvalidBase) { - { - // Valgrind zaps AT_SYSINFO_EHDR and friends from the auxv[] - // on stack, and so glibc works as if VDSO was not present. - // But going directly to kernel via /proc/self/auxv below bypasses - // Valgrind zapping. So we check for Valgrind separately. - if (RunningOnValgrind()) { - vdso_base_.store(nullptr, std::memory_order_relaxed); - getcpu_fn_.store(&GetCPUViaSyscall, std::memory_order_relaxed); - return nullptr; - } - int fd = open("/proc/self/auxv", O_RDONLY); - if (fd == -1) { - // Kernel too old to have a VDSO. - vdso_base_.store(nullptr, std::memory_order_relaxed); - getcpu_fn_.store(&GetCPUViaSyscall, std::memory_order_relaxed); - return nullptr; - } - ElfW(auxv_t) aux; - while (read(fd, &aux, sizeof(aux)) == sizeof(aux)) { - if (aux.a_type == AT_SYSINFO_EHDR) { - vdso_base_.store(reinterpret_cast(aux.a_un.a_val), - std::memory_order_relaxed); - break; - } + const auto kInvalidBase = debug_internal::ElfMemImage::kInvalidBase; +#if __GLIBC_PREREQ(2, 16) + if (vdso_base_.load(std::memory_order_relaxed) == kInvalidBase) { + errno = 0; + const void *const sysinfo_ehdr = + reinterpret_cast(getauxval(AT_SYSINFO_EHDR)); + if (errno == 0) { + vdso_base_.store(sysinfo_ehdr, std::memory_order_relaxed); + } + } +#endif // __GLIBC_PREREQ(2, 16) + if (vdso_base_.load(std::memory_order_relaxed) == kInvalidBase) { + // Valgrind zaps AT_SYSINFO_EHDR and friends from the auxv[] + // on stack, and so glibc works as if VDSO was not present. + // But going directly to kernel via /proc/self/auxv below bypasses + // Valgrind zapping. So we check for Valgrind separately. + if (RunningOnValgrind()) { + vdso_base_.store(nullptr, std::memory_order_relaxed); + getcpu_fn_.store(&GetCPUViaSyscall, std::memory_order_relaxed); + return nullptr; + } + int fd = open("/proc/self/auxv", O_RDONLY); + if (fd == -1) { + // Kernel too old to have a VDSO. + vdso_base_.store(nullptr, std::memory_order_relaxed); + getcpu_fn_.store(&GetCPUViaSyscall, std::memory_order_relaxed); + return nullptr; + } + ElfW(auxv_t) aux; + while (read(fd, &aux, sizeof(aux)) == sizeof(aux)) { + if (aux.a_type == AT_SYSINFO_EHDR) { + vdso_base_.store(reinterpret_cast(aux.a_un.a_val), + std::memory_order_relaxed); + break; } - close(fd); } - if (vdso_base_.load(std::memory_order_relaxed) == - debug_internal::ElfMemImage::kInvalidBase) { + close(fd); + if (vdso_base_.load(std::memory_order_relaxed) == kInvalidBase) { // Didn't find AT_SYSINFO_EHDR in auxv[]. vdso_base_.store(nullptr, std::memory_order_relaxed); } @@ -135,6 +149,7 @@ long VDSOSupport::GetCPUViaSyscall(unsigned *cpu, // NOLINT(runtime/int) return syscall(SYS_getcpu, cpu, nullptr, nullptr); #else // x86_64 never implemented sys_getcpu(), except as a VDSO call. + static_cast(cpu); // Avoid an unused argument compiler warning. errno = ENOSYS; return -1; #endif From 0fece732a21c5ae8fef5fa8b3f0b8487bca68d83 Mon Sep 17 00:00:00 2001 From: Abseil Team Date: Thu, 26 Oct 2017 11:22:13 -0700 Subject: [PATCH 12/12] Changes imported from Abseil "staging" branch: - 5b675ef65e4977b3ac778a75a21e99db1ebe78e7 Remove "not an official google project" disclaimer. by Abseil Team - 77d2aacc03efe6841612b38bcbb745dde1ad7d3e Avoid weak virtual table warnings (-Wweak-vtables) and re... by Abseil Team GitOrigin-RevId: 5b675ef65e4977b3ac778a75a21e99db1ebe78e7 Change-Id: Ia0d1d6e39169c7ad9783d25dc92dad041de3a966 --- README.md | 4 ---- absl/base/internal/malloc_extension.cc | 7 +++++++ absl/base/internal/malloc_extension.h | 7 ++++--- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index c6db64ab..9d75b67d 100644 --- a/README.md +++ b/README.md @@ -84,7 +84,3 @@ For more information about Abseil: * Peruse our [Abseil Compatibility Guarantees](http://abseil.io/about/compatibility) to understand both what we promise to you, and what we expect of you in return. - -## Disclaimer - -* This is not an official Google product. diff --git a/absl/base/internal/malloc_extension.cc b/absl/base/internal/malloc_extension.cc index 3da981ce..d48ec5bc 100644 --- a/absl/base/internal/malloc_extension.cc +++ b/absl/base/internal/malloc_extension.cc @@ -29,6 +29,13 @@ namespace base_internal { SysAllocator::~SysAllocator() {} void SysAllocator::GetStats(char* buffer, int) { buffer[0] = 0; } +// Dummy key method to avoid weak vtable. +void MallocExtensionWriter::UnusedKeyMethod() {} + +void StringMallocExtensionWriter::Write(const char* buf, int len) { + out_->append(buf, len); +} + // Default implementation -- does nothing MallocExtension::~MallocExtension() { } bool MallocExtension::VerifyAllMemory() { return true; } diff --git a/absl/base/internal/malloc_extension.h b/absl/base/internal/malloc_extension.h index 46b767ff..75a00ce9 100644 --- a/absl/base/internal/malloc_extension.h +++ b/absl/base/internal/malloc_extension.h @@ -388,6 +388,9 @@ class MallocExtensionWriter { MallocExtensionWriter() {} MallocExtensionWriter(const MallocExtensionWriter&) = delete; MallocExtensionWriter& operator=(const MallocExtensionWriter&) = delete; + + private: + virtual void UnusedKeyMethod(); // Dummy key method to avoid weak vtable. }; // A subclass that writes to the std::string "out". NOTE: The generated @@ -396,9 +399,7 @@ class MallocExtensionWriter { class StringMallocExtensionWriter : public MallocExtensionWriter { public: explicit StringMallocExtensionWriter(std::string* out) : out_(out) {} - virtual void Write(const char* buf, int len) { - out_->append(buf, len); - } + void Write(const char* buf, int len) override; private: std::string* const out_;