Bring the feedback button to the Mac sad tab

base/metrics/histogram_macros.h

 // Copyright 2014 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.
 
 #ifndef BASE_METRICS_HISTOGRAM_MACROS_H_
 #define BASE_METRICS_HISTOGRAM_MACROS_H_
 
 #include "base/atomicops.h"
 #include "base/logging.h"
 #include "base/metrics/histogram.h"
@@ skipping 130 matching lines @@
 
 #define LOCAL_HISTOGRAM_CUSTOM_COUNTS(name, sample, min, max, bucket_count) \
     STATIC_HISTOGRAM_POINTER_BLOCK(name, Add(sample), \
         base::Histogram::FactoryGet(name, min, max, bucket_count, \
                                     base::HistogramBase::kNoFlags))
 
 // This is a helper macro used by other macros and shouldn't be used directly.
 // One additional bucket is created in the LinearHistogram for the illegal
 // values >= boundary_value so that mistakes in calling the UMA enumeration
 // macros can be detected.
-#define HISTOGRAM_ENUMERATION_WITH_FLAG(name, sample, boundary, flag) \
-    STATIC_HISTOGRAM_POINTER_BLOCK(name, Add(sample), \
-        base::LinearHistogram::FactoryGet(name, 1, boundary, boundary + 1, \
-            flag))
+#define HISTOGRAM_ENUMERATION_WITH_FLAG(name, sample, boundary, flag)  \
+  STATIC_HISTOGRAM_POINTER_BLOCK(                                      \
+      name, Add(static_cast<base::HistogramBase::Sample>(sample)),     \
+      base::LinearHistogram::FactoryGet(                               \
+          name, 1, static_cast<base::HistogramBase::Sample>(boundary), \
+          static_cast<uint32_t>(boundary) + 1, flag))

[Ilya Sherman, 2016/08/22 20:52:12]

I'm not comfortable with adding a blanket static_cast to the macro, as it might
hide legitimate compilation warnings/errors as well as just the enum class ones.
 Is there a way to more specifically target just enum classes?

[Sidney San Martín, 2016/08/25 00:55:11]

Thanks, in retrospect that was careless. I created a function,
base::underlying_number, that returns enums as their integral types and passes
other arithmetic values through. Let me know what you think.

[Ilya Sherman, 2016/08/25 01:12:56]

Very cool!  What does a compile error look like if someone attempts to use this
macro with something of non-integral type?  Say, a double, or a string?  (And,
what did the compile error look like before?  I'm not sure, but I'm hoping it's
sane prior to this change ... and hopefully after it as well =D)

[Sidney San Martín, 2016/08/30 18:46:37]

It passed doubles through, since existing code already passes doubles to these
macros — is that intentional?

In the patchset you saw, wrong types caused compile errors of this flavor:

    example.cc:39:39: error: no matching function for call to
'underlying_number'
    template_util.h:138:35: note: candidate template ignored: disabled by
'enable_if' [with T = nullptr_t]
    template_util.h:150:35: note: candidate template ignored: disabled by
'enable_if' [with T = char const[4]]

I was unhappy with that because I originally wanted the function to be more
transparent. So in the new patchset it's transparent to all non-enum types, and
you get errors like this:

    example.cc:50:5: error: cannot initialize a parameter of type 'Sample' (aka
'int') with an lvalue of type 'char const[4]'
    example.cc:53:5: error: cannot initialize a parameter of type 'Sample' (aka
'int') with an rvalue of type 'nullptr_t'

It's now called `base::underlying_value` to match `std::underlying_type` (but,
unlike `std::underlying_type`, it's a no-op instead of UB to use with a
non-enum).

My solution is "interesting", but maybe it would be smarter to handle this at
the Histogram level (e.g. an overload that takes enum types)?

 
 #define LOCAL_HISTOGRAM_PERCENTAGE(name, under_one_hundred) \
     LOCAL_HISTOGRAM_ENUMERATION(name, under_one_hundred, 101)
 
 #define LOCAL_HISTOGRAM_BOOLEAN(name, sample) \
     STATIC_HISTOGRAM_POINTER_BLOCK(name, AddBoolean(sample), \
         base::BooleanHistogram::FactoryGet(name, base::Histogram::kNoFlags))
 
 // Support histograming of an enumerated value.  The samples should always be
 // strictly less than |boundary_value| -- this prevents you from running into
@@ skipping 129 matching lines @@
         UMA_HISTOGRAM_LONG_TIMES_100(name, elapsed); \
       } else { \
         UMA_HISTOGRAM_TIMES(name, elapsed); \
       } \
     } \
    private: \
     base::TimeTicks constructed_; \
   } scoped_histogram_timer_##key
 
 #endif  // BASE_METRICS_HISTOGRAM_MACROS_H_