Commit 530f4cc2 authored by Maksim Ivanov's avatar Maksim Ivanov Committed by Commit Bot

Fix support of C++ references in UMA macros

This CL fixes compilation errors caused when passing const-refs or
non-const-refs as value arguments for the UMA macros.


Before this CL, an attempt to pass a const-ref as a value resulted
in errors like this:

base/metrics/histogram_macros_internal.h:30:5: error: static_assert failed due to requirement 'sizeof(const TestEnum &) == 0' "enumerator must define kMaxValue enumerat
or to use this macro!"
    static_assert(
    ^
base/metrics/histogram_macros_unittest.cc:63:3: note: in instantiation of member function 'base::internal::EnumSizeTraits<const TestEnum &, void>::Count' requested here
  UMA_HISTOGRAM_ENUMERATION("Test.ScopedEnumeration3", value_ref);
  ^
base/metrics/histogram_macros.h:84:7: note: expanded from macro 'UMA_HISTOGRAM_ENUMERATION'
      INTERNAL_UMA_HISTOGRAM_ENUMERATION_DEDUCE_BOUNDARY)               \
      ^
In file included from ../../base/metrics/histogram_macros_unittest.cc:5:
In file included from ../../base/metrics/histogram_macros.h:10:
base/metrics/histogram_macros_internal.h:33:12: error: reference to type 'const TestEnum' requires an initializer
    return Enum();


Bug: none
Change-Id: I875ff783bda12cc30079bcf6188ff843b9b55a0c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1729593Reviewed-by: default avatarJesse Doherty <jwd@chromium.org>
Commit-Queue: Maksim Ivanov <emaxx@chromium.org>
Cr-Commit-Position: refs/heads/master@{#682898}
parent 822ee7c8
...@@ -145,9 +145,9 @@ struct EnumSizeTraits< ...@@ -145,9 +145,9 @@ struct EnumSizeTraits<
#define INTERNAL_HISTOGRAM_EXACT_LINEAR_WITH_FLAG(name, sample, boundary, \ #define INTERNAL_HISTOGRAM_EXACT_LINEAR_WITH_FLAG(name, sample, boundary, \
flag) \ flag) \
do { \ do { \
static_assert(!std::is_enum<decltype(sample)>::value, \ static_assert(!std::is_enum<std::decay_t<decltype(sample)>>::value, \
"|sample| should not be an enum type!"); \ "|sample| should not be an enum type!"); \
static_assert(!std::is_enum<decltype(boundary)>::value, \ static_assert(!std::is_enum<std::decay_t<decltype(boundary)>>::value, \
"|boundary| should not be an enum type!"); \ "|boundary| should not be an enum type!"); \
STATIC_HISTOGRAM_POINTER_BLOCK( \ STATIC_HISTOGRAM_POINTER_BLOCK( \
name, Add(sample), \ name, Add(sample), \
...@@ -161,9 +161,9 @@ struct EnumSizeTraits< ...@@ -161,9 +161,9 @@ struct EnumSizeTraits<
#define INTERNAL_HISTOGRAM_SCALED_EXACT_LINEAR_WITH_FLAG( \ #define INTERNAL_HISTOGRAM_SCALED_EXACT_LINEAR_WITH_FLAG( \
name, sample, count, boundary, scale, flag) \ name, sample, count, boundary, scale, flag) \
do { \ do { \
static_assert(!std::is_enum<decltype(sample)>::value, \ static_assert(!std::is_enum<std::decay_t<decltype(sample)>>::value, \
"|sample| should not be an enum type!"); \ "|sample| should not be an enum type!"); \
static_assert(!std::is_enum<decltype(boundary)>::value, \ static_assert(!std::is_enum<std::decay_t<decltype(boundary)>>::value, \
"|boundary| should not be an enum type!"); \ "|boundary| should not be an enum type!"); \
class ScaledLinearHistogramInstance : public base::ScaledLinearHistogram { \ class ScaledLinearHistogramInstance : public base::ScaledLinearHistogram { \
public: \ public: \
...@@ -186,7 +186,8 @@ struct EnumSizeTraits< ...@@ -186,7 +186,8 @@ struct EnumSizeTraits<
#define INTERNAL_UMA_HISTOGRAM_ENUMERATION_DEDUCE_BOUNDARY(name, sample, \ #define INTERNAL_UMA_HISTOGRAM_ENUMERATION_DEDUCE_BOUNDARY(name, sample, \
flags) \ flags) \
INTERNAL_HISTOGRAM_ENUMERATION_WITH_FLAG( \ INTERNAL_HISTOGRAM_ENUMERATION_WITH_FLAG( \
name, sample, base::internal::EnumSizeTraits<decltype(sample)>::Count(), \ name, sample, \
base::internal::EnumSizeTraits<std::decay_t<decltype(sample)>>::Count(), \
flags) flags)
// Note: The value in |sample| must be strictly less than |enum_size|. // Note: The value in |sample| must be strictly less than |enum_size|.
...@@ -231,8 +232,8 @@ struct EnumSizeTraits< ...@@ -231,8 +232,8 @@ struct EnumSizeTraits<
using decayed_sample = std::decay<decltype(sample)>::type; \ using decayed_sample = std::decay<decltype(sample)>::type; \
static_assert(std::is_enum<decayed_sample>::value, \ static_assert(std::is_enum<decayed_sample>::value, \
"Unexpected: |sample| is not at enum."); \ "Unexpected: |sample| is not at enum."); \
constexpr auto boundary = \ constexpr auto boundary = base::internal::EnumSizeTraits< \
base::internal::EnumSizeTraits<decltype(sample)>::Count(); \ std::decay_t<decltype(sample)>>::Count(); \
static_assert( \ static_assert( \
static_cast<uintmax_t>(boundary) < \ static_cast<uintmax_t>(boundary) < \
static_cast<uintmax_t>( \ static_cast<uintmax_t>( \
......
...@@ -20,7 +20,7 @@ TEST(ScopedHistogramTimer, TwoTimersOneScope) { ...@@ -20,7 +20,7 @@ TEST(ScopedHistogramTimer, TwoTimersOneScope) {
// - integral types // - integral types
// - unscoped enums // - unscoped enums
// - scoped enums // - scoped enums
TEST(HistogramMacro, IntegralPsuedoEnumeration) { TEST(HistogramMacro, IntegralPseudoEnumeration) {
UMA_HISTOGRAM_ENUMERATION("Test.FauxEnumeration", 1, 1000); UMA_HISTOGRAM_ENUMERATION("Test.FauxEnumeration", 1, 1000);
} }
...@@ -54,4 +54,20 @@ TEST(HistogramMacro, ScopedEnumeration) { ...@@ -54,4 +54,20 @@ TEST(HistogramMacro, ScopedEnumeration) {
TestEnum2::MAX_ENTRIES); TestEnum2::MAX_ENTRIES);
} }
// Compile tests for UMA_HISTOGRAM_ENUMERATION when the value type is:
// - a const reference to an enum
// - a non-const reference to an enum
TEST(HistogramMacro, EnumerationConstRef) {
enum class TestEnum { kValue, kMaxValue = kValue };
const TestEnum& value_ref = TestEnum::kValue;
UMA_HISTOGRAM_ENUMERATION("Test.ScopedEnumeration3", value_ref);
}
TEST(HistogramMacro, EnumerationNonConstRef) {
enum class TestEnum { kValue, kMaxValue = kValue };
TestEnum value = TestEnum::kValue;
TestEnum& value_ref = value;
UMA_HISTOGRAM_ENUMERATION("Test.ScopedEnumeration4", value_ref);
}
} // namespace base } // namespace base
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment