Index: base/debug/crash_logging.h |
diff --git a/base/debug/crash_logging.h b/base/debug/crash_logging.h |
index 97bac754a4b626985fe26935d37de8008fb7f7d7..1a5315ce9dfd220bfe67fa9e01ce64e63d81ffcc 100644 |
--- a/base/debug/crash_logging.h |
+++ b/base/debug/crash_logging.h |
@@ -8,6 +8,7 @@ |
#include <stddef.h> |
#include <string> |
+#include <type_traits> |
#include <vector> |
#include "base/base_export.h" |
@@ -42,6 +43,10 @@ BASE_EXPORT void SetCrashKeyFromAddresses(const base::StringPiece& key, |
const void* const* addresses, |
size_t count); |
+enum ScopedCrashKeyNeedsNameTag { |
danakj
2016/07/19 21:02:57
nit: I'd nest this inside ScopedCrashKey, beside (
dcheng
2016/07/22 05:39:22
Done.
|
+ SCOPED_CRASH_KEY_NEEDS_NAME, |
+}; |
+ |
// A scoper that sets the specified key to value for the lifetime of the |
// object, and clears it on destruction. |
class BASE_EXPORT ScopedCrashKey { |
@@ -49,12 +54,46 @@ class BASE_EXPORT ScopedCrashKey { |
ScopedCrashKey(const base::StringPiece& key, const base::StringPiece& value); |
~ScopedCrashKey(); |
+ // Helper to force a static_assert when instantiating a ScopedCrashKey |
+ // temporary without a name. The usual idiom is to just #define a macro that |
+ // static_asserts with the message; however, that doesn't work well when the |
+ // type is in a namespace. |
+ // |
+ // Instead, we use a templated helper to trigger the static_assert, observing |
+ // two rules: |
+ // - The static_assert needs to be in a normally uninstantiated template; |
+ // otherwise, it will fail to compile =) |
+ // - Similarly, the static_assert must be dependent on the template argument, |
+ // to prevent it from being evaluated until the template is instantiated. |
+ // |
+ // To prevent this constructor from being accidentally invoked, it takes a |
+ // special enum as an argument. |
+ |
+ // TODO(dcheng): For reasons unknown to me, we can't make this a template |
+ // function taking just one parameter (the enum tag). While the build |
+ // correctly fails, the static_assert is never triggered. I think this is |
+ // because SFINAE removes the overload from consideration before it's ever |
+ // instantiated, but I'm not sure... |
+ template <typename... Args> |
+ explicit ScopedCrashKey(ScopedCrashKeyNeedsNameTag, const Args&...) { |
tzik
2016/07/14 07:39:53
Can we just disable this by "= delete"?
dcheng
2016/07/14 07:41:15
We can, but the idea is to provide a useful error
tzik
2016/07/14 07:45:41
Ok, so. How about define this:
template <typename.
|
+ constexpr bool always_false = sizeof...(Args) == 0 && sizeof...(Args) != 0; |
+ static_assert( |
+ always_false, |
+ "scoped crash key objects should not be unnamed temporaries."); |
+ } |
+ |
private: |
std::string key_; |
DISALLOW_COPY_AND_ASSIGN(ScopedCrashKey); |
}; |
+// Disallow an instantation of ScopedCrashKey without a name, since this results |
+// in a temporary that is immediately destroyed. Doing so will trigger the |
+// static_assert in the templated constructor helper in ScopedCrashKey. |
+#define ScopedCrashKey(...) \ |
+ ScopedCrashKey(base::debug::SCOPED_CRASH_KEY_NEEDS_NAME, __VA_ARGS__) |
+ |
// Before setting values for a key, all the keys must be registered. |
struct BASE_EXPORT CrashKey { |
// The name of the crash key, used in the above functions. |