diff options
| -rw-r--r-- | absl/FAQ.md | 144 | ||||
| -rw-r--r-- | absl/flags/BUILD.bazel | 1 | ||||
| -rw-r--r-- | absl/flags/config.h | 8 | ||||
| -rw-r--r-- | absl/flags/flag.cc | 7 | ||||
| -rw-r--r-- | absl/flags/flag.h | 15 | ||||
| -rw-r--r-- | absl/flags/flag_test.cc | 13 | ||||
| -rw-r--r-- | absl/flags/internal/commandlineflag.h | 19 | ||||
| -rw-r--r-- | absl/flags/internal/flag.cc | 45 | ||||
| -rw-r--r-- | absl/flags/internal/flag.h | 48 |
9 files changed, 229 insertions, 71 deletions
diff --git a/absl/FAQ.md b/absl/FAQ.md new file mode 100644 index 00000000..af721307 --- /dev/null +++ b/absl/FAQ.md @@ -0,0 +1,144 @@ +# Abseil FAQ + +## Is Abseil the right home for my utility library? + +Most often the answer to the question is "no." As both the [About +Abseil](https://abseil.io/about/) page and our [contributing +guidelines](https://github.com/abseil/abseil-cpp/blob/master/CONTRIBUTING.md#contribution-guidelines) +explain, Abseil contains a variety of core C++ library code that is widely used +at [Google](https://www.google.com/). As such, Abseil's primary purpose is to be +used as a dependency by Google's open source C++ projects. While we do hope that +Abseil is also useful to the C++ community at large, this added constraint also +means that we are unlikely to accept a contribution of utility code that isn't +already widely used by Google. + +## How to I set the C++ dialect used to build Abseil? + +The short answer is that whatever mechanism you choose, you need to make sure +that you set this option consistently at the global level for your entire +project. If, for example, you want to set the C++ dialect to C++17, with +[Bazel](https://bazel/build/) as the build system and `gcc` or `clang` as the +compiler, there several ways to do this: +* Pass `--cxxopt=-std=c++17` on the command line (for example, `bazel build + --cxxopt=-std=c++17 ...`) +* Set the environment variable `BAZEL_CXXOPTS` (for example, + `BAZEL_CXXOPTS=-std=c++17`) +* Add `build --cxxopt=-std=c++17` to your [`.bazelrc` + file](https://docs.bazel.build/versions/master/guide.html#bazelrc) + +If you are using CMake as the build system, you'll need to add a line like +`set(CMAKE_CXX_STANDARD 17)` to your top level `CMakeLists.txt` file. See the +[CMake build +instructions](https://github.com/abseil/abseil-cpp/blob/master/CMake/README.md) +for more information. + +For a longer answer to this question and to understand why some other approaches +don't work, see the answer to "What is ABI and why don't you recommend using a +pre-compiled version of Abseil?" + +## What is ABI and why don't you recommend using a pre-compiled version of Abseil? + +For the purposes of this discussion, you can think of +[ABI](https://en.wikipedia.org/wiki/Application_binary_interface) as the +compiled representation of the interfaces in code. This is in contrast to +[API](https://en.wikipedia.org/wiki/Application_programming_interface), which +you can think of as the interfaces as defined by the code itself. [Abseil has a +strong promise of API compatibility, but does not make any promise of ABI +compatibility](https://abseil.io/about/compatibility). Let's take a look at what +this means in practice. + +You might be tempted to do something like this in a +[Bazel](https://bazel.build/) `BUILD` file: + +``` +# DON'T DO THIS!!! +cc_library( + name = "my_library", + srcs = ["my_library.cc"], + copts = ["-std=c++17"], # May create a mixed-mode compile! + deps = ["@com_google_absl//absl/strings"], +) +``` + +Applying `-std=c++17` to an individual target in your `BUILD` file is going to +compile that specific target in C++17 mode, but it isn't going to ensure the +Abseil library is built in C++17 mode, since the Abseil library itself is a +different build target. If your code includes an Abseil header, then your +program may contain conflicting definitions of the same +class/function/variable/enum, etc. As a rule, all compile options that affect +the ABI of a program need to be applied to the entire build on a global basis. + +C++ has something called the [One Definition +Rule](https://en.wikipedia.org/wiki/One_Definition_Rule) (ODR). C++ doesn't +allow multiple definitions of the same class/function/variable/enum, etc. ODR +violations sometimes result in linker errors, but linkers do not always catch +violations. Uncaught ODR violations can result in strange runtime behaviors or +crashes that can be hard to debug. + +If you build the Abseil library and your code using different compile options +that affect ABI, there is a good chance you will run afoul of the One Definition +Rule. Examples of GCC compile options that affect ABI include (but aren't +limited to) language dialect (e.g. `-std=`), optimization level (e.g. `-O2`), +code generation flags (e.g. `-fexceptions`), and preprocessor defines +(e.g. `-DNDEBUG`). + +If you use a pre-compiled version of Abseil, (for example, from your Linux +distribution package manager or from something like +[vcpkg](https://github.com/microsoft/vcpkg)) you have to be very careful to +ensure ABI compatibility across the components of your program. The only way you +can be sure your program is going to be correct regarding ABI is to ensure +you've used the exact same compile options as were used to build the +pre-compiled library. This does not mean that Abseil cannot work as part of a +Linux distribution since a knowledgeable binary packager will have ensured that +all packages have been built with consistent compile options. This is one of the +reasons we warn against - though do not outright reject - using Abseil as a +pre-compiled library. + +Another possible way that you might afoul of ABI issues is if you accidentally +include two versions of Abseil in your program. Multiple versions of Abseil can +end up within the same binary if your program uses the Abseil library and +another library also transitively depends on Abseil (resulting in what is +sometimes called the diamond dependency problem). In cases such as this you must +structure your build so that all libraries use the same version of Abseil. +[Abseil's strong promise of API compatibility between +releases](https://abseil.io/about/compatibility) means the latest "HEAD" release +of Abseil is almost certainly the right choice if you are doing as we recommend +and building all of your code from source. + +For these reasons we recommend you avoid pre-compiled code and build the Abseil +library yourself in a consistent manner with the rest of your code. + +## What is "live at head" and how do I do it? + +From Abseil's point-of-view, "live at head" means that every Abseil source +release (which happens on an almost daily basis) is either API compatible with +the previous release, or comes with an automated tool that you can run over code +to make it compatible. In practice, the need to use an automated tool is +extremely rare. This means that upgrading from one source release to another +should be a routine practice that can and should be performed often. + +We recommend you update to the latest release of Abseil as often as +possible. Not only will you pick up bug fixes more quickly, but if you have good +automated testing, you will catch and be able to fix any [Hyrum's +Law](https://www.hyrumslaw.com/) dependency problems on an incremental basis +instead of being overwhelmed by them and having difficulty isolating them if you +wait longer between updates. + +If you are using the [Bazel](https://bazel.build/) build system and its +[external dependencies](https://docs.bazel.build/versions/master/external.html) +feature, updating the +[`http_archive`](https://docs.bazel.build/versions/master/repo/http.html#http_archive) +rule in your +[`WORKSPACE`](https://docs.bazel.build/versions/master/be/workspace.html) for +`com_google_abseil` to point to the latest release is all you need to do. You +can commit the updated `WORKSPACE` file to your source control every time you +update, and if you have good automated testing, you might even consider +automating this. + +One thing we don't recommend is using GitHub's `master.zip` files (for example +[https://github.com/abseil/abseil-cpp/archive/master.zip](https://github.com/abseil/abseil-cpp/archive/master.zip)), +which are always the latest commit in the `master` branch, to implement live at +head. Since these `master.zip` URLs are not versioned, you will lose build +reproducibility. In addition, some build systems, including Bazel, will simply +cache this file, which means you won't actually be updating to the latest +release until your cache is cleared or invalidated. diff --git a/absl/flags/BUILD.bazel b/absl/flags/BUILD.bazel index 03833d44..d2ca5c6f 100644 --- a/absl/flags/BUILD.bazel +++ b/absl/flags/BUILD.bazel @@ -138,6 +138,7 @@ cc_library( "//absl/flags:__pkg__", ], deps = [ + ":config", ":marshalling", "//absl/base:config", "//absl/base:core_headers", diff --git a/absl/flags/config.h b/absl/flags/config.h index fbe34961..001f8fea 100644 --- a/absl/flags/config.h +++ b/absl/flags/config.h @@ -56,4 +56,12 @@ #define ABSL_FLAGS_INTERNAL_ATOMIC_DOUBLE_WORD 1 #endif +// ABSL_FLAGS_INTERNAL_HAS_RTTI macro is used for selecting if we can use RTTI +// for flag type identification. +#ifdef ABSL_FLAGS_INTERNAL_HAS_RTTI +#error ABSL_FLAGS_INTERNAL_HAS_RTTI cannot be directly set +#elif !defined(__GNUC__) || defined(__GXX_RTTI) +#define ABSL_FLAGS_INTERNAL_HAS_RTTI 1 +#endif // !defined(__GNUC__) || defined(__GXX_RTTI) + #endif // ABSL_FLAGS_CONFIG_H_ diff --git a/absl/flags/flag.cc b/absl/flags/flag.cc index e67f7304..f7a457bf 100644 --- a/absl/flags/flag.cc +++ b/absl/flags/flag.cc @@ -22,13 +22,6 @@ namespace absl { ABSL_NAMESPACE_BEGIN -#ifndef NDEBUG -#define ABSL_FLAGS_GET(T) \ - T GetFlag(const absl::Flag<T>& flag) { return flag.Get(); } -ABSL_FLAGS_INTERNAL_BUILTIN_TYPES(ABSL_FLAGS_GET) -#undef ABSL_FLAGS_GET -#endif - // This global mutex protects on-demand construction of flag objects in MSVC // builds. #if defined(_MSC_VER) && !defined(__clang__) diff --git a/absl/flags/flag.h b/absl/flags/flag.h index 782dee2e..274838cb 100644 --- a/absl/flags/flag.h +++ b/absl/flags/flag.h @@ -191,21 +191,6 @@ ABSL_MUST_USE_RESULT T GetFlag(const absl::Flag<T>& flag) { return flag.Get(); } -#ifndef NDEBUG -// We want to validate the type mismatch between type definition and -// declaration. The lock-free implementation does not allow us to do it, -// so in debug builds we always use the slower implementation, which always -// validates the type. - -// We currently need an external linkage for built-in types because shared -// libraries have different addresses of flags_internal::FlagOps<T> which -// might cause log spam when checking the same flag type. -#define ABSL_FLAGS_INTERNAL_BUILT_IN_EXPORT(T) \ - ABSL_MUST_USE_RESULT T GetFlag(const absl::Flag<T>& flag); -ABSL_FLAGS_INTERNAL_BUILTIN_TYPES(ABSL_FLAGS_INTERNAL_BUILT_IN_EXPORT) -#undef ABSL_FLAGS_INTERNAL_BUILT_IN_EXPORT -#endif - // SetFlag() // // Sets the value of an `absl::Flag` to the value `v`. Do not construct an diff --git a/absl/flags/flag_test.cc b/absl/flags/flag_test.cc index 6722329c..6429a3e1 100644 --- a/absl/flags/flag_test.cc +++ b/absl/flags/flag_test.cc @@ -387,19 +387,20 @@ TEST_F(FlagTest, TestCustomUDT) { // MSVC produces link error on the type mismatch. // Linux does not have build errors and validations work as expected. -#if 0 // !defined(_WIN32) && GTEST_HAS_DEATH_TEST +#if !defined(_WIN32) && GTEST_HAS_DEATH_TEST -TEST(Flagtest, TestTypeMismatchValidations) { - // For builtin types, GetFlag() only does validation in debug mode. +using FlagDeathTest = FlagTest; + +TEST_F(FlagDeathTest, TestTypeMismatchValidations) { EXPECT_DEBUG_DEATH( - absl::GetFlag(FLAGS_mistyped_int_flag), + static_cast<void>(absl::GetFlag(FLAGS_mistyped_int_flag)), "Flag 'mistyped_int_flag' is defined as one type and declared " "as another"); - EXPECT_DEATH(absl::SetFlag(&FLAGS_mistyped_int_flag, 0), + EXPECT_DEATH(absl::SetFlag(&FLAGS_mistyped_int_flag, 1), "Flag 'mistyped_int_flag' is defined as one type and declared " "as another"); - EXPECT_DEATH(absl::GetFlag(FLAGS_mistyped_string_flag), + EXPECT_DEATH(static_cast<void>(absl::GetFlag(FLAGS_mistyped_string_flag)), "Flag 'mistyped_string_flag' is defined as one type and " "declared as another"); EXPECT_DEATH( diff --git a/absl/flags/internal/commandlineflag.h b/absl/flags/internal/commandlineflag.h index 4bc0c12f..6a0b5fad 100644 --- a/absl/flags/internal/commandlineflag.h +++ b/absl/flags/internal/commandlineflag.h @@ -21,9 +21,11 @@ #include <memory> #include <string> +#include <typeinfo> #include "absl/base/config.h" #include "absl/base/macros.h" +#include "absl/flags/config.h" #include "absl/flags/marshalling.h" #include "absl/strings/string_view.h" #include "absl/types/optional.h" @@ -41,7 +43,10 @@ enum FlagOp { kCopyConstruct, kSizeof, kParse, - kUnparse + kUnparse, +#if defined(ABSL_FLAGS_INTERNAL_HAS_RTTI) + kRuntimeTypeId +#endif }; using FlagOpFn = void* (*)(FlagOp, const void*, void*); using FlagMarshallingOpFn = void* (*)(FlagOp, const void*, void*, void*); @@ -84,6 +89,11 @@ void* FlagOps(FlagOp op, const void* v1, void* v2) { return nullptr; case kSizeof: return reinterpret_cast<void*>(sizeof(T)); +#if defined(ABSL_FLAGS_INTERNAL_HAS_RTTI) + case kRuntimeTypeId: + return const_cast<std::type_info*>(&typeid(T)); + break; +#endif default: return nullptr; } @@ -146,6 +156,13 @@ inline size_t Sizeof(FlagOpFn op) { op(flags_internal::kSizeof, nullptr, nullptr))); } +#if defined(ABSL_FLAGS_INTERNAL_HAS_RTTI) +inline const std::type_info& RuntimeTypeId(FlagOpFn op) { + return *static_cast<const std::type_info*>( + op(flags_internal::kRuntimeTypeId, nullptr, nullptr)); +} +#endif + // Handle to FlagState objects. Specific flag state objects will restore state // of a flag produced this flag state from method CommandLineFlag::SaveState(). class FlagStateInterface { diff --git a/absl/flags/internal/flag.cc b/absl/flags/internal/flag.cc index 6ce7def2..cfc0cf4d 100644 --- a/absl/flags/internal/flag.cc +++ b/absl/flags/internal/flag.cc @@ -56,6 +56,14 @@ bool ShouldValidateFlagValue(FlagOpFn flag_type_id) { return true; } +#if defined(ABSL_FLAGS_INTERNAL_HAS_RTTI) +bool MatchRuntimeTypeId(FlagOpFn lhs_type_id, FlagOpFn rhs_type_id) { + return RuntimeTypeId(lhs_type_id) == RuntimeTypeId(rhs_type_id); +} +#else +bool MatchRuntimeTypeId(FlagOpFn, FlagOpFn) { return true; } +#endif + // RAII helper used to temporarily unlock and relock `absl::Mutex`. // This is used when we need to ensure that locks are released while // invoking user supplied callbacks and then reacquired, since callbacks may @@ -133,6 +141,18 @@ void FlagImpl::Destroy() { is_data_guard_inited_ = false; } +void FlagImpl::AssertValidType(const flags_internal::FlagOpFn op) const { + // `op` is the unmarshaling operation corresponding to the declaration + // visibile at the call site. `op_` is the Flag's defined unmarshalling + // operation. They must match for this operation to be well-defined. + if (ABSL_PREDICT_FALSE(op != op_) && !MatchRuntimeTypeId(op, op_)) { + ABSL_INTERNAL_LOG( + FATAL, + absl::StrCat("Flag '", Name(), + "' is defined as one type and declared as another")); + } +} + std::unique_ptr<void, DynValueDeleter> FlagImpl::MakeInitValue() const { void* res = nullptr; if (DefaultKind() == FlagDefaultKind::kDynamicValue) { @@ -219,7 +239,7 @@ bool FlagImpl::RestoreState(const void* value, bool modified, if (counter_ == counter) return false; } - Write(value, op_); + Write(value); { absl::MutexLock l(DataGuard()); @@ -254,18 +274,9 @@ bool FlagImpl::TryParse(void** dst, absl::string_view value, return true; } -void FlagImpl::Read(void* dst, const flags_internal::FlagOpFn dst_op) const { +void FlagImpl::Read(void* dst) const { absl::ReaderMutexLock l(DataGuard()); - // `dst_op` is the unmarshaling operation corresponding to the declaration - // visibile at the call site. `op` is the Flag's defined unmarshalling - // operation. They must match for this operation to be well-defined. - if (ABSL_PREDICT_FALSE(dst_op != op_)) { - ABSL_INTERNAL_LOG( - ERROR, - absl::StrCat("Flag '", Name(), - "' is defined as one type and declared as another")); - } CopyConstruct(op_, value_.dynamic, dst); } @@ -286,19 +297,9 @@ void FlagImpl::StoreAtomic() { #endif } -void FlagImpl::Write(const void* src, const flags_internal::FlagOpFn src_op) { +void FlagImpl::Write(const void* src) { absl::MutexLock l(DataGuard()); - // `src_op` is the marshalling operation corresponding to the declaration - // visible at the call site. `op` is the Flag's defined marshalling operation. - // They must match for this operation to be well-defined. - if (ABSL_PREDICT_FALSE(src_op != op_)) { - ABSL_INTERNAL_LOG( - ERROR, - absl::StrCat("Flag '", Name(), - "' is defined as one type and declared as another")); - } - if (ShouldValidateFlagValue(op_)) { void* obj = Clone(op_, src); std::string ignored_error; diff --git a/absl/flags/internal/flag.h b/absl/flags/internal/flag.h index b5471fa8..c6c4a2f7 100644 --- a/absl/flags/internal/flag.h +++ b/absl/flags/internal/flag.h @@ -301,41 +301,44 @@ class FlagImpl { bool IsSpecifiedOnCommandLine() const ABSL_LOCKS_EXCLUDED(*DataGuard()); std::string DefaultValue() const ABSL_LOCKS_EXCLUDED(*DataGuard()); std::string CurrentValue() const ABSL_LOCKS_EXCLUDED(*DataGuard()); - void Read(void* dst, const FlagOpFn dst_op) const - ABSL_LOCKS_EXCLUDED(*DataGuard()); + void Read(void* dst) const ABSL_LOCKS_EXCLUDED(*DataGuard()); // Attempts to parse supplied `value` std::string. If parsing is successful, then // it replaces `dst` with the new value. bool TryParse(void** dst, absl::string_view value, std::string* err) const ABSL_EXCLUSIVE_LOCKS_REQUIRED(*DataGuard()); -#ifndef NDEBUG - template <typename T> - void Get(T* dst) const { - Read(dst, &FlagOps<T>); - } -#else template <typename T, typename std::enable_if< !IsAtomicFlagTypeTrait<T>::value, int>::type = 0> void Get(T* dst) const { - Read(dst, &FlagOps<T>); + AssertValidType(&flags_internal::FlagOps<T>); + Read(dst); } // Overload for `GetFlag()` for types that support lock-free reads. template <typename T, typename std::enable_if<IsAtomicFlagTypeTrait<T>::value, int>::type = 0> void Get(T* dst) const { - using U = BestAtomicType<T>; - const typename U::type r = value_.atomics.template load<T>(); + // For flags of types which can be accessed "atomically" we want to avoid + // slowing down flag value access due to type validation. That's why + // this validation is hidden behind !NDEBUG +#ifndef NDEBUG + AssertValidType(&flags_internal::FlagOps<T>); +#endif + using U = flags_internal::BestAtomicType<T>; + typename U::type r = value_.atomics.template load<T>(); if (r != U::AtomicInit()) { std::memcpy(static_cast<void*>(dst), &r, sizeof(T)); } else { - Read(dst, &FlagOps<T>); + Read(dst); } } -#endif + template <typename T> + void Set(const T& src) { + AssertValidType(&flags_internal::FlagOps<T>); + Write(&src); + } // Mutating access methods - void Write(const void* src, const FlagOpFn src_op) - ABSL_LOCKS_EXCLUDED(*DataGuard()); + void Write(const void* src) ABSL_LOCKS_EXCLUDED(*DataGuard()); bool SetFromString(absl::string_view value, FlagSettingMode set_mode, ValueSource source, std::string* err) ABSL_LOCKS_EXCLUDED(*DataGuard()); @@ -383,6 +386,13 @@ class FlagImpl { ABSL_EXCLUSIVE_LOCKS_REQUIRED(*DataGuard()) { return static_cast<FlagDefaultKind>(def_kind_); } + // Used in read/write operations to validate source/target has correct type. + // For example if flag is declared as absl::Flag<int> FLAGS_foo, a call to + // absl::GetFlag(FLAGS_foo) validates that the type of FLAGS_foo is indeed + // int. To do that we pass the "assumed" type id (which is deduced from type + // int) as an argument `op`, which is in turn is validated against the type id + // stored in flag object by flag definition statement. + void AssertValidType(const flags_internal::FlagOpFn op) const; // Immutable flag's state. @@ -461,9 +471,7 @@ class Flag final : public flags_internal::CommandLineFlag { impl_.Get(&u.value); return std::move(u.value); } - - void Set(const T& v) { impl_.Write(&v, &FlagOps<T>); } - + void Set(const T& v) { impl_.Set(v); } void SetCallback(const FlagCallbackFunc mutation_callback) { impl_.SetCallback(mutation_callback); } @@ -509,10 +517,10 @@ class Flag final : public flags_internal::CommandLineFlag { void Destroy() override { impl_.Destroy(); } - void Read(void* dst) const override { impl_.Read(dst, &FlagOps<T>); } + void Read(void* dst) const override { impl_.Read(dst); } FlagOpFn TypeId() const override { return &FlagOps<T>; } - // Flag's implementation with value type abstracted out. + // Flag's data FlagImpl impl_; }; |