Skip to content

Commit

Permalink
Add clang-analyzer-* and clang-diagnostic-* to .clang-tidy (#31291)
Browse files Browse the repository at this point in the history
  • Loading branch information
zanderso authored Feb 9, 2022
1 parent 2a8f388 commit ba8f1c5
Show file tree
Hide file tree
Showing 44 changed files with 191 additions and 56 deletions.
11 changes: 9 additions & 2 deletions .clang-tidy
Original file line number Diff line number Diff line change
@@ -1,13 +1,20 @@
# Prefix check with "-" to ignore.
Checks: "google-*,\
clang-analyzer-*,\
clang-diagnostic-*,\
-google-objc-global-variable-declaration,\
-google-objc-avoid-throwing-exception"
-google-objc-avoid-throwing-exception,\
-clang-analyzer-nullability.NullPassedToNonnull,\
-clang-analyzer-nullability.NullablePassedToNonnull,\
-clang-analyzer-nullability.NullReturnedFromNonnull,\
-clang-analyzer-nullability.NullableReturnedFromNonnull"

# Only warnings treated as errors are reported
# in the "ci/lint.sh" script and pre-push git hook.
# Add checks when all warnings are fixed
# to prevent new warnings being introduced.
# https://github.com/flutter/flutter/issues/93279
WarningsAsErrors: "clang-analyzer-osx.*,\
WarningsAsErrors: "clang-analyzer-*,\
clang-diagnostic-*,\
google-objc-*,\
google-explicit-constructor"
3 changes: 3 additions & 0 deletions common/graphics/persistent_cache.cc
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,9 @@ sk_sp<SkData> ParseBase64(const std::string& input) {
}

size_t PersistentCache::PrecompileKnownSkSLs(GrDirectContext* context) const {
// clang-tidy has trouble reasoning about some of the complicated array and
// pointer-arithmetic code in rapidjson.
// NOLINTNEXTLINE(clang-analyzer-cplusplus.PlacementNew)
auto known_sksls = LoadSkSLs();
// A trace must be present even if no precompilations have been completed.
FML_TRACE_EVENT("flutter", "PersistentCache::PrecompileKnownSkSLs", "count",
Expand Down
41 changes: 21 additions & 20 deletions display_list/display_list_benchmarks.cc
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ void BM_DrawLine(benchmark::State& state,
auto display_list = builder.Build();

// We only want to time the actual rasterization.
for (auto _ : state) {
for ([[maybe_unused]] auto _ : state) {
display_list->RenderTo(canvas);
canvas_provider->GetSurface()->flushAndSubmit(true);
}
Expand Down Expand Up @@ -162,7 +162,7 @@ void BM_DrawRect(benchmark::State& state,
auto display_list = builder.Build();

// We only want to time the actual rasterization.
for (auto _ : state) {
for ([[maybe_unused]] auto _ : state) {
display_list->RenderTo(canvas);
canvas_provider->GetSurface()->flushAndSubmit(true);
}
Expand Down Expand Up @@ -207,7 +207,7 @@ void BM_DrawOval(benchmark::State& state,
auto display_list = builder.Build();

// We only want to time the actual rasterization.
for (auto _ : state) {
for ([[maybe_unused]] auto _ : state) {
display_list->RenderTo(canvas);
canvas_provider->GetSurface()->flushAndSubmit(true);
}
Expand Down Expand Up @@ -254,7 +254,7 @@ void BM_DrawCircle(benchmark::State& state,
auto display_list = builder.Build();

// We only want to time the actual rasterization.
for (auto _ : state) {
for ([[maybe_unused]] auto _ : state) {
display_list->RenderTo(canvas);
canvas_provider->GetSurface()->flushAndSubmit(true);
}
Expand Down Expand Up @@ -331,7 +331,7 @@ void BM_DrawRRect(benchmark::State& state,
auto display_list = builder.Build();

// We only want to time the actual rasterization.
for (auto _ : state) {
for ([[maybe_unused]] auto _ : state) {
display_list->RenderTo(canvas);
canvas_provider->GetSurface()->flushAndSubmit(true);
}
Expand Down Expand Up @@ -412,7 +412,7 @@ void BM_DrawDRRect(benchmark::State& state,
auto display_list = builder.Build();

// We only want to time the actual rasterization.
for (auto _ : state) {
for ([[maybe_unused]] auto _ : state) {
display_list->RenderTo(canvas);
canvas_provider->GetSurface()->flushAndSubmit(true);
}
Expand Down Expand Up @@ -464,7 +464,7 @@ void BM_DrawArc(benchmark::State& state,
auto display_list = builder.Build();

// We only want to time the actual rasterization.
for (auto _ : state) {
for ([[maybe_unused]] auto _ : state) {
display_list->RenderTo(canvas);
canvas_provider->GetSurface()->flushAndSubmit(true);
}
Expand All @@ -482,7 +482,7 @@ std::vector<SkPoint> GetPolygonPoints(size_t n, SkPoint center, SkScalar r) {
float angle;
float full_circle = 2.0f * M_PI;
for (size_t i = 0; i < n; i++) {
angle = (full_circle / (float)n) * (float)i;
angle = (full_circle / static_cast<float>(n)) * static_cast<float>(i);
x = center.x() + r * std::cosf(angle);
y = center.y() + r * std::sinf(angle);
points.push_back(SkPoint::Make(x, y));
Expand Down Expand Up @@ -666,7 +666,7 @@ void BM_DrawPath(benchmark::State& state,
auto display_list = builder.Build();

// We only want to time the actual rasterization.
for (auto _ : state) {
for ([[maybe_unused]] auto _ : state) {
display_list->RenderTo(canvas);
canvas_provider->GetSurface()->flushAndSubmit(true);
}
Expand Down Expand Up @@ -709,12 +709,13 @@ sk_sp<SkVertices> GetTestVertices(SkPoint center,
colors.push_back(SK_ColorCYAN);
for (size_t i = 0; i <= outer_points.size(); i++) {
vertices.push_back(outer_points[i % outer_points.size()]);
if (i % 3 == 0)
if (i % 3 == 0) {
colors.push_back(SK_ColorRED);
else if (i % 3 == 1)
} else if (i % 3 == 1) {
colors.push_back(SK_ColorGREEN);
else
} else {
colors.push_back(SK_ColorBLUE);
}
}
break;
case SkVertices::VertexMode::kTriangles_VertexMode:
Expand Down Expand Up @@ -810,7 +811,7 @@ void BM_DrawVertices(benchmark::State& state,
auto display_list = builder.Build();

// We only want to time the actual rasterization.
for (auto _ : state) {
for ([[maybe_unused]] auto _ : state) {
display_list->RenderTo(canvas);
canvas_provider->GetSurface()->flushAndSubmit(true);
}
Expand Down Expand Up @@ -913,7 +914,7 @@ void BM_DrawPoints(benchmark::State& state,

auto display_list = builder.Build();

for (auto _ : state) {
for ([[maybe_unused]] auto _ : state) {
display_list->RenderTo(canvas);
canvas_provider->GetSurface()->flushAndSubmit(true);
}
Expand Down Expand Up @@ -987,7 +988,7 @@ void BM_DrawImage(benchmark::State& state,

auto display_list = builder.Build();

for (auto _ : state) {
for ([[maybe_unused]] auto _ : state) {
display_list->RenderTo(canvas);
canvas_provider->GetSurface()->flushAndSubmit(true);
}
Expand Down Expand Up @@ -1069,7 +1070,7 @@ void BM_DrawImageRect(benchmark::State& state,

auto display_list = builder.Build();

for (auto _ : state) {
for ([[maybe_unused]] auto _ : state) {
display_list->RenderTo(canvas);
canvas_provider->GetSurface()->flushAndSubmit(true);
}
Expand Down Expand Up @@ -1153,7 +1154,7 @@ void BM_DrawImageNine(benchmark::State& state,

auto display_list = builder.Build();

for (auto _ : state) {
for ([[maybe_unused]] auto _ : state) {
display_list->RenderTo(canvas);
canvas_provider->GetSurface()->flushAndSubmit(true);
}
Expand Down Expand Up @@ -1226,7 +1227,7 @@ void BM_DrawTextBlob(benchmark::State& state,

auto display_list = builder.Build();

for (auto _ : state) {
for ([[maybe_unused]] auto _ : state) {
display_list->RenderTo(canvas);
canvas_provider->GetSurface()->flushAndSubmit(true);
}
Expand Down Expand Up @@ -1290,7 +1291,7 @@ void BM_DrawShadow(benchmark::State& state,
auto display_list = builder.Build();

// We only want to time the actual rasterization.
for (auto _ : state) {
for ([[maybe_unused]] auto _ : state) {
display_list->RenderTo(canvas);
canvas_provider->GetSurface()->flushAndSubmit(true);
}
Expand Down Expand Up @@ -1343,7 +1344,7 @@ void BM_SaveLayer(benchmark::State& state,
auto display_list = builder.Build();

// We only want to time the actual rasterization.
for (auto _ : state) {
for ([[maybe_unused]] auto _ : state) {
display_list->RenderTo(canvas);
canvas_provider->GetSurface()->flushAndSubmit(true);
}
Expand Down
3 changes: 3 additions & 0 deletions flow/paint_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,11 @@ void DrawCheckerboard(SkCanvas* canvas, const SkRect& rect) {
canvas->save();
canvas->clipRect(rect);

// Secure random number generation isn't needed here.
// NOLINTBEGIN(clang-analyzer-security.insecureAPI.rand)
auto checkerboard_color =
SkColorSetARGB(64, rand() % 256, rand() % 256, rand() % 256);
// NOLINTEND(clang-analyzer-security.insecureAPI.rand)

DrawCheckerboard(canvas, checkerboard_color, 0x00000000, 12);
canvas->restore();
Expand Down
13 changes: 8 additions & 5 deletions fml/platform/android/jni_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ JNIEnv* AttachCurrentThread() {
} else {
args.name = thread_name;
}
jint ret = g_jvm->AttachCurrentThread(&env, &args);
[[maybe_unused]] jint ret = g_jvm->AttachCurrentThread(&env, &args);
FML_DCHECK(JNI_OK == ret);

FML_DCHECK(tls_jni_detach.get() == nullptr);
Expand Down Expand Up @@ -182,9 +182,10 @@ ScopedJavaLocalRef<jobjectArray> VectorToBufferArray(
env->NewObjectArray(vector.size(), byte_buffer_clazz.obj(), NULL);
ASSERT_NO_EXCEPTION();
for (size_t i = 0; i < vector.size(); ++i) {
uint8_t* data = const_cast<uint8_t*>(vector[i].data());
ScopedJavaLocalRef<jobject> item(
env,
env->NewDirectByteBuffer((void*)(vector[i].data()), vector[i].size()));
env, env->NewDirectByteBuffer(reinterpret_cast<void*>(data),
vector[i].size()));
env->SetObjectArrayElement(java_array, i, item.obj());
}
return ScopedJavaLocalRef<jobjectArray>(env, java_array);
Expand All @@ -195,16 +196,18 @@ bool HasException(JNIEnv* env) {
}

bool ClearException(JNIEnv* env) {
if (!HasException(env))
if (!HasException(env)) {
return false;
}
env->ExceptionDescribe();
env->ExceptionClear();
return true;
}

bool CheckException(JNIEnv* env) {
if (!HasException(env))
if (!HasException(env)) {
return true;
}

jthrowable exception = env->ExceptionOccurred();
env->ExceptionClear();
Expand Down
2 changes: 1 addition & 1 deletion fml/platform/android/message_loop_android.cc
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ void MessageLoopAndroid::Terminate() {
}

void MessageLoopAndroid::WakeUp(fml::TimePoint time_point) {
bool result = TimerRearm(timer_fd_.get(), time_point);
[[maybe_unused]] bool result = TimerRearm(timer_fd_.get(), time_point);
FML_DCHECK(result);
}

Expand Down
17 changes: 11 additions & 6 deletions fml/platform/android/scoped_java_ref.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,14 @@ namespace jni {
static const int kDefaultLocalFrameCapacity = 16;

ScopedJavaLocalFrame::ScopedJavaLocalFrame(JNIEnv* env) : env_(env) {
int failed = env_->PushLocalFrame(kDefaultLocalFrameCapacity);
[[maybe_unused]] int failed =
env_->PushLocalFrame(kDefaultLocalFrameCapacity);
FML_DCHECK(!failed);
}

ScopedJavaLocalFrame::ScopedJavaLocalFrame(JNIEnv* env, int capacity)
: env_(env) {
int failed = env_->PushLocalFrame(capacity);
[[maybe_unused]] int failed = env_->PushLocalFrame(capacity);
FML_DCHECK(!failed);
}

Expand All @@ -43,10 +44,12 @@ JNIEnv* JavaRef<jobject>::SetNewLocalRef(JNIEnv* env, jobject obj) {
} else {
FML_DCHECK(env == AttachCurrentThread()); // Is |env| on correct thread.
}
if (obj)
if (obj) {
obj = env->NewLocalRef(obj);
if (obj_)
}
if (obj_) {
env->DeleteLocalRef(obj_);
}
obj_ = obj;
return env;
}
Expand All @@ -57,10 +60,12 @@ void JavaRef<jobject>::SetNewGlobalRef(JNIEnv* env, jobject obj) {
} else {
FML_DCHECK(env == AttachCurrentThread()); // Is |env| on correct thread.
}
if (obj)
if (obj) {
obj = env->NewGlobalRef(obj);
if (obj_)
}
if (obj_) {
env->DeleteGlobalRef(obj_);
}
obj_ = obj;
}

Expand Down
5 changes: 5 additions & 0 deletions fml/synchronization/waitable_event_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@
#include "flutter/fml/macros.h"
#include "gtest/gtest.h"

// rand() is only used for tests in this file.
// NOLINTBEGIN(clang-analyzer-security.insecureAPI.rand)

namespace fml {
namespace {

Expand Down Expand Up @@ -183,3 +186,5 @@ TEST(ManualResetWaitableEventTest, SignalMultiple) {

} // namespace
} // namespace fml

// NOLINTEND(clang-analyzer-security.insecureAPI.rand)
5 changes: 5 additions & 0 deletions lib/ui/compositing/scene_builder_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@
#include "flutter/shell/common/thread_host.h"
#include "flutter/testing/testing.h"

// CREATE_NATIVE_ENTRY is leaky by design
// NOLINTBEGIN(clang-analyzer-core.StackAddressEscape)

namespace flutter {
namespace testing {

Expand Down Expand Up @@ -156,3 +159,5 @@ TEST_F(ShellTest, EngineLayerDisposeReleasesReference) {

} // namespace testing
} // namespace flutter

// NOLINTEND(clang-analyzer-core.StackAddressEscape)
5 changes: 5 additions & 0 deletions lib/ui/hooks_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@
#include "flutter/testing/testing.h"
#include "third_party/dart/runtime/include/dart_api.h"

// CREATE_NATIVE_ENTRY is leaky by design
// NOLINTBEGIN(clang-analyzer-core.StackAddressEscape)

namespace flutter {
namespace testing {

Expand Down Expand Up @@ -82,3 +85,5 @@ TEST_F(HooksTest, HooksUnitTests) {

} // namespace testing
} // namespace flutter

// NOLINTEND(clang-analyzer-core.StackAddressEscape)
5 changes: 5 additions & 0 deletions lib/ui/painting/image_encoding_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@
#include "flutter/testing/testing.h"
#include "gmock/gmock.h"

// CREATE_NATIVE_ENTRY is leaky by design
// NOLINTBEGIN(clang-analyzer-core.StackAddressEscape)

namespace flutter {
namespace testing {

Expand Down Expand Up @@ -167,3 +170,5 @@ TEST_F(ShellTest, EncodeImageAccessesSyncSwitch) {

} // namespace testing
} // namespace flutter

// NOLINTEND(clang-analyzer-core.StackAddressEscape)
3 changes: 3 additions & 0 deletions lib/ui/painting/multi_frame_codec.cc
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,9 @@ Dart_Handle MultiFrameCodec::getNextFrame(Dart_Handle callback_handle) {
}));

return Dart_Null();
// The static leak checker gets confused by the control flow, unique pointers
// and closures in this function.
// NOLINTNEXTLINE(clang-analyzer-cplusplus.NewDeleteLeaks)
}

int MultiFrameCodec::frameCount() const {
Expand Down
2 changes: 2 additions & 0 deletions runtime/dart_isolate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1008,9 +1008,11 @@ Dart_Isolate DartIsolate::CreateDartIsolateGroup(
{
// Ownership of the isolate data objects has been transferred to the Dart
// VM.
// NOLINTBEGIN(clang-analyzer-cplusplus.NewDeleteLeaks)
std::shared_ptr<DartIsolate> embedder_isolate(*isolate_data);
isolate_group_data.release();
isolate_data.release();
// NOLINTEND(clang-analyzer-cplusplus.NewDeleteLeaks)

success = InitializeIsolate(std::move(embedder_isolate), isolate, error);
}
Expand Down
Loading

0 comments on commit ba8f1c5

Please sign in to comment.