From 968ca7b38235bdcae6ec82173e1fe01bcf248eb1 Mon Sep 17 00:00:00 2001 From: "willchan@chromium.org" Date: Wed, 15 Dec 2010 09:10:43 +0000 Subject: Reland 69237 - Fix raw_scoped_refptr_mismatch_checker.h. BUG=28083 TEST=none Review URL: http://codereview.chromium.org/3549010 TBR=willchan@chromium.org Review URL: http://codereview.chromium.org/5885001 TBR=phajdan.jr@chromium.org Review URL: http://codereview.chromium.org/5887001 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@69243 0039d316-1c4b-4281-b951-d872f2087c98 --- base/callback.h | 5 +- base/raw_scoped_refptr_mismatch_checker.h | 225 ++++++++++++------------------ base/task.h | 15 +- base/template_util.h | 47 +++++++ 4 files changed, 151 insertions(+), 141 deletions(-) (limited to 'base') diff --git a/base/callback.h b/base/callback.h index 7f2eb70..e5ea771 100644 --- a/base/callback.h +++ b/base/callback.h @@ -206,8 +206,9 @@ template class UnboundMethod { public: UnboundMethod(Method m, const Params& p) : m_(m), p_(p) { - COMPILE_ASSERT((MethodUsesScopedRefptrCorrectly::value), - badunboundmethodparams); + COMPILE_ASSERT( + (base::internal::ParamsUseScopedRefptrCorrectly::value), + badunboundmethodparams); } void Run(T* obj) const { DispatchToMethod(obj, m_, p_); diff --git a/base/raw_scoped_refptr_mismatch_checker.h b/base/raw_scoped_refptr_mismatch_checker.h index d2913e7..b79cfb5 100644 --- a/base/raw_scoped_refptr_mismatch_checker.h +++ b/base/raw_scoped_refptr_mismatch_checker.h @@ -7,165 +7,124 @@ #pragma once #include "base/ref_counted.h" +#include "base/template_util.h" #include "base/tuple.h" +#include "build/build_config.h" -// It is dangerous to post a task with a raw pointer argument to a function -// that expects a scoped_refptr<>. The compiler will happily accept the -// situation, but it will not attempt to increase the refcount until the task -// runs. Callers expecting the argument to be refcounted up at post time are -// in for a nasty surprise! Example: http://crbug.com/27191 +// It is dangerous to post a task with a T* argument where T is a subtype of +// RefCounted(Base|ThreadSafeBase), since by the time the parameter is used, the +// object may already have been deleted since it was not held with a +// scoped_refptr. Example: http://crbug.com/27191 // The following set of traits are designed to generate a compile error // whenever this antipattern is attempted. -template -struct ExpectsScopedRefptrButGetsRawPtr { + +namespace base { + +// This is a base internal implementation file used by task.h and callback.h. +// Not for public consumption, so we wrap it in namespace internal. +namespace internal { + +template +struct NeedsScopedRefptrButGetsRawPtr { +#if defined(OS_WIN) + enum { + value = base::false_type::value + }; +#else + enum { + // Human readable translation: you needed to be a scoped_refptr if you are a + // raw pointer type and are convertible to a RefCounted(Base|ThreadSafeBase) + // type. + value = (is_pointer::value && + (is_convertible::value || + is_convertible::value)) + }; +#endif +}; + +template +struct ParamsUseScopedRefptrCorrectly { enum { value = 0 }; }; -template -struct ExpectsScopedRefptrButGetsRawPtr, B*> { +template <> +struct ParamsUseScopedRefptrCorrectly { enum { value = 1 }; }; -template -struct FunctionUsesScopedRefptrCorrectly { - enum { value = 1 }; +template +struct ParamsUseScopedRefptrCorrectly > { + enum { value = !NeedsScopedRefptrButGetsRawPtr::value }; }; -template -struct FunctionUsesScopedRefptrCorrectly > { - enum { value = !ExpectsScopedRefptrButGetsRawPtr::value }; +template +struct ParamsUseScopedRefptrCorrectly > { + enum { value = !(NeedsScopedRefptrButGetsRawPtr::value || + NeedsScopedRefptrButGetsRawPtr::value) }; }; -template -struct FunctionUsesScopedRefptrCorrectly > { - enum { value = !(ExpectsScopedRefptrButGetsRawPtr::value || - ExpectsScopedRefptrButGetsRawPtr::value) }; +template +struct ParamsUseScopedRefptrCorrectly > { + enum { value = !(NeedsScopedRefptrButGetsRawPtr::value || + NeedsScopedRefptrButGetsRawPtr::value || + NeedsScopedRefptrButGetsRawPtr::value) }; }; -template -struct FunctionUsesScopedRefptrCorrectly > { - enum { value = !(ExpectsScopedRefptrButGetsRawPtr::value || - ExpectsScopedRefptrButGetsRawPtr::value || - ExpectsScopedRefptrButGetsRawPtr::value) }; +template +struct ParamsUseScopedRefptrCorrectly > { + enum { value = !(NeedsScopedRefptrButGetsRawPtr::value || + NeedsScopedRefptrButGetsRawPtr::value || + NeedsScopedRefptrButGetsRawPtr::value || + NeedsScopedRefptrButGetsRawPtr::value) }; }; -template -struct FunctionUsesScopedRefptrCorrectly > { - enum { value = !(ExpectsScopedRefptrButGetsRawPtr::value || - ExpectsScopedRefptrButGetsRawPtr::value || - ExpectsScopedRefptrButGetsRawPtr::value || - ExpectsScopedRefptrButGetsRawPtr::value) }; +template +struct ParamsUseScopedRefptrCorrectly > { + enum { value = !(NeedsScopedRefptrButGetsRawPtr::value || + NeedsScopedRefptrButGetsRawPtr::value || + NeedsScopedRefptrButGetsRawPtr::value || + NeedsScopedRefptrButGetsRawPtr::value || + NeedsScopedRefptrButGetsRawPtr::value) }; }; -template -struct FunctionUsesScopedRefptrCorrectly > { - enum { value = !(ExpectsScopedRefptrButGetsRawPtr::value || - ExpectsScopedRefptrButGetsRawPtr::value || - ExpectsScopedRefptrButGetsRawPtr::value || - ExpectsScopedRefptrButGetsRawPtr::value || - ExpectsScopedRefptrButGetsRawPtr::value) }; +template +struct ParamsUseScopedRefptrCorrectly > { + enum { value = !(NeedsScopedRefptrButGetsRawPtr::value || + NeedsScopedRefptrButGetsRawPtr::value || + NeedsScopedRefptrButGetsRawPtr::value || + NeedsScopedRefptrButGetsRawPtr::value || + NeedsScopedRefptrButGetsRawPtr::value || + NeedsScopedRefptrButGetsRawPtr::value) }; }; -template -struct FunctionUsesScopedRefptrCorrectly > { - enum { value = !(ExpectsScopedRefptrButGetsRawPtr::value || - ExpectsScopedRefptrButGetsRawPtr::value || - ExpectsScopedRefptrButGetsRawPtr::value || - ExpectsScopedRefptrButGetsRawPtr::value || - ExpectsScopedRefptrButGetsRawPtr::value || - ExpectsScopedRefptrButGetsRawPtr::value) }; +template +struct ParamsUseScopedRefptrCorrectly > { + enum { value = !(NeedsScopedRefptrButGetsRawPtr::value || + NeedsScopedRefptrButGetsRawPtr::value || + NeedsScopedRefptrButGetsRawPtr::value || + NeedsScopedRefptrButGetsRawPtr::value || + NeedsScopedRefptrButGetsRawPtr::value || + NeedsScopedRefptrButGetsRawPtr::value || + NeedsScopedRefptrButGetsRawPtr::value) }; }; -template -struct FunctionUsesScopedRefptrCorrectly > { - enum { value = !(ExpectsScopedRefptrButGetsRawPtr::value || - ExpectsScopedRefptrButGetsRawPtr::value || - ExpectsScopedRefptrButGetsRawPtr::value || - ExpectsScopedRefptrButGetsRawPtr::value || - ExpectsScopedRefptrButGetsRawPtr::value || - ExpectsScopedRefptrButGetsRawPtr::value || - ExpectsScopedRefptrButGetsRawPtr::value) }; +template +struct ParamsUseScopedRefptrCorrectly > { + enum { value = !(NeedsScopedRefptrButGetsRawPtr::value || + NeedsScopedRefptrButGetsRawPtr::value || + NeedsScopedRefptrButGetsRawPtr::value || + NeedsScopedRefptrButGetsRawPtr::value || + NeedsScopedRefptrButGetsRawPtr::value || + NeedsScopedRefptrButGetsRawPtr::value || + NeedsScopedRefptrButGetsRawPtr::value || + NeedsScopedRefptrButGetsRawPtr::value) }; }; -template -struct MethodUsesScopedRefptrCorrectly { - enum { value = 1 }; -}; +} // namespace internal -template -struct MethodUsesScopedRefptrCorrectly > { - enum { value = !ExpectsScopedRefptrButGetsRawPtr::value }; -}; - -template -struct MethodUsesScopedRefptrCorrectly > { - enum { value = !(ExpectsScopedRefptrButGetsRawPtr::value || - ExpectsScopedRefptrButGetsRawPtr::value) }; -}; - -template -struct MethodUsesScopedRefptrCorrectly > { - enum { value = !(ExpectsScopedRefptrButGetsRawPtr::value || - ExpectsScopedRefptrButGetsRawPtr::value || - ExpectsScopedRefptrButGetsRawPtr::value) }; -}; - -template -struct MethodUsesScopedRefptrCorrectly > { - enum { value = !(ExpectsScopedRefptrButGetsRawPtr::value || - ExpectsScopedRefptrButGetsRawPtr::value || - ExpectsScopedRefptrButGetsRawPtr::value || - ExpectsScopedRefptrButGetsRawPtr::value) }; -}; - -template -struct MethodUsesScopedRefptrCorrectly > { - enum { value = !(ExpectsScopedRefptrButGetsRawPtr::value || - ExpectsScopedRefptrButGetsRawPtr::value || - ExpectsScopedRefptrButGetsRawPtr::value || - ExpectsScopedRefptrButGetsRawPtr::value || - ExpectsScopedRefptrButGetsRawPtr::value) }; -}; - -template -struct MethodUsesScopedRefptrCorrectly > { - enum { value = !(ExpectsScopedRefptrButGetsRawPtr::value || - ExpectsScopedRefptrButGetsRawPtr::value || - ExpectsScopedRefptrButGetsRawPtr::value || - ExpectsScopedRefptrButGetsRawPtr::value || - ExpectsScopedRefptrButGetsRawPtr::value || - ExpectsScopedRefptrButGetsRawPtr::value) }; -}; - -template -struct MethodUsesScopedRefptrCorrectly > { - enum { value = !(ExpectsScopedRefptrButGetsRawPtr::value || - ExpectsScopedRefptrButGetsRawPtr::value || - ExpectsScopedRefptrButGetsRawPtr::value || - ExpectsScopedRefptrButGetsRawPtr::value || - ExpectsScopedRefptrButGetsRawPtr::value || - ExpectsScopedRefptrButGetsRawPtr::value || - ExpectsScopedRefptrButGetsRawPtr::value) }; -}; +} // namespace base #endif // BASE_RAW_SCOPED_REFPTR_MISMATCH_CHECKER_H_ diff --git a/base/task.h b/base/task.h index b21ccd8..1949e74 100644 --- a/base/task.h +++ b/base/task.h @@ -149,8 +149,9 @@ class ScopedRunnableMethodFactory { : obj_(obj), meth_(meth), params_(params) { - COMPILE_ASSERT((MethodUsesScopedRefptrCorrectly::value), - badscopedrunnablemethodparams); + COMPILE_ASSERT( + (base::internal::ParamsUseScopedRefptrCorrectly::value), + badscopedrunnablemethodparams); } virtual void Run() { @@ -317,8 +318,9 @@ class RunnableMethod : public CancelableTask { RunnableMethod(T* obj, Method meth, const Params& params) : obj_(obj), meth_(meth), params_(params) { traits_.RetainCallee(obj_); - COMPILE_ASSERT((MethodUsesScopedRefptrCorrectly::value), - badrunnablemethodparams); + COMPILE_ASSERT( + (base::internal::ParamsUseScopedRefptrCorrectly::value), + badrunnablemethodparams); } ~RunnableMethod() { @@ -429,8 +431,9 @@ class RunnableFunction : public CancelableTask { public: RunnableFunction(Function function, const Params& params) : function_(function), params_(params) { - COMPILE_ASSERT((FunctionUsesScopedRefptrCorrectly::value), - badrunnablefunctionparams); + COMPILE_ASSERT( + (base::internal::ParamsUseScopedRefptrCorrectly::value), + badrunnablefunctionparams); } ~RunnableFunction() { diff --git a/base/template_util.h b/base/template_util.h index 2cfe04c..27bdb73 100644 --- a/base/template_util.h +++ b/base/template_util.h @@ -6,6 +6,8 @@ #define BASE_TEMPLATE_UTIL_H_ #pragma once +#include "build/build_config.h" + namespace base { // template definitions from tr1 @@ -25,6 +27,51 @@ typedef integral_constant false_type; template struct is_pointer : false_type {}; template struct is_pointer : true_type {}; +namespace internal { + +// Types small_ and big_ are guaranteed such that sizeof(small_) < +// sizeof(big_) +typedef char small_; + +struct big_ { + small_ dummy[2]; +}; + +#if !defined(OS_WIN) + +// This class is an implementation detail for is_convertible, and you +// don't need to know how it works to use is_convertible. For those +// who care: we declare two different functions, one whose argument is +// of type To and one with a variadic argument list. We give them +// return types of different size, so we can use sizeof to trick the +// compiler into telling us which function it would have chosen if we +// had called it with an argument of type From. See Alexandrescu's +// _Modern C++ Design_ for more details on this sort of trick. + +template +struct ConvertHelper { + static small_ Test(To); + static big_ Test(...); + static From Create(); +}; + +#endif // !defined(OS_WIN) + +} // namespace internal + +#if !defined(OS_WIN) + +// Inherits from true_type if From is convertible to To, false_type otherwise. +template +struct is_convertible + : integral_constant::Test( + internal::ConvertHelper::Create())) + == sizeof(internal::small_)> { +}; + +#endif // !defined(OS_WIN) + } // namespace base #endif // BASE_TEMPLATE_UTIL_H_ -- cgit v1.1