Turned on performance-unnecessary-value-param everywhere. (flutter#37447

) * Turned on performance-unnecessary-value-param everywhere. * linux host additions * ios patch * reverted bad fix * revert bad fix * another ios patch * removed lint fix printer
matanlurey · Nov 9, 2022 · aa4b3ea · aa4b3ea
1 parent 62b581e
commit aa4b3ea
Show file tree

Hide file tree

Showing 85 changed files with 232 additions and 196 deletions.
diff --git a/.clang-tidy b/.clang-tidy
@@ -28,7 +28,8 @@ clang-analyzer-*,\
 readability-identifier-naming,\
 clang-diagnostic-*,\
 google-objc-*,\
-google-explicit-constructor"
+google-explicit-constructor,\
+performance-unnecessary-value-param"
 
 CheckOptions:
   - key:    modernize-use-default-member-init.UseAssignment

diff --git a/ci/lint.sh b/ci/lint.sh
@@ -34,7 +34,7 @@ DART_BIN="${SRC_DIR}/third_party/dart/tools/sdks/dart-sdk/bin"
 DART="${DART_BIN}/dart"
 # TODO(https://github.com/flutter/flutter/issues/113848): Migrate all platforms
 # to have this as an error.
-MAC_HOST_WARNINGS_AS_ERRORS="performance-move-const-arg,performance-unnecessary-value-param"
+MAC_HOST_WARNINGS_AS_ERRORS="performance-move-const-arg"
 
 # FLUTTER_LINT_PRINT_FIX will make it so that fix is executed and the generated
 # diff is printed to stdout if clang-tidy fails. This is helpful for enabling

diff --git a/fml/platform/android/jni_weak_ref.cc b/fml/platform/android/jni_weak_ref.cc
@@ -54,12 +54,14 @@ ScopedJavaLocalRef<jobject> GetRealObject(JNIEnv* env, jweak obj) {
 }
 
 void JavaObjectWeakGlobalRef::Assign(const JavaObjectWeakGlobalRef& other) {
-  if (&other == this)
+  if (&other == this) {
     return;
+  }
 
   JNIEnv* env = AttachCurrentThread();
-  if (obj_)
+  if (obj_) {
     env->DeleteWeakGlobalRef(obj_);
+  }
 
   obj_ = other.obj_ ? env->NewWeakGlobalRef(other.obj_) : NULL;
 }

diff --git a/impeller/renderer/backend/vulkan/command_buffer_vk.cc b/impeller/renderer/backend/vulkan/command_buffer_vk.cc
@@ -4,6 +4,8 @@
 
 #include "impeller/renderer/backend/vulkan/command_buffer_vk.h"
 
+#include <utility>
+
 #include "flutter/fml/logging.h"
 #include "impeller/base/validation.h"
 #include "impeller/renderer/backend/vulkan/context_vk.h"
@@ -15,7 +17,7 @@
 namespace impeller {
 
 std::shared_ptr<CommandBufferVK> CommandBufferVK::Create(
-    std::weak_ptr<const Context> context,
+    const std::weak_ptr<const Context>& context,
     vk::Device device,
     vk::CommandPool command_pool,
     SurfaceProducerVK* surface_producer) {
@@ -41,7 +43,7 @@ CommandBufferVK::CommandBufferVK(std::weak_ptr<const Context> context,
                                  SurfaceProducerVK* surface_producer,
                                  vk::CommandPool command_pool,
                                  vk::UniqueCommandBuffer command_buffer)
-    : CommandBuffer(context),
+    : CommandBuffer(std::move(context)),
       device_(device),
       command_pool_(command_pool),
       command_buffer_(std::move(command_buffer)),

diff --git a/impeller/renderer/backend/vulkan/command_buffer_vk.h b/impeller/renderer/backend/vulkan/command_buffer_vk.h
@@ -14,7 +14,7 @@ namespace impeller {
 class CommandBufferVK final : public CommandBuffer {
  public:
   static std::shared_ptr<CommandBufferVK> Create(
-      std::weak_ptr<const Context> context,
+      const std::weak_ptr<const Context>& context,
       vk::Device device,
       vk::CommandPool command_pool,
       SurfaceProducerVK* surface_producer);

diff --git a/impeller/renderer/backend/vulkan/device_buffer_vk.cc b/impeller/renderer/backend/vulkan/device_buffer_vk.cc
@@ -39,7 +39,7 @@ DeviceBufferVK::DeviceBufferVK(
     DeviceBufferDescriptor desc,
     ContextVK& context,
     std::unique_ptr<DeviceBufferAllocationVK> device_allocation)
-    : DeviceBuffer(std::move(desc)),
+    : DeviceBuffer(desc),
       context_(context),
       device_allocation_(std::move(device_allocation)) {}
 

diff --git a/impeller/renderer/backend/vulkan/pipeline_vk.cc b/impeller/renderer/backend/vulkan/pipeline_vk.cc
@@ -40,9 +40,9 @@ vk::DescriptorSetLayout PipelineCreateInfoVK::GetDescriptorSetLayout() const {
 }
 
 PipelineVK::PipelineVK(std::weak_ptr<PipelineLibrary> library,
-                       PipelineDescriptor desc,
+                       const PipelineDescriptor& desc,
                        std::unique_ptr<PipelineCreateInfoVK> create_info)
-    : Pipeline(std::move(library), std::move(desc)),
+    : Pipeline(std::move(library), desc),
       pipeline_info_(std::move(create_info)) {}
 
 PipelineVK::~PipelineVK() = default;

diff --git a/impeller/renderer/backend/vulkan/pipeline_vk.h b/impeller/renderer/backend/vulkan/pipeline_vk.h
@@ -43,7 +43,7 @@ class PipelineVK final
       public BackendCast<PipelineVK, Pipeline<PipelineDescriptor>> {
  public:
   PipelineVK(std::weak_ptr<PipelineLibrary> library,
-             PipelineDescriptor desc,
+             const PipelineDescriptor& desc,
              std::unique_ptr<PipelineCreateInfoVK> create_info);
 
   // |Pipeline|

diff --git a/impeller/renderer/backend/vulkan/surface_producer_vk.cc b/impeller/renderer/backend/vulkan/surface_producer_vk.cc
@@ -5,6 +5,7 @@
 #include "impeller/renderer/backend/vulkan/surface_producer_vk.h"
 
 #include <array>
+#include <utility>
 
 #include "impeller/base/validation.h"
 #include "impeller/renderer/backend/vulkan/surface_vk.h"
@@ -13,7 +14,7 @@
 namespace impeller {
 
 std::unique_ptr<SurfaceProducerVK> SurfaceProducerVK::Create(
-    std::weak_ptr<Context> context,
+    const std::weak_ptr<Context>& context,
     const SurfaceProducerCreateInfoVK& create_info) {
   auto surface_producer =
       std::make_unique<SurfaceProducerVK>(context, create_info);
@@ -28,7 +29,7 @@ std::unique_ptr<SurfaceProducerVK> SurfaceProducerVK::Create(
 SurfaceProducerVK::SurfaceProducerVK(
     std::weak_ptr<Context> context,
     const SurfaceProducerCreateInfoVK& create_info)
-    : context_(context), create_info_(create_info) {}
+    : context_(std::move(context)), create_info_(create_info) {}
 
 SurfaceProducerVK::~SurfaceProducerVK() = default;
 

diff --git a/impeller/renderer/backend/vulkan/surface_producer_vk.h b/impeller/renderer/backend/vulkan/surface_producer_vk.h
@@ -33,7 +33,7 @@ class SurfaceSyncObjectsVK {
 class SurfaceProducerVK {
  public:
   static std::unique_ptr<SurfaceProducerVK> Create(
-      std::weak_ptr<Context> context,
+      const std::weak_ptr<Context>& context,
       const SurfaceProducerCreateInfoVK& create_info);
 
   SurfaceProducerVK(std::weak_ptr<Context> context,

diff --git a/impeller/renderer/backend/vulkan/surface_vk.cc b/impeller/renderer/backend/vulkan/surface_vk.cc
@@ -34,7 +34,7 @@ std::unique_ptr<SurfaceVK> SurfaceVK::WrapSwapchainImage(
               .frame_num = frame_num,
           },
   });
-  color0.texture = std::make_shared<TextureVK>(std::move(color0_tex), context,
+  color0.texture = std::make_shared<TextureVK>(color0_tex, context,
                                                std::move(color_texture_info));
   color0.clear_color = Color::DarkSlateGray();
   color0.load_action = LoadAction::kClear;
@@ -59,19 +59,18 @@ std::unique_ptr<SurfaceVK> SurfaceVK::WrapSwapchainImage(
           },
   });
   stencil0.texture = std::make_shared<TextureVK>(
-      std::move(stencil0_tex), context, std::move(stencil_texture_info));
+      stencil0_tex, context, std::move(stencil_texture_info));
   stencil0.load_action = LoadAction::kClear;
   stencil0.store_action = StoreAction::kDontCare;
 
   RenderTarget render_target_desc;
   render_target_desc.SetColorAttachment(color0, 0u);
 
-  return std::unique_ptr<SurfaceVK>(new SurfaceVK(std::move(render_target_desc),
-                                                  swapchain_image,
-                                                  std::move(swap_callback)));
+  return std::unique_ptr<SurfaceVK>(new SurfaceVK(
+      render_target_desc, swapchain_image, std::move(swap_callback)));
 }
 
-SurfaceVK::SurfaceVK(RenderTarget target,
+SurfaceVK::SurfaceVK(const RenderTarget& target,
                      SwapchainImageVK* swapchain_image,
                      SwapCallback swap_callback)
     : Surface(target) {

diff --git a/impeller/renderer/backend/vulkan/surface_vk.h b/impeller/renderer/backend/vulkan/surface_vk.h
@@ -23,7 +23,7 @@ class SurfaceVK final : public Surface {
       ContextVK* context,
       SwapCallback swap_callback);
 
-  SurfaceVK(RenderTarget target,
+  SurfaceVK(const RenderTarget& target,
             SwapchainImageVK* swapchain_image,
             SwapCallback swap_callback);
 

diff --git a/impeller/toolkit/egl/config.cc b/impeller/toolkit/egl/config.cc
@@ -10,7 +10,7 @@ namespace impeller {
 namespace egl {
 
 Config::Config(ConfigDescriptor descriptor, EGLConfig config)
-    : desc_(std::move(descriptor)), config_(config) {}
+    : desc_(descriptor), config_(config) {}
 
 Config::~Config() = default;
 

diff --git a/impeller/toolkit/egl/context.cc b/impeller/toolkit/egl/context.cc
@@ -72,7 +72,7 @@ bool Context::ClearCurrent() const {
 }
 
 std::optional<UniqueID> Context::AddLifecycleListener(
-    LifecycleListener listener) {
+    const LifecycleListener& listener) {
   if (!listener) {
     return std::nullopt;
   }

diff --git a/impeller/toolkit/egl/context.h b/impeller/toolkit/egl/context.h
@@ -35,7 +35,8 @@ class Context {
     kWillClearCurrent,
   };
   using LifecycleListener = std::function<void(LifecycleEvent)>;
-  std::optional<UniqueID> AddLifecycleListener(LifecycleListener listener);
+  std::optional<UniqueID> AddLifecycleListener(
+      const LifecycleListener& listener);
 
   bool RemoveLifecycleListener(UniqueID id);
 

diff --git a/impeller/toolkit/egl/display.cc b/impeller/toolkit/egl/display.cc
@@ -166,7 +166,7 @@ std::unique_ptr<Config> Display::ChooseConfig(ConfigDescriptor config) const {
     return nullptr;
   }
 
-  return std::make_unique<Config>(std::move(config), config_out);
+  return std::make_unique<Config>(config, config_out);
 }
 
 std::unique_ptr<Surface> Display::CreateWindowSurface(

diff --git a/lib/ui/ui_dart_state.cc b/lib/ui/ui_dart_state.cc
@@ -215,15 +215,16 @@ void UIDartState::LogMessage(const std::string& tag,
     // Fall back to previous behavior if unspecified.
 #if defined(FML_OS_ANDROID)
     __android_log_print(ANDROID_LOG_INFO, tag.c_str(), "%.*s",
-                        (int)message.size(), message.c_str());
+                        static_cast<int>(message.size()), message.c_str());
 #elif defined(FML_OS_IOS)
     std::stringstream stream;
     if (!tag.empty()) {
       stream << tag << ": ";
     }
     stream << message;
     std::string log = stream.str();
-    syslog(1 /* LOG_ALERT */, "%.*s", (int)log.size(), log.c_str());
+    syslog(1 /* LOG_ALERT */, "%.*s", static_cast<int>(log.size()),
+           log.c_str());
 #else
     if (!tag.empty()) {
       std::cout << tag << ": ";

diff --git a/shell/common/rasterizer.cc b/shell/common/rasterizer.cc
@@ -292,7 +292,7 @@ std::unique_ptr<Rasterizer::GpuImageResult> Rasterizer::MakeSkiaGpuImage(
 // I can't seem to get the GPU path working on it.
 // https://github.com/flutter/flutter/issues/108835
 #if FML_OS_LINUX
-  return MakeBitmapImage(std::move(display_list), image_info);
+  return MakeBitmapImage(display_list, image_info);
 #endif
 
   std::unique_ptr<SnapshotDelegate::GpuImageResult> result;

diff --git a/shell/common/shell_io_manager.cc b/shell/common/shell_io_manager.cc
@@ -14,7 +14,7 @@ namespace flutter {
 
 sk_sp<GrDirectContext> ShellIOManager::CreateCompatibleResourceLoadingContext(
     GrBackend backend,
-    sk_sp<const GrGLInterface> gl_interface) {
+    const sk_sp<const GrGLInterface>& gl_interface) {
 #if SK_GL
   if (backend != GrBackend::kOpenGL_GrBackend) {
     return nullptr;

diff --git a/shell/common/shell_io_manager.h b/shell/common/shell_io_manager.h
@@ -22,7 +22,7 @@ class ShellIOManager final : public IOManager {
   // they so desire.
   static sk_sp<GrDirectContext> CreateCompatibleResourceLoadingContext(
       GrBackend backend,
-      sk_sp<const GrGLInterface> gl_interface);
+      const sk_sp<const GrGLInterface>& gl_interface);
 
   ShellIOManager(
       sk_sp<GrDirectContext> resource_context,

diff --git a/shell/platform/android/android_context_gl_skia.cc b/shell/platform/android/android_context_gl_skia.cc
@@ -70,7 +70,7 @@ AndroidContextGLSkia::AndroidContextGLSkia(
     const TaskRunners& task_runners,
     uint8_t msaa_samples)
     : AndroidContext(AndroidRenderingAPI::kOpenGLES),
-      environment_(environment),
+      environment_(std::move(environment)),
       config_(nullptr),
       task_runners_(task_runners) {
   if (!environment_->IsValid()) {
@@ -146,7 +146,7 @@ AndroidContextGLSkia::~AndroidContextGLSkia() {
 }
 
 std::unique_ptr<AndroidEGLSurface> AndroidContextGLSkia::CreateOnscreenSurface(
-    fml::RefPtr<AndroidNativeWindow> window) const {
+    const fml::RefPtr<AndroidNativeWindow>& window) const {
   if (window->IsFakeWindow()) {
     return CreatePbufferSurface();
   } else {

diff --git a/shell/platform/android/android_context_gl_skia.h b/shell/platform/android/android_context_gl_skia.h
@@ -41,7 +41,7 @@ class AndroidContextGLSkia : public AndroidContext {
   /// @return     The window surface.
   ///
   std::unique_ptr<AndroidEGLSurface> CreateOnscreenSurface(
-      fml::RefPtr<AndroidNativeWindow> window) const;
+      const fml::RefPtr<AndroidNativeWindow>& window) const;
 
   //----------------------------------------------------------------------------
   /// @brief      Allocates an 1x1 pbuffer surface that is used for making the

diff --git a/shell/platform/android/android_external_texture_gl.cc b/shell/platform/android/android_external_texture_gl.cc
@@ -6,6 +6,8 @@
 
 #include <GLES/glext.h>
 
+#include <utility>
+
 #include "third_party/skia/include/core/SkAlphaType.h"
 #include "third_party/skia/include/core/SkColorSpace.h"
 #include "third_party/skia/include/core/SkColorType.h"
@@ -20,7 +22,7 @@ AndroidExternalTextureGL::AndroidExternalTextureGL(
     const fml::jni::ScopedJavaGlobalRef<jobject>& surface_texture,
     std::shared_ptr<PlatformViewAndroidJNI> jni_facade)
     : Texture(id),
-      jni_facade_(jni_facade),
+      jni_facade_(std::move(jni_facade)),
       surface_texture_(surface_texture),
       transform(SkMatrix::I()) {}
 

diff --git a/shell/platform/android/android_image_generator.cc b/shell/platform/android/android_image_generator.cc
@@ -5,6 +5,7 @@
 #include "flutter/shell/platform/android/android_image_generator.h"
 
 #include <memory>
+#include <utility>
 
 #include <android/bitmap.h>
 #include <android/hardware_buffer.h>
@@ -21,7 +22,7 @@ static jmethodID g_decode_image_method = nullptr;
 AndroidImageGenerator::~AndroidImageGenerator() = default;
 
 AndroidImageGenerator::AndroidImageGenerator(sk_sp<SkData> data)
-    : data_(data), image_info_(SkImageInfo::MakeUnknown(-1, -1)) {}
+    : data_(std::move(data)), image_info_(SkImageInfo::MakeUnknown(-1, -1)) {}
 
 const SkImageInfo& AndroidImageGenerator::GetInfo() {
   header_decoded_latch_.Wait();
@@ -173,7 +174,7 @@ bool AndroidImageGenerator::Register(JNIEnv* env) {
 
 std::shared_ptr<ImageGenerator> AndroidImageGenerator::MakeFromData(
     sk_sp<SkData> data,
-    fml::RefPtr<fml::TaskRunner> task_runner) {
+    const fml::RefPtr<fml::TaskRunner>& task_runner) {
   std::shared_ptr<AndroidImageGenerator> generator(
       new AndroidImageGenerator(std::move(data)));
 

diff --git a/shell/platform/android/android_image_generator.h b/shell/platform/android/android_image_generator.h
@@ -50,7 +50,7 @@ class AndroidImageGenerator : public ImageGenerator {
 
   static std::shared_ptr<ImageGenerator> MakeFromData(
       sk_sp<SkData> data,
-      fml::RefPtr<fml::TaskRunner> task_runner);
+      const fml::RefPtr<fml::TaskRunner>& task_runner);
 
   static void NativeImageHeaderCallback(JNIEnv* env,
                                         jclass jcaller,

diff --git a/shell/platform/android/android_shell_holder.cc b/shell/platform/android/android_shell_holder.cc
@@ -78,9 +78,9 @@ static PlatformData GetDefaultPlatformData() {
 }
 
 AndroidShellHolder::AndroidShellHolder(
-    flutter::Settings settings,
+    const flutter::Settings& settings,
     std::shared_ptr<PlatformViewAndroidJNI> jni_facade)
-    : settings_(std::move(settings)), jni_facade_(jni_facade) {
+    : settings_(settings), jni_facade_(jni_facade) {
   static size_t thread_host_count = 1;
   auto thread_label = std::to_string(thread_host_count++);
 
@@ -164,7 +164,7 @@ AndroidShellHolder::AndroidShellHolder(
 
     shell_->RegisterImageDecoder(
         [runner = task_runners.GetIOTaskRunner()](sk_sp<SkData> buffer) {
-          return AndroidImageGenerator::MakeFromData(buffer, runner);
+          return AndroidImageGenerator::MakeFromData(std::move(buffer), runner);
         },
         -1);
     FML_DLOG(INFO) << "Registered Android SDK image decoder (API level 28+)";
@@ -181,7 +181,7 @@ AndroidShellHolder::AndroidShellHolder(
     const std::shared_ptr<ThreadHost>& thread_host,
     std::unique_ptr<Shell> shell,
     const fml::WeakPtr<PlatformViewAndroid>& platform_view)
-    : settings_(std::move(settings)),
+    : settings_(settings),
       jni_facade_(jni_facade),
       platform_view_(platform_view),
       thread_host_(thread_host),
@@ -335,13 +335,12 @@ std::optional<RunConfiguration> AndroidShellHolder::BuildRunConfiguration(
 
   {
     if (!entrypoint.empty() && !libraryUrl.empty()) {
-      config.SetEntrypointAndLibrary(std::move(entrypoint),
-                                     std::move(libraryUrl));
+      config.SetEntrypointAndLibrary(entrypoint, libraryUrl);
     } else if (!entrypoint.empty()) {
-      config.SetEntrypoint(std::move(entrypoint));
+      config.SetEntrypoint(entrypoint);
     }
     if (!entrypoint_args.empty()) {
-      config.SetEntrypointArgs(std::move(entrypoint_args));
+      config.SetEntrypointArgs(entrypoint_args);
     }
   }
   return config;

diff --git a/shell/platform/android/android_shell_holder.h b/shell/platform/android/android_shell_holder.h
@@ -41,7 +41,7 @@ namespace flutter {
 ///
 class AndroidShellHolder {
  public:
-  AndroidShellHolder(flutter::Settings settings,
+  AndroidShellHolder(const flutter::Settings& settings,
                      std::shared_ptr<PlatformViewAndroidJNI> jni_facade);
 
   ~AndroidShellHolder();