Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix code smells reported by chrome's clang plugin #6833

Merged
merged 11 commits into from
Nov 13, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion assets/asset_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ class AssetManager final : public AssetResolver {
public:
AssetManager();

~AssetManager();
~AssetManager() override;

void PushFront(std::unique_ptr<AssetResolver> resolver);

Expand Down
4 changes: 4 additions & 0 deletions ci/licenses_golden/licenses_flutter
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ FILE: ../../../flutter/flow/compositor_context.cc
FILE: ../../../flutter/flow/compositor_context.h
FILE: ../../../flutter/flow/debug_print.cc
FILE: ../../../flutter/flow/debug_print.h
FILE: ../../../flutter/flow/embedded_views.cc
FILE: ../../../flutter/flow/embedded_views.h
FILE: ../../../flutter/flow/export_node.cc
FILE: ../../../flutter/flow/export_node.h
Expand Down Expand Up @@ -189,6 +190,7 @@ FILE: ../../../flutter/fml/task_runner.cc
FILE: ../../../flutter/fml/task_runner.h
FILE: ../../../flutter/fml/thread.cc
FILE: ../../../flutter/fml/thread.h
FILE: ../../../flutter/fml/thread_local.cc
FILE: ../../../flutter/fml/thread_local.h
FILE: ../../../flutter/fml/thread_local_unittests.cc
FILE: ../../../flutter/fml/thread_unittests.cc
Expand Down Expand Up @@ -311,6 +313,7 @@ FILE: ../../../flutter/lib/ui/window/pointer_data.cc
FILE: ../../../flutter/lib/ui/window/pointer_data.h
FILE: ../../../flutter/lib/ui/window/pointer_data_packet.cc
FILE: ../../../flutter/lib/ui/window/pointer_data_packet.h
FILE: ../../../flutter/lib/ui/window/viewport_metrics.cc
FILE: ../../../flutter/lib/ui/window/viewport_metrics.h
FILE: ../../../flutter/lib/ui/window/window.cc
FILE: ../../../flutter/lib/ui/window/window.h
Expand Down Expand Up @@ -496,6 +499,7 @@ FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/FlutterPlatfor
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/FlutterPlatformPlugin.mm
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.mm
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/FlutterPluginAppLifeCycleDelegate.mm
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/FlutterStandardCodec.mm
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/FlutterStandardCodec_Internal.h
Expand Down
6 changes: 6 additions & 0 deletions common/settings.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,12 @@

namespace blink {

Settings::Settings() = default;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We usually add an empty line between method methods. Here and elsewhere in this commit.

Copy link
Member Author

@goderbauer goderbauer Nov 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. I was expecting that clang-format would add those spaces for me. I guess it's not that smart...


Settings::Settings(const Settings& other) = default;

Settings::~Settings() = default;

std::string Settings::ToString() const {
std::stringstream stream;
stream << "Settings: " << std::endl;
Expand Down
6 changes: 6 additions & 0 deletions common/settings.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,12 @@ using TaskObserverAdd =
using TaskObserverRemove = std::function<void(intptr_t /* key */)>;

struct Settings {
Settings();

Settings(const Settings& other);

~Settings();

// VM settings
std::string vm_snapshot_data_path;
std::string vm_snapshot_instr_path;
Expand Down
2 changes: 2 additions & 0 deletions common/task_runners.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ TaskRunners::TaskRunners(std::string label,
ui_(std::move(ui)),
io_(std::move(io)) {}

TaskRunners::TaskRunners(const TaskRunners& other) = default;

TaskRunners::~TaskRunners() = default;

const std::string& TaskRunners::GetLabel() const {
Expand Down
2 changes: 2 additions & 0 deletions common/task_runners.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ class TaskRunners {
fml::RefPtr<fml::TaskRunner> ui,
fml::RefPtr<fml::TaskRunner> io);

TaskRunners(const TaskRunners& other);

~TaskRunners();

const std::string& GetLabel() const;
Expand Down
1 change: 1 addition & 0 deletions flow/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ source_set("flow") {
"compositor_context.h",
"debug_print.cc",
"debug_print.h",
"embedded_views.cc",
"embedded_views.h",
"instrumentation.cc",
"instrumentation.h",
Expand Down
12 changes: 12 additions & 0 deletions flow/embedded_views.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// Copyright 2013 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "flutter/flow/embedded_views.h"

namespace flow {

bool ExternalViewEmbedder::SubmitFrame(GrContext* context) {
return false;
};
} // namespace flow
2 changes: 1 addition & 1 deletion flow/embedded_views.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class ExternalViewEmbedder {
virtual SkCanvas* CompositeEmbeddedView(int view_id,
const EmbeddedViewParams& params) = 0;

virtual bool SubmitFrame(GrContext* context) { return false; };
virtual bool SubmitFrame(GrContext* context);

virtual ~ExternalViewEmbedder() = default;

Expand Down
2 changes: 1 addition & 1 deletion flow/layers/layer_tree.cc
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ sk_sp<SkPicture> LayerTree::Flatten(const SkRect& bounds) {
TRACE_EVENT0("flutter", "LayerTree::Flatten");

SkPictureRecorder recorder;
auto canvas = recorder.beginRecording(bounds);
auto* canvas = recorder.beginRecording(bounds);

if (!canvas) {
return nullptr;
Expand Down
2 changes: 1 addition & 1 deletion flow/layers/picture_layer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ PictureLayer::~PictureLayer() = default;
void PictureLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) {
SkPicture* sk_picture = picture();

if (auto cache = context->raster_cache) {
if (auto* cache = context->raster_cache) {
SkMatrix ctm = matrix;
ctm.postTranslate(offset_.x(), offset_.y());
#ifndef SUPPORT_FRACTIONAL_TRANSLATION
Expand Down
10 changes: 10 additions & 0 deletions flow/raster_cache.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,16 @@

namespace flow {

RasterCacheResult::RasterCacheResult() {}

RasterCacheResult::RasterCacheResult(const RasterCacheResult& other) = default;

RasterCacheResult::~RasterCacheResult() = default;

RasterCacheResult::RasterCacheResult(sk_sp<SkImage> image,
const SkRect& logical_rect)
: image_(std::move(image)), logical_rect_(logical_rect) {}

void RasterCacheResult::draw(SkCanvas& canvas, const SkPaint* paint) const {
SkAutoCanvasRestore auto_restore(&canvas, true);
SkIRect bounds =
Expand Down
9 changes: 6 additions & 3 deletions flow/raster_cache.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,13 @@ namespace flow {

class RasterCacheResult {
public:
RasterCacheResult() {}
RasterCacheResult();

RasterCacheResult(sk_sp<SkImage> image, const SkRect& logical_rect)
: image_(std::move(image)), logical_rect_(logical_rect) {}
RasterCacheResult(const RasterCacheResult& other);

~RasterCacheResult();

RasterCacheResult(sk_sp<SkImage> image, const SkRect& logical_rect);

operator bool() const { return static_cast<bool>(image_); }

Expand Down
1 change: 1 addition & 0 deletions fml/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ source_set("fml") {
"task_runner.h",
"thread.cc",
"thread.h",
"thread_local.cc",
"thread_local.h",
"time/time_delta.h",
"time/time_point.cc",
Expand Down
15 changes: 15 additions & 0 deletions fml/file.cc
Original file line number Diff line number Diff line change
Expand Up @@ -43,4 +43,19 @@ fml::UniqueFD CreateDirectory(const fml::UniqueFD& base_directory,
return CreateDirectory(base_directory, components, permission, 0);
}

ScopedTemporaryDirectory::ScopedTemporaryDirectory() {
path_ = CreateTemporaryDirectory();
if (path_ != "") {
dir_fd_ = OpenDirectory(path_.c_str(), false, FilePermission::kRead);
}
}

ScopedTemporaryDirectory::~ScopedTemporaryDirectory() {
if (path_ != "") {
if (!UnlinkDirectory(path_.c_str())) {
FML_LOG(ERROR) << "Could not remove directory: " << path_;
}
}
}

} // namespace fml
17 changes: 3 additions & 14 deletions fml/file.h
Original file line number Diff line number Diff line change
Expand Up @@ -75,20 +75,9 @@ bool WriteAtomically(const fml::UniqueFD& base_directory,

class ScopedTemporaryDirectory {
public:
ScopedTemporaryDirectory() {
path_ = CreateTemporaryDirectory();
if (path_ != "") {
dir_fd_ = OpenDirectory(path_.c_str(), false, FilePermission::kRead);
}
}

~ScopedTemporaryDirectory() {
if (path_ != "") {
if (!UnlinkDirectory(path_.c_str())) {
FML_LOG(ERROR) << "Could not remove directory: " << path_;
}
}
}
ScopedTemporaryDirectory();

~ScopedTemporaryDirectory();

const UniqueFD& fd() { return dir_fd_; }

Expand Down
2 changes: 1 addition & 1 deletion fml/logging.cc
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ const char* StripDots(const char* path) {
}

const char* StripPath(const char* path) {
auto p = strrchr(path, '/');
auto* p = strrchr(path, '/');
if (p)
return p + 1;
else
Expand Down
8 changes: 8 additions & 0 deletions fml/mapping.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,12 @@ const uint8_t* DataMapping::GetMapping() const {
return data_.data();
}

size_t NonOwnedMapping::GetSize() const {
return size_;
}

const uint8_t* NonOwnedMapping::GetMapping() const {
return data_;
}

} // namespace fml
4 changes: 2 additions & 2 deletions fml/mapping.h
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,9 @@ class NonOwnedMapping : public Mapping {
NonOwnedMapping(const uint8_t* data, size_t size)
: data_(data), size_(size) {}

size_t GetSize() const override { return size_; }
size_t GetSize() const override;

const uint8_t* GetMapping() const override { return data_; }
const uint8_t* GetMapping() const override;

private:
const uint8_t* const data_;
Expand Down
6 changes: 5 additions & 1 deletion fml/message.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@

namespace fml {

size_t MessageSerializable::GetSerializableTag() const {
return 0;
};

Message::Message() = default;

Message::~Message() = default;
Expand Down Expand Up @@ -96,7 +100,7 @@ uint8_t* Message::PrepareDecode(size_t size) {
if ((size + size_read_) > buffer_length_) {
return nullptr;
}
auto buffer = buffer_ + size_read_;
auto* buffer = buffer_ + size_read_;
size_read_ += size;
return buffer;
}
Expand Down
6 changes: 3 additions & 3 deletions fml/message.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ class MessageSerializable {

virtual bool Deserialize(Message& message) = 0;

virtual size_t GetSerializableTag() const { return 0; };
virtual size_t GetSerializableTag() const;
};

// The traits passed to the encode/decode calls that accept traits should be
Expand Down Expand Up @@ -88,7 +88,7 @@ class Message {
template <typename T,
typename = std::enable_if_t<std::is_trivially_copyable<T>::value>>
FML_WARN_UNUSED_RESULT bool Encode(const T& value) {
if (auto buffer = PrepareEncode(sizeof(T))) {
if (auto* buffer = PrepareEncode(sizeof(T))) {
::memcpy(buffer, &value, sizeof(T));
return true;
}
Expand Down Expand Up @@ -131,7 +131,7 @@ class Message {
template <typename T,
typename = std::enable_if_t<std::is_trivially_copyable<T>::value>>
FML_WARN_UNUSED_RESULT bool Decode(T& value) {
if (auto buffer = PrepareDecode(sizeof(T))) {
if (auto* buffer = PrepareDecode(sizeof(T))) {
::memcpy(&value, buffer, sizeof(T));
return true;
}
Expand Down
2 changes: 1 addition & 1 deletion fml/message_loop.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ FML_THREAD_LOCAL ThreadLocal tls_message_loop([](intptr_t value) {
});

MessageLoop& MessageLoop::GetCurrent() {
auto loop = reinterpret_cast<MessageLoop*>(tls_message_loop.Get());
auto* loop = reinterpret_cast<MessageLoop*>(tls_message_loop.Get());
FML_CHECK(loop != nullptr)
<< "MessageLoop::EnsureInitializedForCurrentThread was not called on "
"this thread prior to message loop use.";
Expand Down
9 changes: 9 additions & 0 deletions fml/message_loop_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -145,4 +145,13 @@ void MessageLoopImpl::RunExpiredTasks() {
}
}

MessageLoopImpl::DelayedTask::DelayedTask(size_t p_order,
fml::closure p_task,
fml::TimePoint p_target_time)
: order(p_order), task(std::move(p_task)), target_time(p_target_time) {}

MessageLoopImpl::DelayedTask::DelayedTask(const DelayedTask& other) = default;

MessageLoopImpl::DelayedTask::~DelayedTask() = default;

} // namespace fml
7 changes: 5 additions & 2 deletions fml/message_loop_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,11 @@ class MessageLoopImpl : public fml::RefCountedThreadSafe<MessageLoopImpl> {

DelayedTask(size_t p_order,
fml::closure p_task,
fml::TimePoint p_target_time)
: order(p_order), task(std::move(p_task)), target_time(p_target_time) {}
fml::TimePoint p_target_time);

DelayedTask(const DelayedTask& other);

~DelayedTask();
};

struct DelayedTaskCompare {
Expand Down
2 changes: 1 addition & 1 deletion fml/platform/posix/file_posix.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ namespace fml {

std::string CreateTemporaryDirectory() {
char directory_name[] = "/tmp/flutter_XXXXXXXX";
auto result = ::mkdtemp(directory_name);
auto* result = ::mkdtemp(directory_name);
if (result == nullptr) {
return "";
}
Expand Down
2 changes: 1 addition & 1 deletion fml/platform/posix/mapping_posix.cc
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ FileMapping::FileMapping(const fml::UniqueFD& handle,

const auto is_writable = IsWritable(protection);

auto mapping =
auto* mapping =
::mmap(nullptr, stat_buffer.st_size, ToPosixProtectionFlags(protection),
is_writable ? MAP_SHARED : MAP_PRIVATE, handle.get(), 0);

Expand Down
2 changes: 1 addition & 1 deletion fml/platform/posix/native_library_posix.cc
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ fml::RefPtr<NativeLibrary> NativeLibrary::CreateForCurrentProcess() {
}

const uint8_t* NativeLibrary::ResolveSymbol(const char* symbol) {
auto resolved_symbol = static_cast<const uint8_t*>(::dlsym(handle_, symbol));
auto* resolved_symbol = static_cast<const uint8_t*>(::dlsym(handle_, symbol));
if (resolved_symbol == nullptr) {
FML_DLOG(INFO) << "Could not resolve symbol in library: " << symbol;
}
Expand Down
8 changes: 4 additions & 4 deletions fml/string_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ size_t StringView::find(StringView s, size_t pos) const {
if (s.empty())
return pos;

auto result = std::search(begin() + pos, end(), s.begin(), s.end());
auto* result = std::search(begin() + pos, end(), s.begin(), s.end());
if (result == end())
return npos;
return result - begin();
Expand All @@ -94,7 +94,7 @@ size_t StringView::find(char c, size_t pos) const {
if (pos > size_)
return npos;

auto result = std::find(begin() + pos, end(), c);
auto* result = std::find(begin() + pos, end(), c);
if (result == end())
return npos;
return result - begin();
Expand All @@ -106,8 +106,8 @@ size_t StringView::rfind(StringView s, size_t pos) const {
if (s.empty())
return std::min(pos, size_);

auto last = begin() + std::min(size_ - s.size(), pos) + s.size();
auto result = std::find_end(begin(), last, s.begin(), s.end());
auto* last = begin() + std::min(size_ - s.size(), pos) + s.size();
auto* result = std::find_end(begin(), last, s.begin(), s.end());
if (result == last)
return npos;
return result - begin();
Expand Down
Loading