diff options
Diffstat (limited to 'absl/flags')
| -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 |
8 files changed, 85 insertions, 71 deletions
diff --git a/absl/flags/BUILD.bazel b/absl/flags/BUILD.bazel index 03833d440f55..d2ca5c6f8317 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 fbe349611f54..001f8feaf637 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 e67f7304c6ff..f7a457bf0ce2 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 782dee2ee8df..274838cbc7f0 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 6722329c1f12..6429a3e1e1b3 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 4bc0c12fc68c..6a0b5fad89c7 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 6ce7def2d3d2..cfc0cf4d93d1 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 b5471fa8a734..c6c4a2f7beb5 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_; }; |