Skip to content

Commit

Permalink
Remove base's implicit_cast.
Browse files Browse the repository at this point in the history
There's no momentum on making this a thing in the C++ standard. We
should have less magic that is non-standard in base.

R=ricea@chromium.org, thakis@chromium.org, vmpstr
BUG=529769

Review URL: https://codereview.chromium.org/1340683002

Cr-Commit-Position: refs/heads/master@{#348978}
  • Loading branch information
danakj authored and Commit bot committed Sep 15, 2015
1 parent bdf9eb3 commit a5f0d96
Show file tree
Hide file tree
Showing 22 changed files with 64 additions and 89 deletions.
2 changes: 1 addition & 1 deletion android_webview/native/aw_contents.cc
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ AwBrowserPermissionRequestDelegate* AwBrowserPermissionRequestDelegate::FromID(
content::WebContents::FromRenderFrameHost(
content::RenderFrameHost::FromID(render_process_id,
render_frame_id)));
return implicit_cast<AwBrowserPermissionRequestDelegate*>(aw_contents);
return aw_contents;
}

AwContents::AwContents(scoped_ptr<WebContents> web_contents)
Expand Down
7 changes: 5 additions & 2 deletions base/android/scoped_java_ref.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "base/base_export.h"
#include "base/basictypes.h"
#include "base/logging.h"
#include "base/template_util.h"

namespace base {
namespace android {
Expand Down Expand Up @@ -178,7 +179,8 @@ class ScopedJavaLocalRef : public JavaRef<T> {

template<typename U>
void Reset(JNIEnv* env, U obj) {
implicit_cast<T>(obj); // Ensure U is assignable to T
static_assert(base::is_convertible<U, T>::value,
"U must be convertible to T");
env_ = this->SetNewLocalRef(env, obj);
}

Expand Down Expand Up @@ -242,7 +244,8 @@ class ScopedJavaGlobalRef : public JavaRef<T> {

template<typename U>
void Reset(JNIEnv* env, U obj) {
implicit_cast<T>(obj); // Ensure U is assignable to T
static_assert(base::is_convertible<U, T>::value,
"U must be convertible to T");
this->SetNewGlobalRef(env, obj);
}

Expand Down
2 changes: 1 addition & 1 deletion base/files/file_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -517,7 +517,7 @@ TEST(FileTest, MemoryCorruption) {
// Test that changing the checksum value is detected.
base::File file;
EXPECT_NE(file.file_.file_memory_checksum_,
implicit_cast<unsigned int>(file.GetPlatformFile()));
static_cast<unsigned int>(file.GetPlatformFile()));
file.file_.file_memory_checksum_ = file.GetPlatformFile();
EXPECT_DEATH(file.IsValid(), "");

Expand Down
23 changes: 0 additions & 23 deletions base/macros.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,29 +55,6 @@
template <typename T, size_t N> char (&ArraySizeHelper(T (&array)[N]))[N];
#define arraysize(array) (sizeof(ArraySizeHelper(array)))


// Use implicit_cast as a safe version of static_cast or const_cast
// for upcasting in the type hierarchy (i.e. casting a pointer to Foo
// to a pointer to SuperclassOfFoo or casting a pointer to Foo to
// a const pointer to Foo).
// When you use implicit_cast, the compiler checks that the cast is safe.
// Such explicit implicit_casts are necessary in surprisingly many
// situations where C++ demands an exact type match instead of an
// argument type convertible to a target type.
//
// The From type can be inferred, so the preferred syntax for using
// implicit_cast is the same as for static_cast etc.:
//
// implicit_cast<ToType>(expr)
//
// implicit_cast would have been part of the C++ standard library,
// but the proposal was submitted too late. It will probably make
// its way into the language in the future.
template<typename To, typename From>
inline To implicit_cast(From const &f) {
return f;
}

// The COMPILE_ASSERT macro can be used to verify that a compile time
// expression is true. For example, you could use it to verify the
// size of a static array:
Expand Down
4 changes: 1 addition & 3 deletions base/memory/scoped_ptr.h
Original file line number Diff line number Diff line change
Expand Up @@ -465,9 +465,7 @@ class scoped_ptr<T[], D> {
// (C++98 [expr.delete]p3). If you're doing this, fix your code.
// - it cannot be const-qualified differently from T per unique_ptr spec
// (http://cplusplus.github.com/LWG/lwg-active.html#2118). Users wanting
// to work around this may use implicit_cast<const T*>().
// However, because of the first bullet in this comment, users MUST
// NOT use implicit_cast<Base*>() to upcast the static type of the array.
// to work around this may use const_cast<const T*>().
explicit scoped_ptr(element_type* array) : impl_(array) {}

// Constructor. Allows construction from a nullptr.
Expand Down
6 changes: 3 additions & 3 deletions base/numerics/safe_numerics_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -673,9 +673,9 @@ TEST(SafeNumerics, IsValueInRangeForNumericType) {
EXPECT_TRUE(IsValueInRangeForNumericType<int32_t>(
std::numeric_limits<int32_t>::min()));
EXPECT_TRUE(IsValueInRangeForNumericType<int32_t>(
implicit_cast<int64_t>(std::numeric_limits<int32_t>::min())));
static_cast<int64_t>(std::numeric_limits<int32_t>::min())));
EXPECT_FALSE(IsValueInRangeForNumericType<int32_t>(
implicit_cast<int64_t>(std::numeric_limits<int32_t>::min()) - 1));
static_cast<int64_t>(std::numeric_limits<int32_t>::min()) - 1));
EXPECT_FALSE(IsValueInRangeForNumericType<int32_t>(
std::numeric_limits<int64_t>::min()));

Expand Down Expand Up @@ -715,7 +715,7 @@ TEST(SafeNumerics, IsValueInRangeForNumericType) {
EXPECT_TRUE(IsValueInRangeForNumericType<int64_t>(
std::numeric_limits<int32_t>::min()));
EXPECT_TRUE(IsValueInRangeForNumericType<int64_t>(
implicit_cast<int64_t>(std::numeric_limits<int32_t>::min())));
static_cast<int64_t>(std::numeric_limits<int32_t>::min())));
EXPECT_TRUE(IsValueInRangeForNumericType<int64_t>(
std::numeric_limits<int64_t>::min()));
}
10 changes: 5 additions & 5 deletions chrome/browser/apps/app_shim/app_shim_host_mac_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ class AppShimHostTest : public testing::Test,
}

void SimulateDisconnect() {
implicit_cast<IPC::Listener*>(host_.release())->OnChannelError();
static_cast<IPC::Listener*>(host_.release())->OnChannelError();
}

protected:
Expand Down Expand Up @@ -128,16 +128,16 @@ TEST_F(AppShimHostTest, TestLaunchAppWithHandler) {
apps::AppShimHandler::RegisterHandler(kTestAppId, this);
LaunchApp(apps::APP_SHIM_LAUNCH_NORMAL);
EXPECT_EQ(kTestAppId,
implicit_cast<apps::AppShimHandler::Host*>(host())->GetAppId());
static_cast<apps::AppShimHandler::Host*>(host())->GetAppId());
EXPECT_EQ(apps::APP_SHIM_LAUNCH_SUCCESS, GetLaunchResult());
EXPECT_EQ(1, launch_count_);
EXPECT_EQ(1, launch_now_count_);
EXPECT_EQ(0, focus_count_);
EXPECT_EQ(0, close_count_);

// A second OnAppLaunchComplete is ignored.
implicit_cast<apps::AppShimHandler::Host*>(host())->
OnAppLaunchComplete(apps::APP_SHIM_LAUNCH_APP_NOT_FOUND);
static_cast<apps::AppShimHandler::Host*>(host())
->OnAppLaunchComplete(apps::APP_SHIM_LAUNCH_APP_NOT_FOUND);
EXPECT_EQ(apps::APP_SHIM_LAUNCH_SUCCESS, GetLaunchResult());

EXPECT_TRUE(host()->ReceiveMessage(
Expand All @@ -157,7 +157,7 @@ TEST_F(AppShimHostTest, TestNoLaunchNow) {
apps::AppShimHandler::RegisterHandler(kTestAppId, this);
LaunchApp(apps::APP_SHIM_LAUNCH_REGISTER_ONLY);
EXPECT_EQ(kTestAppId,
implicit_cast<apps::AppShimHandler::Host*>(host())->GetAppId());
static_cast<apps::AppShimHandler::Host*>(host())->GetAppId());
EXPECT_EQ(apps::APP_SHIM_LAUNCH_SUCCESS, GetLaunchResult());
EXPECT_EQ(1, launch_count_);
EXPECT_EQ(0, launch_now_count_);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ - (id)initForNativeAppWindow:(ChromeNativeAppWindowViewsMac*)nativeAppWindow {
addObserver:self
selector:@selector(onWindowWillStartLiveResize:)
name:NSWindowWillStartLiveResizeNotification
object:implicit_cast<ui::BaseWindow*>(nativeAppWindow)
object:static_cast<ui::BaseWindow*>(nativeAppWindow)
->GetNativeWindow()];
}
return self;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,7 @@ bool ChromeNativeAppWindowViewsWin::IsRunningInAsh() {
if (!ash::Shell::HasInstance())
return false;

views::Widget* widget =
implicit_cast<views::WidgetDelegate*>(this)->GetWidget();
views::Widget* widget = GetWidget();
chrome::HostDesktopType host_desktop_type =
chrome::GetHostDesktopTypeForNativeWindow(widget->GetNativeWindow());
return host_desktop_type == chrome::HOST_DESKTOP_TYPE_ASH;
Expand Down
3 changes: 2 additions & 1 deletion components/history/core/browser/history_backend.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1367,7 +1367,8 @@ void HistoryBackend::QueryFilteredURLs(int result_count,

// Limit to the top |result_count| results.
std::sort(data.begin(), data.end(), PageUsageData::Predicate);
if (result_count && implicit_cast<int>(data.size()) > result_count)
DCHECK_GE(result_count, 0);
if (result_count && data.size() > static_cast<size_t>(result_count))
data.resize(result_count);

for (size_t i = 0; i < data.size(); ++i) {
Expand Down
3 changes: 2 additions & 1 deletion components/history/core/browser/visitsegment_database.cc
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,8 @@ void VisitSegmentDatabase::QuerySegmentUsage(

// Limit to the top kResultCount results.
std::sort(results->begin(), results->end(), PageUsageData::Predicate);
if (static_cast<int>(results->size()) > max_result_count) {
DCHECK_GE(max_result_count, 0);
if (results->size() > static_cast<size_t>(max_result_count)) {
STLDeleteContainerPointers(results->begin() + max_result_count,
results->end());
results->resize(max_result_count);
Expand Down
3 changes: 2 additions & 1 deletion content/browser/cache_storage/cache_storage.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "base/location.h"
#include "base/memory/ref_counted.h"
#include "base/metrics/histogram_macros.h"
#include "base/numerics/safe_conversions.h"
#include "base/sha1.h"
#include "base/single_thread_task_runner.h"
#include "base/stl_util.h"
Expand Down Expand Up @@ -276,7 +277,7 @@ class CacheStorage::SimpleCacheLoader : public CacheStorage::CacheLoader {
const BoolCallback& callback,
const scoped_refptr<base::SingleThreadTaskRunner>& original_task_runner) {
int bytes_written = base::WriteFile(tmp_path, data.c_str(), data.size());
if (bytes_written != implicit_cast<int>(data.size())) {
if (bytes_written != base::checked_cast<int>(data.size())) {
base::DeleteFile(tmp_path, /* recursive */ false);
original_task_runner->PostTask(FROM_HERE, base::Bind(callback, false));
}
Expand Down
14 changes: 6 additions & 8 deletions net/disk_cache/backend_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ bool DiskCacheBackendTest::CreateSetOfRandomEntries(
key_pool->insert(key);
entry->Close();
}
return key_pool->size() == implicit_cast<size_t>(cache_->GetEntryCount());
return key_pool->size() == static_cast<size_t>(cache_->GetEntryCount());
}

// Performs iteration over the backend and checks that the keys of entries
Expand All @@ -274,7 +274,7 @@ bool DiskCacheBackendTest::EnumerateAndMatchKeys(
EXPECT_EQ(1U, keys_to_match->erase(entry->GetKey()));
entry->Close();
++(*count);
if (max_to_open >= 0 && implicit_cast<int>(*count) >= max_to_open)
if (max_to_open >= 0 && static_cast<int>(*count) >= max_to_open)
break;
};

Expand Down Expand Up @@ -3327,10 +3327,9 @@ TEST_F(DiskCacheBackendTest, SimpleCacheOpenBadFile) {

disk_cache::SimpleFileHeader header;
header.initial_magic_number = UINT64_C(0xbadf00d);
EXPECT_EQ(
implicit_cast<int>(sizeof(header)),
base::WriteFile(entry_file1_path, reinterpret_cast<char*>(&header),
sizeof(header)));
EXPECT_EQ(static_cast<int>(sizeof(header)),
base::WriteFile(entry_file1_path, reinterpret_cast<char*>(&header),
sizeof(header)));
ASSERT_EQ(net::ERR_FAILED, OpenEntry(key, &entry));
}

Expand Down Expand Up @@ -3489,8 +3488,7 @@ TEST_F(DiskCacheBackendTest, SimpleCacheEnumerationCorruption) {

EXPECT_TRUE(disk_cache::simple_util::CreateCorruptFileForTests(
key, cache_path_));
EXPECT_EQ(key_pool.size() + 1,
implicit_cast<size_t>(cache_->GetEntryCount()));
EXPECT_EQ(key_pool.size() + 1, static_cast<size_t>(cache_->GetEntryCount()));

// Check that enumeration returns all entries but the corrupt one.
std::set<std::string> keys_to_match(key_pool);
Expand Down
2 changes: 1 addition & 1 deletion net/disk_cache/entry_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2718,7 +2718,7 @@ TEST_F(DiskCacheEntryTest, SimpleCacheNoEOF) {

// Truncate the file such that the length isn't sufficient to have an EOF
// record.
int kTruncationBytes = -implicit_cast<int>(sizeof(disk_cache::SimpleFileEOF));
int kTruncationBytes = -static_cast<int>(sizeof(disk_cache::SimpleFileEOF));
const base::FilePath entry_path = cache_path_.AppendASCII(
disk_cache::simple_util::GetFilenameFromKeyAndFileIndex(key, 0));
const int64 invalid_size =
Expand Down
3 changes: 2 additions & 1 deletion net/disk_cache/simple/simple_index_file.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "base/files/memory_mapped_file.h"
#include "base/hash.h"
#include "base/logging.h"
#include "base/numerics/safe_conversions.h"
#include "base/pickle.h"
#include "base/single_thread_task_runner.h"
#include "base/task_runner_util.h"
Expand Down Expand Up @@ -77,7 +78,7 @@ bool WritePickleFile(base::Pickle* pickle, const base::FilePath& file_name) {

int bytes_written =
file.Write(0, static_cast<const char*>(pickle->data()), pickle->size());
if (bytes_written != implicit_cast<int>(pickle->size())) {
if (bytes_written != base::checked_cast<int>(pickle->size())) {
simple_util::SimpleCacheDeleteFile(file_name);
return false;
}
Expand Down
10 changes: 4 additions & 6 deletions net/disk_cache/simple/simple_index_file_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -221,9 +221,8 @@ TEST_F(SimpleIndexFileTest, LoadCorruptIndex) {
ASSERT_TRUE(simple_index_file.CreateIndexFileDirectory());
const base::FilePath& index_path = simple_index_file.GetIndexFilePath();
const std::string kDummyData = "nothing to be seen here";
EXPECT_EQ(
implicit_cast<int>(kDummyData.size()),
base::WriteFile(index_path, kDummyData.data(), kDummyData.size()));
EXPECT_EQ(static_cast<int>(kDummyData.size()),
base::WriteFile(index_path, kDummyData.data(), kDummyData.size()));
base::Time fake_cache_mtime;
ASSERT_TRUE(simple_util::GetMTime(simple_index_file.GetIndexFilePath(),
&fake_cache_mtime));
Expand Down Expand Up @@ -263,9 +262,8 @@ TEST_F(SimpleIndexFileTest, SimpleCacheUpgrade) {
const std::string index_file_contents("incorrectly serialized data");
const base::FilePath old_index_file =
cache_path.AppendASCII("the-real-index");
ASSERT_EQ(implicit_cast<int>(index_file_contents.size()),
base::WriteFile(old_index_file,
index_file_contents.data(),
ASSERT_EQ(static_cast<int>(index_file_contents.size()),
base::WriteFile(old_index_file, index_file_contents.data(),
index_file_contents.size()));

// Upgrade the cache.
Expand Down
10 changes: 5 additions & 5 deletions net/disk_cache/simple/simple_synchronous_entry.cc
Original file line number Diff line number Diff line change
Expand Up @@ -939,7 +939,7 @@ int SimpleSynchronousEntry::InitializeForOpen(
scoped_ptr<char[]> key(new char[header.key_length]);
int key_read_result = files_[i].Read(sizeof(header), key.get(),
header.key_length);
if (key_read_result != implicit_cast<int>(header.key_length)) {
if (key_read_result != base::checked_cast<int>(header.key_length)) {
DLOG(WARNING) << "Cannot read key from entry.";
RecordSyncOpenResult(cache_type_, OPEN_ENTRY_CANT_READ_KEY, had_index);
return net::ERR_FAILED;
Expand Down Expand Up @@ -1018,7 +1018,7 @@ bool SimpleSynchronousEntry::InitializeCreatedFile(

bytes_written = files_[file_index].Write(sizeof(header), key_.data(),
key_.size());
if (bytes_written != implicit_cast<int>(key_.size())) {
if (bytes_written != base::checked_cast<int>(key_.size())) {
*out_result = CREATE_ENTRY_CANT_WRITE_KEY;
return false;
}
Expand Down Expand Up @@ -1248,7 +1248,7 @@ bool SimpleSynchronousEntry::InitializeSparseFile() {

int key_write_result = sparse_file_.Write(sizeof(header), key_.data(),
key_.size());
if (key_write_result != implicit_cast<int>(key_.size())) {
if (key_write_result != base::checked_cast<int>(key_.size())) {
DLOG(WARNING) << "Could not write sparse file key";
return false;
}
Expand Down Expand Up @@ -1378,7 +1378,7 @@ bool SimpleSynchronousEntry::WriteSparseRange(SparseRange* range,
int bytes_written = sparse_file_.Write(range->file_offset - sizeof(header),
reinterpret_cast<char*>(&header),
sizeof(header));
if (bytes_written != implicit_cast<int>(sizeof(header))) {
if (bytes_written != base::checked_cast<int>(sizeof(header))) {
DLOG(WARNING) << "Could not rewrite sparse range header.";
return false;
}
Expand Down Expand Up @@ -1413,7 +1413,7 @@ bool SimpleSynchronousEntry::AppendSparseRange(int64 offset,
int bytes_written = sparse_file_.Write(sparse_tail_offset_,
reinterpret_cast<char*>(&header),
sizeof(header));
if (bytes_written != implicit_cast<int>(sizeof(header))) {
if (bytes_written != base::checked_cast<int>(sizeof(header))) {
DLOG(WARNING) << "Could not append sparse range header.";
return false;
}
Expand Down
Loading

0 comments on commit a5f0d96

Please sign in to comment.