From 28964186e78366fc13a8028bb348a42af1c1a12d Mon Sep 17 00:00:00 2001 From: Erwin Jansen Date: Fri, 22 Mar 2024 13:56:38 -0700 Subject: Deprecate LazyInstance It will no longer compile in C++20. Use the NoDestructor approach introduced by chromium instead. See: https://github.com/microsoft/STL/issues/661 for an extended discussion of why this was changed. Change-Id: Icc15480b1a0262467a568e5c9d7ca56f31e8c119 --- .gitignore | 1 + base/BUILD.bazel | 22 +++++++++- base/NoDestructor_unittest.cpp | 48 ++++++++++++++++++++++ base/include/aemu/base/memory/LazyInstance.h | 16 +++++++- base/include/aemu/base/memory/NoDestructor.h | 60 ++++++++++++++++++++++++++++ host-common/BUILD.bazel | 34 +++------------- 6 files changed, 150 insertions(+), 31 deletions(-) create mode 100644 base/NoDestructor_unittest.cpp create mode 100644 base/include/aemu/base/memory/NoDestructor.h diff --git a/.gitignore b/.gitignore index 4a3faac..4bf2b0c 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,4 @@ +.cache .vscode /build*/ /install*/ diff --git a/base/BUILD.bazel b/base/BUILD.bazel index d199fed..fb16bc9 100644 --- a/base/BUILD.bazel +++ b/base/BUILD.bazel @@ -13,7 +13,10 @@ cc_library( }), includes = ["include"], visibility = ["//visibility:public"], - deps = ["//hardware/google/aemu/host-common:aemu-host-common-headers"], + deps = [ + "//hardware/google/aemu/host-common:aemu-host-common-headers", + "@com_google_absl//absl/strings:str_format", + ], ) cc_library( @@ -35,7 +38,12 @@ objc_library( srcs = [ "system-native-mac.mm", ], + sdk_frameworks = [ + "IOkit", + "AppKit", + ], deps = [":aemu-base-headers"], + alwayslink = True, ) cc_library( @@ -96,6 +104,11 @@ cc_library( "@platforms//os:windows": [ "-DEFAULTLIB:Shlwapi.lib", ], + "@platforms//os:macos": [ + "-framework Foundation", + "-framework AppKit", + "-framework IOKit", + ], "//conditions:default": [], }), visibility = ["//visibility:public"], @@ -103,6 +116,7 @@ cc_library( ":aemu-base-headers", ":aemu-base-metrics", "//external/lz4", + "//hardware/google/aemu/host-common:logging", ] + select({ "@platforms//os:macos": [ ":aemu-base-darwin", @@ -121,6 +135,7 @@ cc_test( "LayoutResolver_unittest.cpp", "LruCache_unittest.cpp", "ManagedDescriptor_unittest.cpp", + "NoDestructor_unittest.cpp", "Optional_unittest.cpp", "StringFormat_unittest.cpp", "SubAllocator_unittest.cpp", @@ -135,7 +150,12 @@ cc_test( deps = [ ":aemu-base", ":aemu-base-headers", + "//hardware/google/aemu/base:aemu-base-darwin", + "//hardware/google/aemu/base:aemu-base-metrics", "//hardware/google/aemu/host-common:logging", + "@com_google_absl//absl/log", + "@com_google_absl//absl/strings", + "@com_google_absl//absl/strings:str_format", "@com_google_googletest//:gtest_main", ], ) diff --git a/base/NoDestructor_unittest.cpp b/base/NoDestructor_unittest.cpp new file mode 100644 index 0000000..ee563ce --- /dev/null +++ b/base/NoDestructor_unittest.cpp @@ -0,0 +1,48 @@ +// Copyright 2018 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. +#include "aemu/base/memory/NoDestructor.h" +#include +#include +#include +#include "aemu/base/Log.h" + +namespace android::base { +namespace { +struct CheckOnDestroy { + ~CheckOnDestroy() { dfatal("Destructor was called"); } +}; +TEST(NoDestructorTest, SkipsDestructors) { + NoDestructor destructor_should_not_run; +} +struct CopyOnly { + CopyOnly() = default; + CopyOnly(const CopyOnly&) = default; + CopyOnly& operator=(const CopyOnly&) = default; + CopyOnly(CopyOnly&&) = delete; + CopyOnly& operator=(CopyOnly&&) = delete; +}; +struct MoveOnly { + MoveOnly() = default; + MoveOnly(const MoveOnly&) = delete; + MoveOnly& operator=(const MoveOnly&) = delete; + MoveOnly(MoveOnly&&) = default; + MoveOnly& operator=(MoveOnly&&) = default; +}; +struct ForwardingTestStruct { + ForwardingTestStruct(const CopyOnly&, MoveOnly&&) {} +}; +TEST(NoDestructorTest, ForwardsArguments) { + CopyOnly copy_only; + MoveOnly move_only; + static NoDestructor test_forwarding( + copy_only, std::move(move_only)); +} +TEST(NoDestructorTest, Accessors) { + static NoDestructor awesome("awesome"); + EXPECT_EQ("awesome", *awesome); + EXPECT_EQ(0, awesome->compare("awesome")); + EXPECT_EQ(0, awesome.get()->compare("awesome")); +} +} // namespace +} // namespace base \ No newline at end of file diff --git a/base/include/aemu/base/memory/LazyInstance.h b/base/include/aemu/base/memory/LazyInstance.h index c859e17..c50baff 100644 --- a/base/include/aemu/base/memory/LazyInstance.h +++ b/base/include/aemu/base/memory/LazyInstance.h @@ -94,6 +94,20 @@ using is_trivially_default_constructible = // Atomic state variable type. Used to ensure to synchronize concurrent // initialization and access without incurring the full cost of a mutex // lock/unlock. + +// !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! DEPRECATED !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! +// Please don't use this macro. Use a function-local static of type +// android::base::NoDestructor instead: +// +// Factory& Factory::GetInstance() { +// static base::NoDestructor instance; +// return *instance; +// } +// +// From C++20 onwards this will no longer compile. See: +// See https://github.com/microsoft/STL/issues/661 +// For details on issues and guarantees around atomic initialization. +// !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! struct LazyInstanceState { enum class State : char { Init = 0, @@ -180,7 +194,7 @@ T* LazyInstance::ptrInternal() const { static_assert(std::is_standard_layout::value, "LazyInstance is not a standard layout type"); #ifndef _MSC_VER - // These checks are not working in vs2019.. + // These checks are not working in vs2019.. static_assert( internal::is_trivially_default_constructible::value, "LazyInstance can't be trivially default constructed"); diff --git a/base/include/aemu/base/memory/NoDestructor.h b/base/include/aemu/base/memory/NoDestructor.h new file mode 100644 index 0000000..3219b6f --- /dev/null +++ b/base/include/aemu/base/memory/NoDestructor.h @@ -0,0 +1,60 @@ +// Copyright 2018 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. +#pragma once +#include +namespace android::base { +// A wrapper that makes it easy to create an object of type T with static +// storage duration that: +// - is only constructed on first access +// - never invokes the destructor +// in order to satisfy the styleguide ban on global constructors and +// destructors. +// +// Runtime constant example: +// const std::string& GetLineSeparator() { +// // Forwards to std::string(size_t, char, const Allocator&) constructor. +// static const base::NoDestructor s(5, '-'); +// return s; +// } +// +// More complex initialization with a lambda: +// const std::string& GetSessionNonce() { +// static const base::NoDestructor nonce([] { +// std::string s(16); +// crypto::RandString(s.data(), s.size()); +// return s; +// })()); +// return *nonce; +// } +// +// NoDestructor stores the object inline, so it also avoids a pointer +// indirection and a malloc. Code should prefer to use NoDestructor over: +// - The CR_DEFINE_STATIC_LOCAL() helper macro. +// - A function scoped static T* or T& that is dynamically initialized. +// - A global base::LazyInstance. +// +// Note that since the destructor is never run, this *will* leak memory if used +// as a stack or member variable. Furthermore, a NoDestructor should never +// have global scope as that may require a static initializer. +template +class NoDestructor { + public: + // Not constexpr; just write static constexpr T x = ...; if the value should + // be a constexpr. + template + explicit NoDestructor(Args&&... args) { + new (get()) T(std::forward(args)...); + } + ~NoDestructor() = default; + const T& operator*() const { return *get(); } + T& operator*() { return *get(); } + const T* operator->() const { return get(); } + T* operator->() { return get(); } + const T* get() const { return reinterpret_cast(&storage_); } + T* get() { return reinterpret_cast(&storage_); } + + private: + alignas(T) char storage_[sizeof(T)]; +}; +} // namespace android::base diff --git a/host-common/BUILD.bazel b/host-common/BUILD.bazel index 0be01e0..08cc1a4 100644 --- a/host-common/BUILD.bazel +++ b/host-common/BUILD.bazel @@ -106,40 +106,16 @@ cc_library( ) # Testing Libraries and Executable (conditional) -cc_library( - name = "aemu-host-common-testing-support", - srcs = [ - "testing/HostAddressSpace.cpp", - "testing/MockAndroidEmulatorWindowAgent.cpp", - "testing/MockAndroidMultiDisplayAgent.cpp", - "testing/MockAndroidVmOperations.cpp", - "testing/MockGraphicsAgentFactory.cpp", - ], - deps = [ - ":aemu-host-common-headers", - "//hardware/google/aemu/base:aemu-base-headers", - "@com_google_gmock//:gmock", - "@com_google_googletest//:gtest", - ], -) - cc_test( name = "aemu-host-common_unittests", - srcs = [ - "GfxstreamFatalError_unittest.cpp", - "HostAddressSpace_unittest.cpp", - "HostmemIdMapping_unittest.cpp", - "address_space_graphics_unittests.cpp", - "address_space_host_memory_allocator_unittests.cpp", - "address_space_shared_slots_host_memory_allocator_unittests.cpp", - "logging_unittest.cpp", - ], + srcs = glob([ + "*_unitests.cpp", + "testing/**", + ]), + includes = ["testing"], deps = [ ":aemu-host-common-headers", - ":aemu-host-common-testing-support", - ":gfxstream_host_common", ":logging", - "//common:gfxstream_base", "//hardware/google/aemu/base:aemu-base-headers", "@com_google_googletest//:gtest_main", ], -- cgit v1.2.3