[lint] Merge impeller .clang-tidy into main config (#33692)

Merges most (but not all) of the impeller .clang-tidy rules into the main .clang-tidy config. Merges: readability-identifier-naming.PrivateMemberSuffix (_) readability-identifier-naming.EnumConstantPrefix (k) modernize-use-default-member-init.UseAssignment Does not merge: readability-identifier-naming.PublicMethodCase (CamelCase) readability-identifier-naming.PrivateMethodCase (CamelCase) These last two are not merged due to the non-trivial number of existing field accessors that use field_name() methods to directly return field_name_. While these are permitted by the C++ style guide, we may want to move to a single, simple rule and name everything in CamelCase. These can be enabled in a followup patch. No new tests added, since this change is style-only.
flutter · Jun 21, 2022 · 31afa38 · 31afa38
1 parent 47988c1
commit 31afa38
Show file tree

Hide file tree

Showing 31 changed files with 170 additions and 172 deletions.
diff --git a/.clang-tidy b/.clang-tidy
@@ -2,6 +2,7 @@
 Checks: "google-*,\
 clang-analyzer-*,\
 clang-diagnostic-*,\
+modernize-use-default-member-init,\
 readability-identifier-naming,\
 -google-objc-global-variable-declaration,\
 -google-objc-avoid-throwing-exception,\
@@ -22,5 +23,15 @@ google-objc-*,\
 google-explicit-constructor"
 
 CheckOptions:
-  - key: readability-identifier-naming.GlobalConstantPrefix
-    value: k
+  - key:    modernize-use-default-member-init.UseAssignment
+    value:  true
+  - key:    readability-identifier-naming.EnumConstantCase
+    value:  'CamelCase'
+  - key:    readability-identifier-naming.EnumConstantPrefix
+    value:  'k'
+  - key:    readability-identifier-naming.GlobalConstantPrefix
+    value:  'k'
+  - key:    readability-identifier-naming.PrivateMemberCase
+    value:  'lower_case'
+  - key:    readability-identifier-naming.PrivateMemberSuffix
+    value:  '_'
diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter
@@ -397,7 +397,6 @@ FILE: ../../../flutter/fml/unique_fd.cc
 FILE: ../../../flutter/fml/unique_fd.h
 FILE: ../../../flutter/fml/unique_object.h
 FILE: ../../../flutter/fml/wakeable.h
-FILE: ../../../flutter/impeller/.clang-tidy
 FILE: ../../../flutter/impeller/aiks/aiks_context.cc
 FILE: ../../../flutter/impeller/aiks/aiks_context.h
 FILE: ../../../flutter/impeller/aiks/aiks_playground.cc

diff --git a/display_list/display_list_unittests.cc b/display_list/display_list_unittests.cc
@@ -1360,15 +1360,15 @@ TEST(DisplayList, DisplayListBlenderRefHandling) {
   class BlenderRefTester : public virtual AttributeRefTester {
    public:
     void setRefToPaint(SkPaint& paint) const override {
-      paint.setBlender(blender);
+      paint.setBlender(blender_);
     }
     void setRefToDisplayList(DisplayListBuilder& builder) const override {
-      builder.setBlender(blender);
+      builder.setBlender(blender_);
     }
-    bool ref_is_unique() const override { return blender->unique(); }
+    bool ref_is_unique() const override { return blender_->unique(); }
 
    private:
-    sk_sp<SkBlender> blender =
+    sk_sp<SkBlender> blender_ =
         SkBlenders::Arithmetic(0.25, 0.25, 0.25, 0.25, true);
   };
 

diff --git a/flow/surface_frame.cc b/flow/surface_frame.cc
@@ -17,8 +17,7 @@ SurfaceFrame::SurfaceFrame(sk_sp<SkSurface> surface,
                            const SubmitCallback& submit_callback,
                            std::unique_ptr<GLContextResult> context_result,
                            bool display_list_fallback)
-    : submitted_(false),
-      surface_(surface),
+    : surface_(surface),
       framebuffer_info_(std::move(framebuffer_info)),
       submit_callback_(submit_callback),
       context_result_(std::move(context_result)) {

diff --git a/fml/platform/posix/mapping_posix.cc b/fml/platform/posix/mapping_posix.cc
@@ -51,8 +51,7 @@ Mapping::Mapping() = default;
 Mapping::~Mapping() = default;
 
 FileMapping::FileMapping(const fml::UniqueFD& handle,
-                         std::initializer_list<Protection> protection)
-    : size_(0), mapping_(nullptr) {
+                         std::initializer_list<Protection> protection) {
   if (!handle.is_valid()) {
     return;
   }

diff --git a/fml/synchronization/semaphore.cc b/fml/synchronization/semaphore.cc
@@ -15,44 +15,44 @@ namespace fml {
 class PlatformSemaphore {
  public:
   explicit PlatformSemaphore(uint32_t count)
-      : _sem(dispatch_semaphore_create(count)), _initial(count) {}
+      : sem_(dispatch_semaphore_create(count)), initial_(count) {}
 
   ~PlatformSemaphore() {
-    for (uint32_t i = 0; i < _initial; ++i) {
+    for (uint32_t i = 0; i < initial_; ++i) {
       Signal();
     }
-    if (_sem != nullptr) {
-      dispatch_release(reinterpret_cast<dispatch_object_t>(_sem));
-      _sem = nullptr;
+    if (sem_ != nullptr) {
+      dispatch_release(reinterpret_cast<dispatch_object_t>(sem_));
+      sem_ = nullptr;
     }
   }
 
-  bool IsValid() const { return _sem != nullptr; }
+  bool IsValid() const { return sem_ != nullptr; }
 
   bool Wait() {
-    if (_sem == nullptr) {
+    if (sem_ == nullptr) {
       return false;
     }
-    return dispatch_semaphore_wait(_sem, DISPATCH_TIME_FOREVER) == 0;
+    return dispatch_semaphore_wait(sem_, DISPATCH_TIME_FOREVER) == 0;
   }
 
   bool TryWait() {
-    if (_sem == nullptr) {
+    if (sem_ == nullptr) {
       return false;
     }
 
-    return dispatch_semaphore_wait(_sem, DISPATCH_TIME_NOW) == 0;
+    return dispatch_semaphore_wait(sem_, DISPATCH_TIME_NOW) == 0;
   }
 
   void Signal() {
-    if (_sem != nullptr) {
-      dispatch_semaphore_signal(_sem);
+    if (sem_ != nullptr) {
+      dispatch_semaphore_signal(sem_);
     }
   }
 
  private:
-  dispatch_semaphore_t _sem;
-  const uint32_t _initial;
+  dispatch_semaphore_t sem_;
+  const uint32_t initial_;
 
   FML_DISALLOW_COPY_AND_ASSIGN(PlatformSemaphore);
 };

diff --git a/impeller/.clang-tidy b/impeller/.clang-tidy
diff --git a/impeller/archivist/archive_database.cc b/impeller/archivist/archive_database.cc
@@ -17,7 +17,7 @@
 namespace impeller {
 
 struct ArchiveDatabase::Handle {
-  Handle(const std::string& filename) {
+  explicit Handle(const std::string& filename) {
     if (::sqlite3_initialize() != SQLITE_OK) {
       VALIDATION_LOG << "Could not initialize sqlite.";
       return;

diff --git a/impeller/archivist/archivist_unittests.cc b/impeller/archivist/archivist_unittests.cc
@@ -18,7 +18,7 @@ static int64_t LastSample = 0;
 
 class Sample : public Archivable {
  public:
-  Sample(uint64_t count = 42) : some_data_(count) {}
+  explicit Sample(uint64_t count = 42) : some_data_(count) {}
 
   Sample(Sample&&) = default;
 

diff --git a/impeller/base/base_unittests.cc b/impeller/base/base_unittests.cc
@@ -44,7 +44,7 @@ TEST(ThreadTest, CanCreateRWMutex) {
   f.mtx.UnlockWriter();
   // int b = f.a; <--- Static analysis error.
   f.mtx.LockReader();
-  int b = f.a;
+  int b = f.a;  // NOLINT(clang-analyzer-deadcode.DeadStores)
   FML_ALLOW_UNUSED_LOCAL(b);
   f.mtx.UnlockReader();
 }
@@ -61,7 +61,7 @@ TEST(ThreadTest, CanCreateRWMutexLock) {
   // int b = f.a; <--- Static analysis error.
   {
     auto read_lock = ReaderLock(f.mtx);
-    int b = f.a;
+    int b = f.a;  // NOLINT(clang-analyzer-deadcode.DeadStores)
     FML_ALLOW_UNUSED_LOCAL(b);
   }
 

diff --git a/impeller/blobcat/blob_library.cc b/impeller/blobcat/blob_library.cc
@@ -42,7 +42,7 @@ BlobLibrary::BlobLibrary(std::shared_ptr<fml::Mapping> mapping)
   {
     const size_t read_size = sizeof(Blob) * header.blob_count;
     ::memcpy(blobs.data(), mapping_->GetMapping() + offset, read_size);
-    offset += read_size;
+    offset += read_size;  // NOLINT(clang-analyzer-deadcode.DeadStores)
   }
 
   // Read the blobs.

diff --git a/impeller/compiler/reflector.cc b/impeller/compiler/reflector.cc
@@ -2,6 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+// FLUTTER_NOLINT: https://github.com/flutter/flutter/issues/105732
+
 #include "impeller/compiler/reflector.h"
 
 #include <atomic>

diff --git a/impeller/entity/entity_unittests.cc b/impeller/entity/entity_unittests.cc
@@ -38,7 +38,8 @@ TEST_P(EntityTest, CanCreateEntity) {
 
 class TestPassDelegate final : public EntityPassDelegate {
  public:
-  TestPassDelegate(std::optional<Rect> coverage) : coverage_(coverage) {}
+  explicit TestPassDelegate(std::optional<Rect> coverage)
+      : coverage_(coverage) {}
 
   // |EntityPassDelegate|
   ~TestPassDelegate() override = default;

diff --git a/impeller/geometry/path_component.h b/impeller/geometry/path_component.h
@@ -130,7 +130,7 @@ struct CubicPathComponent {
 
 struct ContourComponent {
   Point destination;
-  bool is_closed;
+  bool is_closed = false;
 
   ContourComponent() {}
 

diff --git a/impeller/renderer/backend/gles/texture_gles.cc b/impeller/renderer/backend/gles/texture_gles.cc
@@ -90,7 +90,7 @@ struct TexImage2DData {
   GLenum type = GL_NONE;
   std::shared_ptr<const fml::Mapping> data;
 
-  TexImage2DData(PixelFormat pixel_format) {
+  explicit TexImage2DData(PixelFormat pixel_format) {
     switch (pixel_format) {
       case PixelFormat::kA8UNormInt:
         internal_format = GL_ALPHA;

diff --git a/impeller/renderer/backend/metal/command_buffer_mtl.mm b/impeller/renderer/backend/metal/command_buffer_mtl.mm
@@ -8,6 +8,9 @@
 
 namespace impeller {
 namespace {
+
+// NOLINTBEGIN(readability-identifier-naming)
+
 // TODO(dnfield): remove this declaration when we no longer need to build on
 // machines with lower SDK versions than 11.0.
 #if !defined(MAC_OS_VERSION_11_0) || \
@@ -21,6 +24,8 @@ typedef NS_ENUM(NSInteger, MTLCommandEncoderErrorState) {
 } API_AVAILABLE(macos(11.0), ios(14.0));
 #endif
 
+// NOLINTEND(readability-identifier-naming)
+
 API_AVAILABLE(ios(14.0), macos(11.0))
 NSString* MTLCommandEncoderErrorStateToString(
     MTLCommandEncoderErrorState state) {
@@ -39,6 +44,8 @@ typedef NS_ENUM(NSInteger, MTLCommandEncoderErrorState) {
   return @"unknown";
 }
 
+// NOLINTBEGIN(readability-identifier-naming)
+
 // TODO(dnfield): This can be removed when all bots have been sufficiently
 // upgraded for MAC_OS_VERSION_12_0.
 #if !defined(MAC_OS_VERSION_12_0) || \
@@ -47,6 +54,8 @@ typedef NS_ENUM(NSInteger, MTLCommandEncoderErrorState) {
 constexpr int MTLCommandBufferErrorStackOverflow = 12;
 #endif
 
+// NOLINTEND(readability-identifier-naming)
+
 static NSString* MTLCommandBufferErrorToString(MTLCommandBufferError code) {
   switch (code) {
     case MTLCommandBufferErrorNone:

diff --git a/impeller/renderer/backend/metal/render_pass_mtl.mm b/impeller/renderer/backend/metal/render_pass_mtl.mm
@@ -186,7 +186,8 @@ static bool ConfigureStencilAttachment(
 ///             absent.
 ///
 struct PassBindingsCache {
-  PassBindingsCache(id<MTLRenderCommandEncoder> encoder) : encoder_(encoder) {}
+  explicit PassBindingsCache(id<MTLRenderCommandEncoder> encoder)
+      : encoder_(encoder) {}
 
   PassBindingsCache(const PassBindingsCache&) = delete;
 

diff --git a/lib/ui/painting/paint.cc b/lib/ui/painting/paint.cc
@@ -64,7 +64,7 @@ constexpr float kInvertColors[20] = {
 // clang-format on
 
 // Must be kept in sync with the MaskFilter private constants in painting.dart.
-enum MaskFilterType { Null, Blur };
+enum MaskFilterType { kNull, kBlur };
 
 Paint::Paint(Dart_Handle paint_objects, Dart_Handle paint_data)
     : paint_objects_(paint_objects), paint_data_(paint_data) {}
@@ -169,9 +169,9 @@ const SkPaint* Paint::paint(SkPaint& paint) const {
   }
 
   switch (uint_data[kMaskFilterIndex]) {
-    case Null:
+    case kNull:
       break;
-    case Blur:
+    case kBlur:
       SkBlurStyle blur_style =
           static_cast<SkBlurStyle>(uint_data[kMaskFilterBlurStyleIndex]);
       double sigma = float_data[kMaskFilterSigmaIndex];
@@ -300,10 +300,10 @@ bool Paint::sync_to(DisplayListBuilder* builder,
 
   if (flags.applies_mask_filter()) {
     switch (uint_data[kMaskFilterIndex]) {
-      case Null:
+      case kNull:
         builder->setMaskFilter(nullptr);
         break;
-      case Blur:
+      case kBlur:
         SkBlurStyle blur_style =
             static_cast<SkBlurStyle>(uint_data[kMaskFilterBlurStyleIndex]);
         double sigma = float_data[kMaskFilterSigmaIndex];

diff --git a/runtime/dart_isolate.cc b/runtime/dart_isolate.cc
@@ -42,7 +42,7 @@ constexpr std::string_view kFileUriPrefix = "file://";
 
 class DartErrorString {
  public:
-  DartErrorString() : str_(nullptr) {}
+  DartErrorString() {}
   ~DartErrorString() {
     if (str_) {
       ::free(str_);
@@ -54,7 +54,7 @@ class DartErrorString {
 
  private:
   FML_DISALLOW_COPY_AND_ASSIGN(DartErrorString);
-  char* str_;
+  char* str_ = nullptr;
 };
 
 }  // anonymous namespace

diff --git a/shell/common/skia_event_tracer_impl.cc b/shell/common/skia_event_tracer_impl.cc
@@ -81,7 +81,7 @@ class FlutterEventTracer : public SkEventTracer {
 
   FlutterEventTracer(bool enabled,
                      const std::optional<std::vector<std::string>>& allowlist)
-      : enabled_(enabled ? kYes : kNo), shaders_category_flag_(nullptr) {
+      : enabled_(enabled ? kYes : kNo) {
     if (allowlist.has_value()) {
       allowlist_.emplace();
       for (const std::string& category : *allowlist) {
@@ -311,7 +311,7 @@ class FlutterEventTracer : public SkEventTracer {
   std::mutex flag_map_mutex_;
   std::map<const char*, uint8_t> category_flag_map_;
   std::map<const uint8_t*, const char*> reverse_flag_map_;
-  const uint8_t* shaders_category_flag_;
+  const uint8_t* shaders_category_flag_ = nullptr;
   FML_DISALLOW_COPY_AND_ASSIGN(FlutterEventTracer);
 };
 

diff --git a/shell/gpu/gpu_surface_gl_skia.cc b/shell/gpu/gpu_surface_gl_skia.cc
@@ -73,7 +73,7 @@ GPUSurfaceGLSkia::GPUSurfaceGLSkia(sk_sp<GrDirectContext> gr_context,
                                    bool render_to_surface)
     : delegate_(delegate),
       context_(gr_context),
-      context_owner_(false),
+
       render_to_surface_(render_to_surface),
       weak_factory_(this) {
   auto context_switch = delegate_->GLContextMakeCurrent();