Bring the feedback button to the Mac sad tab

base/metrics/histogram_macros.h

Index: base/metrics/histogram_macros.h
diff --git a/base/metrics/histogram_macros.h b/base/metrics/histogram_macros.h
index dcde7255c8dafee8e28c09e21baf492408ea8682..b60c3abdbb03cd0687d6bf6deb3dcebb002cbcd1 100644
--- a/base/metrics/histogram_macros.h
+++ b/base/metrics/histogram_macros.h
@@ -148,10 +148,12 @@
 // 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)