Lint and fix bugprone-use-after-move violations (flutter#35978)

.clang-tidy

@@ -1,7 +1,8 @@
 # Prefix check with "-" to ignore.
-Checks: "google-*,\
+Checks: "bugprone-use-after-move,\
 clang-analyzer-*,\
 clang-diagnostic-*,\
+google-*,\
 modernize-use-default-member-init,\
 readability-identifier-naming,\
 -google-objc-global-variable-declaration,\
@@ -16,7 +17,8 @@ readability-identifier-naming,\
 # Add checks when all warnings are fixed
 # to prevent new warnings being introduced.
 # https://github.com/flutter/flutter/issues/93279
-WarningsAsErrors: "clang-analyzer-*,\
+WarningsAsErrors: "bugprone-use-after-move,\
+clang-analyzer-*,\
 readability-identifier-naming,\
 clang-diagnostic-*,\
 google-objc-*,\

display_list/display_list_builder.cc

@@ -1041,9 +1041,9 @@ void DisplayListBuilder::drawPicture(const sk_sp<SkPicture> picture,
                                      const SkMatrix* matrix,
                                      bool render_with_attributes) {
   matrix  //
-      ? Push<DrawSkPictureMatrixOp>(0, 1, std::move(picture), *matrix,
+      ? Push<DrawSkPictureMatrixOp>(0, 1, picture, *matrix,
                                     render_with_attributes)
-      : Push<DrawSkPictureOp>(0, 1, std::move(picture), render_with_attributes);
+      : Push<DrawSkPictureOp>(0, 1, picture, render_with_attributes);
   // The non-nested op count accumulated in the |Push| method will include
   // this call to |drawPicture| for non-nested op count metrics.
   // But, for nested op count metrics we want the |drawPicture| call itself
@@ -1056,7 +1056,7 @@ void DisplayListBuilder::drawPicture(const sk_sp<SkPicture> picture,
 }
 void DisplayListBuilder::drawDisplayList(
     const sk_sp<DisplayList> display_list) {
-  Push<DrawDisplayListOp>(0, 1, std::move(display_list));
+  Push<DrawDisplayListOp>(0, 1, display_list);
   // The non-nested op count accumulated in the |Push| method will include
   // this call to |drawDisplayList| for non-nested op count metrics.
   // But, for nested op count metrics we want the |drawDisplayList| call itself

fml/mapping_unittests.cc

@@ -26,7 +26,8 @@ TEST(MallocMapping, MoveConstructor) {
   MallocMapping moved = std::move(mapping);
 
   ASSERT_EQ(nullptr,
-            mapping.GetMapping());  // NOLINT(clang-analyzer-cplusplus.Move)
+            mapping.GetMapping());  // NOLINT(clang-analyzer-cplusplus.Move,
+                                    // bugprone-use-after-move)
   ASSERT_EQ(0u, mapping.GetSize());
   ASSERT_NE(nullptr, moved.GetMapping());
   ASSERT_EQ(length, moved.GetSize());

fml/memory/ref_counted_unittest.cc

@@ -229,7 +229,8 @@ TEST(RefCountedTest, NullAssignmentToNull) {
   EXPECT_TRUE(r1.get() == nullptr);
   // The clang linter flags the method called on the moved-from reference, but
   // this is testing the move implementation, so it is marked NOLINT.
-  EXPECT_TRUE(r2.get() == nullptr);  // NOLINT(clang-analyzer-cplusplus.Move)
+  EXPECT_TRUE(r2.get() == nullptr);  // NOLINT(clang-analyzer-cplusplus.Move,
+                                     // bugprone-use-after-move)
   EXPECT_FALSE(r1);
   EXPECT_FALSE(r2);
 
@@ -244,7 +245,7 @@ TEST(RefCountedTest, NullAssignmentToNull) {
   // No-op null assignment using "move" constructor.
   r1 = std::move(r3);
   EXPECT_TRUE(r1.get() == nullptr);
-  EXPECT_TRUE(r3.get() == nullptr);
+  EXPECT_TRUE(r3.get() == nullptr);  // NOLINT(bugprone-use-after-move)
   EXPECT_FALSE(r1);
   EXPECT_FALSE(r3);
 }
@@ -276,7 +277,8 @@ TEST(RefCountedTest, NonNullAssignmentToNull) {
     r2 = std::move(r1);
     // The clang linter flags the method called on the moved-from reference, but
     // this is testing the move implementation, so it is marked NOLINT.
-    EXPECT_TRUE(r1.get() == nullptr);  // NOLINT(clang-analyzer-cplusplus.Move)
+    EXPECT_TRUE(r1.get() == nullptr);  // NOLINT(clang-analyzer-cplusplus.Move,
+                                       // bugprone-use-after-move)
     EXPECT_EQ(created, r2.get());
     EXPECT_FALSE(r1);
     EXPECT_TRUE(r2);
@@ -306,7 +308,7 @@ TEST(RefCountedTest, NonNullAssignmentToNull) {
     RefPtr<MyClass> r2;
     // "Move" assignment (to null ref pointer).
     r2 = std::move(r1);
-    EXPECT_TRUE(r1.get() == nullptr);
+    EXPECT_TRUE(r1.get() == nullptr);  // NOLINT(bugprone-use-after-move)
     EXPECT_EQ(static_cast<MyClass*>(created), r2.get());
     EXPECT_FALSE(r1);
     EXPECT_TRUE(r2);
@@ -342,7 +344,8 @@ TEST(RefCountedTest, NullAssignmentToNonNull) {
   EXPECT_TRUE(r1.get() == nullptr);
   // The clang linter flags the method called on the moved-from reference, but
   // this is testing the move implementation, so it is marked NOLINT.
-  EXPECT_TRUE(r2.get() == nullptr);  // NOLINT(clang-analyzer-cplusplus.Move)
+  EXPECT_TRUE(r2.get() == nullptr);  // NOLINT(clang-analyzer-cplusplus.Move,
+                                     // bugprone-use-after-move)
   EXPECT_FALSE(r1);
   EXPECT_FALSE(r2);
   EXPECT_TRUE(was_destroyed);
@@ -363,7 +366,7 @@ TEST(RefCountedTest, NullAssignmentToNonNull) {
   // Null assignment (to non-null ref pointer) using "move" constructor.
   r1 = std::move(r3);
   EXPECT_TRUE(r1.get() == nullptr);
-  EXPECT_TRUE(r3.get() == nullptr);
+  EXPECT_TRUE(r3.get() == nullptr);  // NOLINT(bugprone-use-after-move)
   EXPECT_FALSE(r1);
   EXPECT_FALSE(r3);
   EXPECT_TRUE(was_destroyed);
@@ -397,7 +400,8 @@ TEST(RefCountedTest, NonNullAssignmentToNonNull) {
     r2 = std::move(r1);
     // The clang linter flags the method called on the moved-from reference, but
     // this is testing the move implementation, so it is marked NOLINT.
-    EXPECT_TRUE(r1.get() == nullptr);  // NOLINT(clang-analyzer-cplusplus.Move)
+    EXPECT_TRUE(r1.get() == nullptr);  // NOLINT(clang-analyzer-cplusplus.Move,
+                                       // bugprone-use-after-move)
     EXPECT_FALSE(r2.get() == nullptr);
     EXPECT_FALSE(r1);
     EXPECT_TRUE(r2);
@@ -428,7 +432,7 @@ TEST(RefCountedTest, NonNullAssignmentToNonNull) {
     RefPtr<MyClass> r2(MakeRefCounted<MyClass>(nullptr, &was_destroyed2));
     // Move assignment (to non-null ref pointer).
     r2 = std::move(r1);
-    EXPECT_TRUE(r1.get() == nullptr);
+    EXPECT_TRUE(r1.get() == nullptr);  // NOLINT(bugprone-use-after-move)
     EXPECT_FALSE(r2.get() == nullptr);
     EXPECT_FALSE(r1);
     EXPECT_TRUE(r2);

fml/memory/weak_ptr_unittest.cc

@@ -146,7 +146,9 @@ TEST(WeakPtrTest, UpcastMoveConstruction) {
   WeakPtrFactory<Derived> factory(&data);
   WeakPtr<Derived> ptr = factory.GetWeakPtr();
   WeakPtr<Base> ptr2(std::move(ptr));
-  EXPECT_EQ(nullptr, ptr.get());
+  // The clang linter flags the method called on the moved-from reference, but
+  // this is testing the move implementation, so it is marked NOLINT.
+  EXPECT_EQ(nullptr, ptr.get());  // NOLINT
   EXPECT_EQ(&data, ptr2.get());
 }
 
@@ -168,7 +170,9 @@ TEST(WeakPtrTest, UpcastMoveAssignment) {
   WeakPtr<Base> ptr2;
   EXPECT_EQ(nullptr, ptr2.get());
   ptr2 = std::move(ptr);
-  EXPECT_EQ(nullptr, ptr.get());
+  // The clang linter flags the method called on the moved-from reference, but
+  // this is testing the move implementation, so it is marked NOLINT.
+  EXPECT_EQ(nullptr, ptr.get());  // NOLINT
   EXPECT_EQ(&data, ptr2.get());
 }
 

fml/platform/darwin/cf_utils_unittests.mm

@@ -46,7 +46,7 @@
     auto string_source = string;
     ASSERT_TRUE(string_source);
     auto string_move = std::move(string_source);
-    ASSERT_FALSE(string_source);
+    ASSERT_FALSE(string_source);  // NOLINT(bugprone-use-after-move)
     ASSERT_EQ(ref_count + 1u, CFGetRetainCount(string));
     string_move.Reset();
     ASSERT_EQ(ref_count, CFGetRetainCount(string));
@@ -55,7 +55,7 @@
   // Move assign.
   {
     auto string_move_assign = std::move(string);
-    ASSERT_FALSE(string);
+    ASSERT_FALSE(string);  // NOLINT(bugprone-use-after-move)
     ASSERT_EQ(ref_count, CFGetRetainCount(string_move_assign));
   }
 }

impeller/aiks/canvas.cc

@@ -334,7 +334,7 @@ void Canvas::SaveLayer(Paint paint,
 void Canvas::DrawTextFrame(TextFrame text_frame, Point position, Paint paint) {
   auto lazy_glyph_atlas = GetCurrentPass().GetLazyGlyphAtlas();
 
-  lazy_glyph_atlas->AddTextFrame(std::move(text_frame));
+  lazy_glyph_atlas->AddTextFrame(text_frame);
 
   auto text_contents = std::make_shared<TextContents>();
   text_contents->SetTextFrame(std::move(text_frame));

impeller/archivist/archive_class_registration.cc

@@ -17,9 +17,9 @@ static constexpr const char* kArchivePrimaryKeyColumnName = "primary_key";
 ArchiveClassRegistration::ArchiveClassRegistration(ArchiveDatabase& database,
                                                    ArchiveDef definition)
     : database_(database), definition_(std::move(definition)) {
-  for (size_t i = 0; i < definition.members.size(); i++) {
+  for (size_t i = 0; i < definition_.members.size(); i++) {
     // The first index entry is the primary key. So add one to the index.
-    column_map_[definition.members[i]] = i + 1;
+    column_map_[definition_.members[i]] = i + 1;
   }
   is_valid_ = CreateTable();
 }

impeller/renderer/backend/gles/sampler_library_gles.cc

@@ -21,7 +21,7 @@ std::shared_ptr<const Sampler> SamplerLibraryGLES::GetSampler(
   if (found != samplers_.end()) {
     return found->second;
   }
-  return samplers_[std::move(descriptor)] =
+  return samplers_[descriptor] =
              std::shared_ptr<SamplerGLES>(new SamplerGLES(descriptor));
 }
 

impeller/typographer/text_run.cc

@@ -7,7 +7,7 @@
 namespace impeller {
 
 TextRun::TextRun(Font font) : font_(std::move(font)) {
-  if (!font.IsValid()) {
+  if (!font_.IsValid()) {
     return;
   }
   is_valid_ = true;

lib/ui/compositing/scene_builder_unittests.cc

@@ -82,8 +82,7 @@ TEST_F(ShellTest, SceneBuilderBuildAndSceneDisposeReleasesLayerStack) {
   AddNativeCallback("ValidateSceneHasNoLayers",
                     CREATE_NATIVE_ENTRY(validate_scene_has_no_layers));
 
-  std::unique_ptr<Shell> shell =
-      CreateShell(std::move(settings), std::move(task_runners));
+  std::unique_ptr<Shell> shell = CreateShell(settings, task_runners);
 
   ASSERT_TRUE(shell->IsSetup());
   auto configuration = RunConfiguration::InferFromSettings(settings);
@@ -142,8 +141,7 @@ TEST_F(ShellTest, EngineLayerDisposeReleasesReference) {
   AddNativeCallback("ValidateEngineLayerDispose",
                     CREATE_NATIVE_ENTRY(validate_engine_layer_dispose));
 
-  std::unique_ptr<Shell> shell =
-      CreateShell(std::move(settings), std::move(task_runners));
+  std::unique_ptr<Shell> shell = CreateShell(settings, task_runners);
 
   ASSERT_TRUE(shell->IsSetup());
   auto configuration = RunConfiguration::InferFromSettings(settings);

lib/ui/hooks_unittests.cc

@@ -35,8 +35,7 @@ TEST_F(HooksTest, HooksUnitTests) {
 
   auto message_latch = std::make_shared<fml::AutoResetWaitableEvent>();
 
-  std::unique_ptr<Shell> shell =
-      CreateShell(std::move(settings), std::move(task_runners));
+  std::unique_ptr<Shell> shell = CreateShell(settings, task_runners);
   ASSERT_TRUE(shell->IsSetup());
 
   auto call_hook = [](Dart_NativeArguments args) {

lib/ui/painting/image_dispose_unittests.cc

@@ -69,7 +69,7 @@ TEST_F(ImageDisposeTest, ImageReleasedAfterFrameAndDisposePictureAndLayer) {
                     CREATE_NATIVE_ENTRY(native_capture_image_and_picture));
   AddNativeCallback("Finish", CREATE_NATIVE_ENTRY(native_finish));
 
-  std::unique_ptr<Shell> shell = CreateShell(std::move(settings), task_runners);
+  std::unique_ptr<Shell> shell = CreateShell(settings, task_runners);
 
   ASSERT_TRUE(shell->IsSetup());
 

lib/ui/painting/image_encoding_unittests.cc

@@ -88,8 +88,7 @@ TEST_F(ShellTest, EncodeImageGivesExternalTypedData) {
   AddNativeCallback("ValidateExternal",
                     CREATE_NATIVE_ENTRY(nativeValidateExternal));
 
-  std::unique_ptr<Shell> shell =
-      CreateShell(std::move(settings), std::move(task_runners));
+  std::unique_ptr<Shell> shell = CreateShell(settings, task_runners);
 
   ASSERT_TRUE(shell->IsSetup());
   auto configuration = RunConfiguration::InferFromSettings(settings);
@@ -153,8 +152,7 @@ TEST_F(ShellTest, EncodeImageAccessesSyncSwitch) {
 
   AddNativeCallback("EncodeImage", CREATE_NATIVE_ENTRY(native_encode_image));
 
-  std::unique_ptr<Shell> shell =
-      CreateShell(std::move(settings), std::move(task_runners));
+  std::unique_ptr<Shell> shell = CreateShell(settings, task_runners);
 
   ASSERT_TRUE(shell->IsSetup());
   auto configuration = RunConfiguration::InferFromSettings(settings);

lib/ui/painting/path_unittests.cc

@@ -50,8 +50,7 @@ TEST_F(ShellTest, PathVolatilityOldPathsBecomeNonVolatile) {
 
   AddNativeCallback("ValidatePath", CREATE_NATIVE_ENTRY(native_validate_path));
 
-  std::unique_ptr<Shell> shell =
-      CreateShell(std::move(settings), std::move(task_runners));
+  std::unique_ptr<Shell> shell = CreateShell(settings, task_runners);
 
   ASSERT_TRUE(shell->IsSetup());
   auto configuration = RunConfiguration::InferFromSettings(settings);
@@ -109,8 +108,7 @@ TEST_F(ShellTest, PathVolatilityGCRemovesPathFromTracker) {
 
   AddNativeCallback("ValidatePath", CREATE_NATIVE_ENTRY(native_validate_path));
 
-  std::unique_ptr<Shell> shell =
-      CreateShell(std::move(settings), std::move(task_runners));
+  std::unique_ptr<Shell> shell = CreateShell(settings, task_runners);
 
   ASSERT_TRUE(shell->IsSetup());
   auto configuration = RunConfiguration::InferFromSettings(settings);
@@ -166,8 +164,7 @@ TEST_F(ShellTest, DeterministicRenderingDisablesPathVolatility) {
 
   AddNativeCallback("ValidatePath", CREATE_NATIVE_ENTRY(native_validate_path));
 
-  std::unique_ptr<Shell> shell =
-      CreateShell(std::move(settings), std::move(task_runners));
+  std::unique_ptr<Shell> shell = CreateShell(settings, task_runners);
 
   ASSERT_TRUE(shell->IsSetup());
   auto configuration = RunConfiguration::InferFromSettings(settings);

lib/ui/painting/single_frame_codec_unittests.cc

@@ -41,8 +41,7 @@ TEST_F(ShellTest, SingleFrameCodecAccuratelyReportsSize) {
   AddNativeCallback("ValidateCodec", CREATE_NATIVE_ENTRY(validate_codec));
   AddNativeCallback("Finish", CREATE_NATIVE_ENTRY(finish));
 
-  std::unique_ptr<Shell> shell =
-      CreateShell(std::move(settings), std::move(task_runners));
+  std::unique_ptr<Shell> shell = CreateShell(settings, task_runners);
 
   ASSERT_TRUE(shell->IsSetup());
   auto configuration = RunConfiguration::InferFromSettings(settings);

lib/ui/semantics/semantics_update_builder_unittests.cc

@@ -74,8 +74,7 @@ TEST_F(SemanticsUpdateBuilderTest, CanHandleAttributedStrings) {
   AddNativeCallback("SemanticsUpdate",
                     CREATE_NATIVE_ENTRY(nativeSemanticsUpdate));
 
-  std::unique_ptr<Shell> shell =
-      CreateShell(std::move(settings), std::move(task_runners));
+  std::unique_ptr<Shell> shell = CreateShell(settings, task_runners);
 
   ASSERT_TRUE(shell->IsSetup());
   auto configuration = RunConfiguration::InferFromSettings(settings);

lib/ui/window/platform_configuration_unittests.cc

@@ -49,8 +49,7 @@ TEST_F(ShellTest, PlatformConfigurationInitialization) {
   AddNativeCallback("ValidateConfiguration",
                     CREATE_NATIVE_ENTRY(nativeValidateConfiguration));
 
-  std::unique_ptr<Shell> shell =
-      CreateShell(std::move(settings), std::move(task_runners));
+  std::unique_ptr<Shell> shell = CreateShell(settings, task_runners);
 
   ASSERT_TRUE(shell->IsSetup());
   auto run_configuration = RunConfiguration::InferFromSettings(settings);
@@ -100,8 +99,7 @@ TEST_F(ShellTest, PlatformConfigurationWindowMetricsUpdate) {
   AddNativeCallback("ValidateConfiguration",
                     CREATE_NATIVE_ENTRY(nativeValidateConfiguration));
 
-  std::unique_ptr<Shell> shell =
-      CreateShell(std::move(settings), std::move(task_runners));
+  std::unique_ptr<Shell> shell = CreateShell(settings, task_runners);
 
   ASSERT_TRUE(shell->IsSetup());
   auto run_configuration = RunConfiguration::InferFromSettings(settings);
@@ -139,8 +137,7 @@ TEST_F(ShellTest, PlatformConfigurationOnErrorHandlesError) {
                            CreateNewThread()        // io
   );
 
-  std::unique_ptr<Shell> shell =
-      CreateShell(std::move(settings), std::move(task_runners));
+  std::unique_ptr<Shell> shell = CreateShell(settings, task_runners);
 
   ASSERT_TRUE(shell->IsSetup());
   auto run_configuration = RunConfiguration::InferFromSettings(settings);
@@ -190,8 +187,7 @@ TEST_F(ShellTest, PlatformConfigurationOnErrorDoesNotHandleError) {
                            CreateNewThread()        // io
   );
 
-  std::unique_ptr<Shell> shell =
-      CreateShell(std::move(settings), std::move(task_runners));
+  std::unique_ptr<Shell> shell = CreateShell(settings, task_runners);
 
   ASSERT_TRUE(shell->IsSetup());
   auto run_configuration = RunConfiguration::InferFromSettings(settings);
@@ -242,8 +238,7 @@ TEST_F(ShellTest, PlatformConfigurationOnErrorThrows) {
                            CreateNewThread()        // io
   );
 
-  std::unique_ptr<Shell> shell =
-      CreateShell(std::move(settings), std::move(task_runners));
+  std::unique_ptr<Shell> shell = CreateShell(settings, task_runners);
 
   ASSERT_TRUE(shell->IsSetup());
   auto run_configuration = RunConfiguration::InferFromSettings(settings);
@@ -291,8 +286,7 @@ TEST_F(ShellTest, PlatformConfigurationSetDartPerformanceMode) {
                            CreateNewThread()        // io
   );
 
-  std::unique_ptr<Shell> shell =
-      CreateShell(std::move(settings), std::move(task_runners));
+  std::unique_ptr<Shell> shell = CreateShell(settings, task_runners);
 
   ASSERT_TRUE(shell->IsSetup());
   auto run_configuration = RunConfiguration::InferFromSettings(settings);

lib/ui/window/platform_message_response_dart_port_unittests.cc

@@ -53,8 +53,7 @@ TEST_F(ShellTest, PlatformMessageResponseDartPort) {
 
   Settings settings = CreateSettingsForFixture();
 
-  std::unique_ptr<Shell> shell =
-      CreateShell(std::move(settings), std::move(task_runners));
+  std::unique_ptr<Shell> shell = CreateShell(settings, task_runners);
 
   ASSERT_TRUE(shell->IsSetup());
   auto configuration = RunConfiguration::InferFromSettings(settings);

lib/ui/window/platform_message_response_dart_unittests.cc

@@ -52,8 +52,7 @@ TEST_F(ShellTest, PlatformMessageResponseDart) {
 
   Settings settings = CreateSettingsForFixture();
 
-  std::unique_ptr<Shell> shell =
-      CreateShell(std::move(settings), std::move(task_runners));
+  std::unique_ptr<Shell> shell = CreateShell(settings, task_runners);
 
   ASSERT_TRUE(shell->IsSetup());
   auto configuration = RunConfiguration::InferFromSettings(settings);