Skip to content

Commit

Permalink
Fix some clang-tidy lints for Linux host_debug (flutter#29734)
Browse files Browse the repository at this point in the history
  • Loading branch information
zanderso authored Nov 18, 2021
1 parent 1327b42 commit eac4cd2
Show file tree
Hide file tree
Showing 50 changed files with 204 additions and 164 deletions.
3 changes: 2 additions & 1 deletion .clang-tidy
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,5 @@ Checks: "google-*,\
# to prevent new warnings being introduced.
# https://github.com/flutter/flutter/issues/93279
WarningsAsErrors: "clang-analyzer-osx.*,\
google-objc-*"
google-objc-*,\
google-explicit-constructor"
100 changes: 50 additions & 50 deletions flow/display_list.cc
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ struct DLOp {
struct Set##name##Op final : DLOp { \
static const auto kType = DisplayListOpType::kSet##name; \
\
Set##name##Op(bool value) : value(value) {} \
explicit Set##name##Op(bool value) : value(value) {} \
\
const bool value; \
\
Expand All @@ -85,17 +85,17 @@ DEFINE_SET_BOOL_OP(InvertColors)
#undef DEFINE_SET_BOOL_OP

// 4 byte header + 4 byte payload packs into minimum 8 bytes
#define DEFINE_SET_ENUM_OP(name) \
struct SetStroke##name##Op final : DLOp { \
static const auto kType = DisplayListOpType::kSetStroke##name; \
\
SetStroke##name##Op(SkPaint::name value) : value(value) {} \
\
const SkPaint::name value; \
\
void dispatch(Dispatcher& dispatcher) const { \
dispatcher.setStroke##name(value); \
} \
#define DEFINE_SET_ENUM_OP(name) \
struct SetStroke##name##Op final : DLOp { \
static const auto kType = DisplayListOpType::kSetStroke##name; \
\
explicit SetStroke##name##Op(SkPaint::name value) : value(value) {} \
\
const SkPaint::name value; \
\
void dispatch(Dispatcher& dispatcher) const { \
dispatcher.setStroke##name(value); \
} \
};
DEFINE_SET_ENUM_OP(Cap)
DEFINE_SET_ENUM_OP(Join)
Expand Down Expand Up @@ -162,26 +162,26 @@ struct SetBlendModeOp final : DLOp {
// Set: 4 byte header + an sk_sp (ptr) uses 16 bytes due to the
// alignment of the ptr.
// (4 bytes unused)
#define DEFINE_SET_CLEAR_SKREF_OP(name, field) \
struct Clear##name##Op final : DLOp { \
static const auto kType = DisplayListOpType::kClear##name; \
\
Clear##name##Op() {} \
\
void dispatch(Dispatcher& dispatcher) const { \
dispatcher.set##name(nullptr); \
} \
}; \
struct Set##name##Op final : DLOp { \
static const auto kType = DisplayListOpType::kSet##name; \
\
Set##name##Op(sk_sp<Sk##name> field) : field(std::move(field)) {} \
\
sk_sp<Sk##name> field; \
\
void dispatch(Dispatcher& dispatcher) const { \
dispatcher.set##name(field); \
} \
#define DEFINE_SET_CLEAR_SKREF_OP(name, field) \
struct Clear##name##Op final : DLOp { \
static const auto kType = DisplayListOpType::kClear##name; \
\
Clear##name##Op() {} \
\
void dispatch(Dispatcher& dispatcher) const { \
dispatcher.set##name(nullptr); \
} \
}; \
struct Set##name##Op final : DLOp { \
static const auto kType = DisplayListOpType::kSet##name; \
\
explicit Set##name##Op(sk_sp<Sk##name> field) : field(std::move(field)) {} \
\
sk_sp<Sk##name> field; \
\
void dispatch(Dispatcher& dispatcher) const { \
dispatcher.set##name(field); \
} \
};
DEFINE_SET_CLEAR_SKREF_OP(Blender, blender)
DEFINE_SET_CLEAR_SKREF_OP(Shader, shader)
Expand All @@ -198,7 +198,7 @@ DEFINE_SET_CLEAR_SKREF_OP(PathEffect, effect)
struct SetMaskBlurFilter##name##Op final : DLOp { \
static const auto kType = DisplayListOpType::kSetMaskBlurFilter##name; \
\
SetMaskBlurFilter##name##Op(SkScalar sigma) : sigma(sigma) {} \
explicit SetMaskBlurFilter##name##Op(SkScalar sigma) : sigma(sigma) {} \
\
SkScalar sigma; \
\
Expand Down Expand Up @@ -432,17 +432,17 @@ struct DrawColorOp final : DLOp {
// (4 bytes unused)
// SkOval is same as SkRect
// SkRRect is 52 more bytes, which packs efficiently into 56 bytes total
#define DEFINE_DRAW_1ARG_OP(op_name, arg_type, arg_name) \
struct Draw##op_name##Op final : DLOp { \
static const auto kType = DisplayListOpType::kDraw##op_name; \
\
Draw##op_name##Op(arg_type arg_name) : arg_name(arg_name) {} \
\
const arg_type arg_name; \
\
void dispatch(Dispatcher& dispatcher) const { \
dispatcher.draw##op_name(arg_name); \
} \
#define DEFINE_DRAW_1ARG_OP(op_name, arg_type, arg_name) \
struct Draw##op_name##Op final : DLOp { \
static const auto kType = DisplayListOpType::kDraw##op_name; \
\
explicit Draw##op_name##Op(arg_type arg_name) : arg_name(arg_name) {} \
\
const arg_type arg_name; \
\
void dispatch(Dispatcher& dispatcher) const { \
dispatcher.draw##op_name(arg_name); \
} \
};
DEFINE_DRAW_1ARG_OP(Rect, SkRect, rect)
DEFINE_DRAW_1ARG_OP(Oval, SkRect, oval)
Expand Down Expand Up @@ -518,7 +518,7 @@ struct DrawArcOp final : DLOp {
struct Draw##name##Op final : DLOp { \
static const auto kType = DisplayListOpType::kDraw##name; \
\
Draw##name##Op(uint32_t count) : count(count) {} \
explicit Draw##name##Op(uint32_t count) : count(count) {} \
\
const uint32_t count; \
\
Expand Down Expand Up @@ -868,7 +868,7 @@ void DisplayList::Dispatch(Dispatcher& dispatcher,
uint8_t* ptr,
uint8_t* end) const {
while (ptr < end) {
auto op = (const DLOp*)ptr;
auto op = reinterpret_cast<const DLOp*>(ptr);
ptr += op->size;
FML_DCHECK(ptr <= end);
switch (op->type) {
Expand All @@ -890,7 +890,7 @@ void DisplayList::Dispatch(Dispatcher& dispatcher,

static void DisposeOps(uint8_t* ptr, uint8_t* end) {
while (ptr < end) {
auto op = (const DLOp*)ptr;
auto op = reinterpret_cast<const DLOp*>(ptr);
ptr += op->size;
FML_DCHECK(ptr <= end);
switch (op->type) {
Expand Down Expand Up @@ -922,8 +922,8 @@ static bool CompareOps(uint8_t* ptrA,
uint8_t* bulkStartA = ptrA;
uint8_t* bulkStartB = ptrB;
while (ptrA < endA && ptrB < endB) {
auto opA = (const DLOp*)ptrA;
auto opB = (const DLOp*)ptrB;
auto opA = reinterpret_cast<const DLOp*>(ptrA);
auto opB = reinterpret_cast<const DLOp*>(ptrB);
if (opA->type != opB->type || opA->size != opB->size) {
return false;
}
Expand Down Expand Up @@ -1047,7 +1047,7 @@ void* DisplayListBuilder::Push(size_t pod, int op_inc, Args&&... args) {
memset(storage_.get() + used_, 0, allocated_ - used_);
}
FML_DCHECK(used_ + size <= allocated_);
auto op = (T*)(storage_.get() + used_);
auto op = reinterpret_cast<T*>(storage_.get() + used_);
used_ += size;
new (op) T{std::forward<Args>(args)...};
op->type = T::kType;
Expand Down
9 changes: 5 additions & 4 deletions flow/display_list.h
Original file line number Diff line number Diff line change
Expand Up @@ -559,7 +559,7 @@ class DisplayListFlags {

class DisplayListFlagsBase : protected DisplayListFlags {
protected:
DisplayListFlagsBase(int flags) : flags_(flags) {}
explicit DisplayListFlagsBase(int flags) : flags_(flags) {}

const int flags_;

Expand Down Expand Up @@ -594,7 +594,8 @@ class DisplayListSpecialGeometryFlags : DisplayListFlagsBase {
bool may_have_acute_joins() const { return has_any(kMayHaveAcuteJoins_); }

private:
DisplayListSpecialGeometryFlags(int flags) : DisplayListFlagsBase(flags) {
explicit DisplayListSpecialGeometryFlags(int flags)
: DisplayListFlagsBase(flags) {
FML_DCHECK((flags & kAnySpecialGeometryMask_) == flags);
}

Expand Down Expand Up @@ -675,7 +676,7 @@ class DisplayListAttributeFlags : DisplayListFlagsBase {
bool is_flood() const { return has_any(kFloodsSurface_); }

private:
DisplayListAttributeFlags(int flags)
explicit DisplayListAttributeFlags(int flags)
: DisplayListFlagsBase(flags),
special_flags_(flags & kAnySpecialGeometryMask_) {
FML_DCHECK((flags & kIsAnyGeometryMask_) == kIsNonGeometric_ ||
Expand All @@ -694,7 +695,7 @@ class DisplayListAttributeFlags : DisplayListFlagsBase {

const DisplayListAttributeFlags without(int remove) const {
FML_DCHECK(has_all(remove));
return (flags_ & ~remove);
return DisplayListAttributeFlags(flags_ & ~remove);
}

const DisplayListSpecialGeometryFlags special_flags_;
Expand Down
6 changes: 3 additions & 3 deletions flow/display_list_canvas_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ static void EmptyDlRenderer(DisplayListBuilder&) {}

class RenderSurface {
public:
RenderSurface(sk_sp<SkSurface> surface) : surface_(surface) {}
explicit RenderSurface(sk_sp<SkSurface> surface) : surface_(surface) {}
~RenderSurface() { sk_free(addr_); }

SkCanvas* canvas() { return surface_->getCanvas(); }
Expand Down Expand Up @@ -311,7 +311,7 @@ class RenderEnvironment {
const SkPixmap* ref_pixmap() const { return ref_pixmap_; }

private:
RenderEnvironment(const SkImageInfo& info)
explicit RenderEnvironment(const SkImageInfo& info)
: info_(info), ref_surface_(MakeSurface()) {}

const SkImageInfo info_;
Expand Down Expand Up @@ -609,7 +609,7 @@ class TestParameters {

class CaseParameters {
public:
CaseParameters(std::string info)
explicit CaseParameters(std::string info)
: CaseParameters(info, EmptyCvRenderer, EmptyDlRenderer) {}

CaseParameters(std::string info, CvSetup cv_setup, DlRenderer dl_setup)
Expand Down
2 changes: 1 addition & 1 deletion flow/display_list_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ void ClipBoundsDispatchHelper::restore() {
}
}
void ClipBoundsDispatchHelper::reset(const SkRect* cull_rect) {
if ((has_clip_ = ((bool)cull_rect)) && !cull_rect->isEmpty()) {
if ((has_clip_ = cull_rect != nullptr) && !cull_rect->isEmpty()) {
bounds_ = *cull_rect;
} else {
bounds_.setEmpty();
Expand Down
3 changes: 2 additions & 1 deletion fml/mapping_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ TEST(MallocMapping, MoveConstructor) {
MallocMapping mapping(reinterpret_cast<uint8_t*>(malloc(length)), length);
MallocMapping moved = std::move(mapping);

ASSERT_EQ(nullptr, mapping.GetMapping());
ASSERT_EQ(nullptr,
mapping.GetMapping()); // NOLINT(clang-analyzer-cplusplus.Move)
ASSERT_EQ(0u, mapping.GetSize());
ASSERT_NE(nullptr, moved.GetMapping());
ASSERT_EQ(length, moved.GetSize());
Expand Down
1 change: 1 addition & 0 deletions fml/platform/linux/message_loop_linux.cc
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ void MessageLoopLinux::Terminate() {
// |fml::MessageLoopImpl|
void MessageLoopLinux::WakeUp(fml::TimePoint time_point) {
bool result = TimerRearm(timer_fd_.get(), time_point);
(void)result;
FML_DCHECK(result);
}

Expand Down
2 changes: 1 addition & 1 deletion fml/platform/linux/timerfd.cc
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ bool TimerRearm(int fd, fml::TimePoint time_point) {
}

struct itimerspec spec = {};
spec.it_value.tv_sec = (time_t)(nano_secs / NSEC_PER_SEC);
spec.it_value.tv_sec = static_cast<time_t>(nano_secs / NSEC_PER_SEC);
spec.it_value.tv_nsec = nano_secs % NSEC_PER_SEC;
spec.it_interval = spec.it_value; // single expiry.

Expand Down
1 change: 1 addition & 0 deletions fml/synchronization/semaphore.cc
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ class PlatformSemaphore {
~PlatformSemaphore() {
if (valid_) {
int result = ::sem_destroy(&sem_);
(void)result;
// Can only be EINVAL which should not be possible since we checked for
// validity.
FML_DCHECK(result == 0);
Expand Down
3 changes: 2 additions & 1 deletion lib/ui/painting/image_filter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,8 @@ static const std::array<SkSamplingOptions, 4> filter_qualities = {
SkSamplingOptions ImageFilter::SamplingFromIndex(int filterQualityIndex) {
if (filterQualityIndex < 0) {
return filter_qualities.front();
} else if (((size_t)filterQualityIndex) >= filter_qualities.size()) {
} else if (static_cast<size_t>(filterQualityIndex) >=
filter_qualities.size()) {
return filter_qualities.back();
} else {
return filter_qualities[filterQualityIndex];
Expand Down
2 changes: 1 addition & 1 deletion lib/ui/painting/image_generator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ unsigned int BuiltinSkiaCodecImageGenerator::GetPlayCount() const {

const ImageGenerator::FrameInfo BuiltinSkiaCodecImageGenerator::GetFrameInfo(
unsigned int frame_index) const {
SkCodec::FrameInfo info;
SkCodec::FrameInfo info = {};
codec_generator_->getFrameInfo(frame_index, &info);
return {
.required_frame = info.fRequiredFrame == SkCodec::kNoFrame
Expand Down
2 changes: 1 addition & 1 deletion lib/ui/painting/image_generator_registry_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ TEST_F(ShellTest, CreateCompatibleReturnsNullptrForInvalidImage) {

class FakeImageGenerator : public ImageGenerator {
public:
FakeImageGenerator(int identifiableFakeWidth)
explicit FakeImageGenerator(int identifiableFakeWidth)
: info_(SkImageInfo::Make(identifiableFakeWidth,
identifiableFakeWidth,
SkColorType::kRGBA_8888_SkColorType,
Expand Down
5 changes: 3 additions & 2 deletions runtime/isolate_configuration.cc
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ class AppSnapshotIsolateConfiguration final : public IsolateConfiguration {

class KernelIsolateConfiguration : public IsolateConfiguration {
public:
KernelIsolateConfiguration(std::unique_ptr<const fml::Mapping> kernel)
explicit KernelIsolateConfiguration(
std::unique_ptr<const fml::Mapping> kernel)
: kernel_(std::move(kernel)) {}

// |IsolateConfiguration|
Expand All @@ -69,7 +70,7 @@ class KernelIsolateConfiguration : public IsolateConfiguration {

class KernelListIsolateConfiguration final : public IsolateConfiguration {
public:
KernelListIsolateConfiguration(
explicit KernelListIsolateConfiguration(
std::vector<std::future<std::unique_ptr<const fml::Mapping>>>
kernel_pieces)
: kernel_piece_futures_(std::move(kernel_pieces)) {
Expand Down
1 change: 1 addition & 0 deletions shell/common/shell.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1367,6 +1367,7 @@ void Shell::OnFrameRasterized(const FrameTiming& timing) {
}

size_t old_count = unreported_timings_.size();
(void)old_count;
for (auto phase : FrameTiming::kPhases) {
unreported_timings_.push_back(
timing.Get(phase).ToEpochDelta().ToMicroseconds());
Expand Down
2 changes: 1 addition & 1 deletion shell/common/shell_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1880,7 +1880,7 @@ TEST_F(ShellTest, CanConvertToAndFromMappings) {
const size_t buffer_size = 2 << 20;

uint8_t* buffer = static_cast<uint8_t*>(::malloc(buffer_size));
ASSERT_NE(buffer, nullptr);
ASSERT_TRUE(buffer != nullptr);
ASSERT_TRUE(MemsetPatternSetOrCheck(
buffer, buffer_size, MemsetPatternOp::kMemsetPatternOpSetBuffer));

Expand Down
2 changes: 1 addition & 1 deletion shell/common/switches.cc
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ static bool GetSwitchValue(const fml::CommandLine& command_line,

std::unique_ptr<fml::Mapping> GetSymbolMapping(std::string symbol_prefix,
std::string native_lib_path) {
const uint8_t* mapping;
const uint8_t* mapping = nullptr;
intptr_t size;

auto lookup_symbol = [&mapping, &size, symbol_prefix](
Expand Down
2 changes: 1 addition & 1 deletion shell/gpu/gpu_surface_gl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ static SkColorType FirstSupportedColorType(GrDirectContext* context,
static sk_sp<SkSurface> WrapOnscreenSurface(GrDirectContext* context,
const SkISize& size,
intptr_t fbo) {
GrGLenum format;
GrGLenum format = kUnknown_SkColorType;
const SkColorType color_type = FirstSupportedColorType(context, &format);

GrGLFramebufferInfo framebuffer_info = {};
Expand Down
Loading

0 comments on commit eac4cd2

Please sign in to comment.