diff --git a/.clang-tidy b/.clang-tidy index 60dc76bf226fc..9103fd70bd432 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -1,4 +1,6 @@ # Prefix check with "-" to ignore. +# Note: Some of the checks here are used as errors selectively, see +# //ci/lint.sh Checks: "bugprone-use-after-move,\ clang-analyzer-*,\ clang-diagnostic-*,\ @@ -19,6 +21,8 @@ performance-unnecessary-value-param" # Add checks when all warnings are fixed # to prevent new warnings being introduced. # https://github.com/flutter/flutter/issues/93279 +# Note: There are platform specific warnings as errors in +# //ci/lint.sh WarningsAsErrors: "bugprone-use-after-move,\ clang-analyzer-*,\ readability-identifier-naming,\ diff --git a/ci/lint.sh b/ci/lint.sh index d42defb51f990..4e27f98769048 100755 --- a/ci/lint.sh +++ b/ci/lint.sh @@ -32,6 +32,9 @@ SRC_DIR="$(cd "$SCRIPT_DIR/../.."; pwd -P)" FLUTTER_DIR="$(cd "$SCRIPT_DIR/.."; pwd -P)" 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" COMPILE_COMMANDS="$SRC_DIR/out/host_debug/compile_commands.json" if [ ! -f "$COMPILE_COMMANDS" ]; then @@ -43,6 +46,7 @@ cd "$SCRIPT_DIR" --disable-dart-dev \ "$SRC_DIR/flutter/tools/clang_tidy/bin/main.dart" \ --src-dir="$SRC_DIR" \ + --mac-host-warnings-as-errors="$MAC_HOST_WARNINGS_AS_ERRORS" \ "$@" cd "$FLUTTER_DIR" diff --git a/impeller/renderer/render_target.cc b/impeller/renderer/render_target.cc index 5d1e85b974a17..ef196a0eec740 100644 --- a/impeller/renderer/render_target.cc +++ b/impeller/renderer/render_target.cc @@ -179,7 +179,7 @@ const std::optional& RenderTarget::GetStencilAttachment() RenderTarget RenderTarget::CreateOffscreen(const Context& context, ISize size, - std::string label, + const std::string& label, StorageMode color_storage_mode, LoadAction color_load_action, StoreAction color_store_action, @@ -230,7 +230,7 @@ RenderTarget RenderTarget::CreateOffscreen(const Context& context, stencil0.texture->SetLabel(SPrintF("%s Stencil Texture", label.c_str())); RenderTarget target; - target.SetColorAttachment(std::move(color0), 0u); + target.SetColorAttachment(color0, 0u); target.SetStencilAttachment(std::move(stencil0)); return target; @@ -239,7 +239,7 @@ RenderTarget RenderTarget::CreateOffscreen(const Context& context, RenderTarget RenderTarget::CreateOffscreenMSAA( const Context& context, ISize size, - std::string label, + const std::string& label, StorageMode color_storage_mode, StorageMode color_resolve_storage_mode, LoadAction color_load_action, @@ -322,7 +322,7 @@ RenderTarget RenderTarget::CreateOffscreenMSAA( stencil0.texture->SetLabel(SPrintF("%s Stencil Texture", label.c_str())); RenderTarget target; - target.SetColorAttachment(std::move(color0), 0u); + target.SetColorAttachment(color0, 0u); target.SetStencilAttachment(std::move(stencil0)); return target; diff --git a/impeller/renderer/render_target.h b/impeller/renderer/render_target.h index c47435bb146c1..802671afa8ad4 100644 --- a/impeller/renderer/render_target.h +++ b/impeller/renderer/render_target.h @@ -22,7 +22,7 @@ class RenderTarget { static RenderTarget CreateOffscreen( const Context& context, ISize size, - std::string label = "Offscreen", + const std::string& label = "Offscreen", StorageMode color_storage_mode = StorageMode::kDevicePrivate, LoadAction color_load_action = LoadAction::kClear, StoreAction color_store_action = StoreAction::kStore, @@ -33,7 +33,7 @@ class RenderTarget { static RenderTarget CreateOffscreenMSAA( const Context& context, ISize size, - std::string label = "Offscreen MSAA", + const std::string& label = "Offscreen MSAA", StorageMode color_storage_mode = StorageMode::kDeviceTransient, StorageMode color_resolve_storage_mode = StorageMode::kDevicePrivate, LoadAction color_load_action = LoadAction::kClear, diff --git a/impeller/typographer/backends/skia/text_render_context_skia.cc b/impeller/typographer/backends/skia/text_render_context_skia.cc index 96ef3f6851839..c5336f5ddfb5e 100644 --- a/impeller/typographer/backends/skia/text_render_context_skia.cc +++ b/impeller/typographer/backends/skia/text_render_context_skia.cc @@ -47,10 +47,10 @@ static FontGlyphPair::Set CollectUniqueFontGlyphPairsSet( static FontGlyphPair::Vector CollectUniqueFontGlyphPairs( GlyphAtlas::Type type, - TextRenderContext::FrameIterator frame_iterator) { + const TextRenderContext::FrameIterator& frame_iterator) { TRACE_EVENT0("impeller", __FUNCTION__); FontGlyphPair::Vector vector; - auto set = CollectUniqueFontGlyphPairsSet(type, std::move(frame_iterator)); + auto set = CollectUniqueFontGlyphPairsSet(type, frame_iterator); vector.reserve(set.size()); for (const auto& item : set) { vector.emplace_back(item); diff --git a/impeller/typographer/typographer_unittests.cc b/impeller/typographer/typographer_unittests.cc index 3802043307901..d51d46feb50d3 100644 --- a/impeller/typographer/typographer_unittests.cc +++ b/impeller/typographer/typographer_unittests.cc @@ -80,7 +80,7 @@ TEST_P(TypographerTest, LazyAtlasTracksColor) { LazyGlyphAtlas lazy_atlas; ASSERT_FALSE(lazy_atlas.HasColor()); - lazy_atlas.AddTextFrame(std::move(frame)); + lazy_atlas.AddTextFrame(frame); ASSERT_FALSE(lazy_atlas.HasColor()); @@ -88,7 +88,7 @@ TEST_P(TypographerTest, LazyAtlasTracksColor) { ASSERT_TRUE(frame.HasColor()); - lazy_atlas.AddTextFrame(std::move(frame)); + lazy_atlas.AddTextFrame(frame); ASSERT_TRUE(lazy_atlas.HasColor()); } diff --git a/lib/ui/compositing/scene.cc b/lib/ui/compositing/scene.cc index 5306d6f795f85..c5070c6376091 100644 --- a/lib/ui/compositing/scene.cc +++ b/lib/ui/compositing/scene.cc @@ -110,7 +110,7 @@ static sk_sp CreateDeferredImage( width, height, kRGBA_8888_SkColorType, kPremul_SkAlphaType); return DlDeferredImageGPUSkia::MakeFromLayerTree( image_info, std::move(layer_tree), std::move(snapshot_delegate), - std::move(raster_task_runner), std::move(unref_queue)); + raster_task_runner, std::move(unref_queue)); } void Scene::RasterizeToImage(uint32_t width, diff --git a/lib/ui/painting/image_encoding.cc b/lib/ui/painting/image_encoding.cc index 1328d3530bb91..7f3e9d56c96d6 100644 --- a/lib/ui/painting/image_encoding.cc +++ b/lib/ui/painting/image_encoding.cc @@ -275,9 +275,9 @@ Dart_Handle EncodeImage(CanvasImage* canvas_image, snapshot_delegate = UIDartState::Current()->GetSnapshotDelegate()]() mutable { EncodeImageAndInvokeDataCallback( - std::move(image), std::move(callback), image_format, ui_task_runner, - std::move(raster_task_runner), std::move(io_task_runner), - io_manager->GetResourceContext(), std::move(snapshot_delegate), + image, std::move(callback), image_format, ui_task_runner, + raster_task_runner, io_task_runner, + io_manager->GetResourceContext(), snapshot_delegate, io_manager->GetIsGpuDisabledSyncSwitch()); })); diff --git a/lib/ui/painting/immutable_buffer.cc b/lib/ui/painting/immutable_buffer.cc index ee367305135fc..0d86b07c24e4a 100644 --- a/lib/ui/painting/immutable_buffer.cc +++ b/lib/ui/painting/immutable_buffer.cc @@ -139,8 +139,9 @@ Dart_Handle ImmutableBuffer::initFromFile(Dart_Handle raw_buffer_handle, sk_data = MakeSkDataWithCopy(bytes, buffer_size); } ui_task_runner->PostTask( - [sk_data = std::move(sk_data), ui_task = std::move(ui_task), - buffer_size]() { ui_task(sk_data, buffer_size); }); + [sk_data = std::move(sk_data), ui_task = ui_task, buffer_size]() { + ui_task(sk_data, buffer_size); + }); }); return Dart_Null(); } diff --git a/lib/ui/painting/multi_frame_codec.cc b/lib/ui/painting/multi_frame_codec.cc index 1b84a36f9f992..b6702aad35d72 100644 --- a/lib/ui/painting/multi_frame_codec.cc +++ b/lib/ui/painting/multi_frame_codec.cc @@ -184,7 +184,7 @@ void MultiFrameCodec::State::GetNextFrameAndInvokeCallback( int duration = 0; sk_sp dlImage = GetNextFrameImage(std::move(resourceContext), gpu_disable_sync_switch, - impeller_context, unref_queue); + std::move(impeller_context), std::move(unref_queue)); if (dlImage) { image = CanvasImage::Create(); image->set_image(dlImage); diff --git a/lib/ui/ui_dart_state.cc b/lib/ui/ui_dart_state.cc index e4b84f7e5a58a..c478c760aac74 100644 --- a/lib/ui/ui_dart_state.cc +++ b/lib/ui/ui_dart_state.cc @@ -49,7 +49,7 @@ UIDartState::Context::Context( advisory_script_uri(std::move(advisory_script_uri)), advisory_script_entrypoint(std::move(advisory_script_entrypoint)), volatile_path_tracker(std::move(volatile_path_tracker)), - concurrent_task_runner(concurrent_task_runner), + concurrent_task_runner(std::move(concurrent_task_runner)), enable_impeller(enable_impeller) {} UIDartState::UIDartState( diff --git a/shell/platform/embedder/tests/embedder_test_compositor.cc b/shell/platform/embedder/tests/embedder_test_compositor.cc index 23afb36f01e72..4bbcf2ac83cac 100644 --- a/shell/platform/embedder/tests/embedder_test_compositor.cc +++ b/shell/platform/embedder/tests/embedder_test_compositor.cc @@ -4,6 +4,8 @@ #include "flutter/shell/platform/embedder/tests/embedder_test_compositor.h" +#include + #include "flutter/fml/logging.h" #include "flutter/shell/platform/embedder/tests/embedder_assertions.h" #include "third_party/skia/include/core/SkSurface.h" @@ -13,7 +15,7 @@ namespace testing { EmbedderTestCompositor::EmbedderTestCompositor(SkISize surface_size, sk_sp context) - : surface_size_(surface_size), context_(context) { + : surface_size_(surface_size), context_(std::move(context)) { FML_CHECK(!surface_size_.isEmpty()) << "Surface size must not be empty"; } @@ -118,16 +120,17 @@ size_t EmbedderTestCompositor::GetBackingStoresCollectedCount() const { } void EmbedderTestCompositor::AddOnCreateRenderTargetCallback( - fml::closure callback) { + const fml::closure& callback) { on_create_render_target_callbacks_.push_back(callback); } void EmbedderTestCompositor::AddOnCollectRenderTargetCallback( - fml::closure callback) { + const fml::closure& callback) { on_collect_render_target_callbacks_.push_back(callback); } -void EmbedderTestCompositor::AddOnPresentCallback(fml::closure callback) { +void EmbedderTestCompositor::AddOnPresentCallback( + const fml::closure& callback) { on_present_callbacks_.push_back(callback); } diff --git a/shell/platform/embedder/tests/embedder_test_compositor.h b/shell/platform/embedder/tests/embedder_test_compositor.h index 26d96de6cda81..2578445c7ba6d 100644 --- a/shell/platform/embedder/tests/embedder_test_compositor.h +++ b/shell/platform/embedder/tests/embedder_test_compositor.h @@ -64,11 +64,11 @@ class EmbedderTestCompositor { size_t GetBackingStoresCollectedCount() const; - void AddOnCreateRenderTargetCallback(fml::closure callback); + void AddOnCreateRenderTargetCallback(const fml::closure& callback); - void AddOnCollectRenderTargetCallback(fml::closure callback); + void AddOnCollectRenderTargetCallback(const fml::closure& callback); - void AddOnPresentCallback(fml::closure callback); + void AddOnPresentCallback(const fml::closure& callback); sk_sp GetGrContext(); diff --git a/shell/platform/embedder/tests/embedder_test_compositor_gl.cc b/shell/platform/embedder/tests/embedder_test_compositor_gl.cc index fbc6c340d32f8..35cdf42828a63 100644 --- a/shell/platform/embedder/tests/embedder_test_compositor_gl.cc +++ b/shell/platform/embedder/tests/embedder_test_compositor_gl.cc @@ -4,6 +4,8 @@ #include "flutter/shell/platform/embedder/tests/embedder_test_compositor_gl.h" +#include + #include "flutter/fml/logging.h" #include "flutter/shell/platform/embedder/tests/embedder_assertions.h" #include "third_party/skia/include/core/SkSurface.h" @@ -14,7 +16,7 @@ namespace testing { EmbedderTestCompositorGL::EmbedderTestCompositorGL( SkISize surface_size, sk_sp context) - : EmbedderTestCompositor(surface_size, context) {} + : EmbedderTestCompositor(surface_size, std::move(context)) {} EmbedderTestCompositorGL::~EmbedderTestCompositorGL() = default; diff --git a/shell/platform/embedder/tests/embedder_test_compositor_metal.cc b/shell/platform/embedder/tests/embedder_test_compositor_metal.cc index bb07d3256b4a0..f92b7cc21e4d0 100644 --- a/shell/platform/embedder/tests/embedder_test_compositor_metal.cc +++ b/shell/platform/embedder/tests/embedder_test_compositor_metal.cc @@ -4,6 +4,8 @@ #include "flutter/shell/platform/embedder/tests/embedder_test_compositor_metal.h" +#include + #include "flutter/fml/logging.h" #include "flutter/shell/platform/embedder/tests/embedder_assertions.h" #include "third_party/skia/include/core/SkSurface.h" @@ -14,7 +16,7 @@ namespace testing { EmbedderTestCompositorMetal::EmbedderTestCompositorMetal( SkISize surface_size, sk_sp context) - : EmbedderTestCompositor(surface_size, context) {} + : EmbedderTestCompositor(surface_size, std::move(context)) {} EmbedderTestCompositorMetal::~EmbedderTestCompositorMetal() = default; diff --git a/shell/platform/embedder/tests/embedder_test_compositor_vulkan.cc b/shell/platform/embedder/tests/embedder_test_compositor_vulkan.cc index a932260513b98..c0df97a5f606c 100644 --- a/shell/platform/embedder/tests/embedder_test_compositor_vulkan.cc +++ b/shell/platform/embedder/tests/embedder_test_compositor_vulkan.cc @@ -4,6 +4,8 @@ #include "flutter/shell/platform/embedder/tests/embedder_test_compositor_vulkan.h" +#include + #include "flutter/fml/logging.h" #include "flutter/shell/platform/embedder/tests/embedder_assertions.h" #include "flutter/shell/platform/embedder/tests/embedder_test_backingstore_producer.h" @@ -15,7 +17,7 @@ namespace testing { EmbedderTestCompositorVulkan::EmbedderTestCompositorVulkan( SkISize surface_size, sk_sp context) - : EmbedderTestCompositor(surface_size, context) {} + : EmbedderTestCompositor(surface_size, std::move(context)) {} EmbedderTestCompositorVulkan::~EmbedderTestCompositorVulkan() = default; diff --git a/shell/platform/embedder/tests/embedder_test_context.cc b/shell/platform/embedder/tests/embedder_test_context.cc index 88f833a9fdfce..261eb0f6b980f 100644 --- a/shell/platform/embedder/tests/embedder_test_context.cc +++ b/shell/platform/embedder/tests/embedder_test_context.cc @@ -4,6 +4,8 @@ #include "flutter/shell/platform/embedder/tests/embedder_test_context.h" +#include + #include "flutter/fml/make_copyable.h" #include "flutter/fml/paths.h" #include "flutter/runtime/dart_vm.h" @@ -94,7 +96,8 @@ void EmbedderTestContext::SetRootSurfaceTransformation(SkMatrix matrix) { root_surface_transformation_ = matrix; } -void EmbedderTestContext::AddIsolateCreateCallback(fml::closure closure) { +void EmbedderTestContext::AddIsolateCreateCallback( + const fml::closure& closure) { if (closure) { isolate_create_callbacks_.push_back(closure); } @@ -226,7 +229,7 @@ void EmbedderTestContext::FireRootSurfacePresentCallbackIfPresent( void EmbedderTestContext::SetVsyncCallback( std::function callback) { - vsync_callback_ = callback; + vsync_callback_ = std::move(callback); } void EmbedderTestContext::RunVsyncCallback(intptr_t baton) { diff --git a/shell/platform/embedder/tests/embedder_test_context.h b/shell/platform/embedder/tests/embedder_test_context.h index ade3333d95fc3..8577abbdbb301 100644 --- a/shell/platform/embedder/tests/embedder_test_context.h +++ b/shell/platform/embedder/tests/embedder_test_context.h @@ -67,7 +67,7 @@ class EmbedderTestContext { void SetRootSurfaceTransformation(SkMatrix matrix); - void AddIsolateCreateCallback(fml::closure closure); + void AddIsolateCreateCallback(const fml::closure& closure); void AddNativeCallback(const char* name, Dart_NativeFunction function); diff --git a/shell/platform/embedder/tests/embedder_test_context_gl.cc b/shell/platform/embedder/tests/embedder_test_context_gl.cc index 0215999814a0d..ed3dabcfe8c71 100644 --- a/shell/platform/embedder/tests/embedder_test_context_gl.cc +++ b/shell/platform/embedder/tests/embedder_test_context_gl.cc @@ -4,6 +4,8 @@ #include "flutter/shell/platform/embedder/tests/embedder_test_context_gl.h" +#include + #include "flutter/fml/make_copyable.h" #include "flutter/fml/paths.h" #include "flutter/runtime/dart_vm.h" @@ -18,7 +20,7 @@ namespace flutter { namespace testing { EmbedderTestContextGL::EmbedderTestContextGL(std::string assets_path) - : EmbedderTestContext(assets_path) {} + : EmbedderTestContext(std::move(assets_path)) {} EmbedderTestContextGL::~EmbedderTestContextGL() { SetGLGetFBOCallback(nullptr); @@ -61,18 +63,18 @@ bool EmbedderTestContextGL::GLPresent(FlutterPresentInfo present_info) { void EmbedderTestContextGL::SetGLGetFBOCallback(GLGetFBOCallback callback) { std::scoped_lock lock(gl_callback_mutex_); - gl_get_fbo_callback_ = callback; + gl_get_fbo_callback_ = std::move(callback); } void EmbedderTestContextGL::SetGLPopulateExistingDamageCallback( GLPopulateExistingDamageCallback callback) { std::scoped_lock lock(gl_callback_mutex_); - gl_populate_existing_damage_callback_ = callback; + gl_populate_existing_damage_callback_ = std::move(callback); } void EmbedderTestContextGL::SetGLPresentCallback(GLPresentCallback callback) { std::scoped_lock lock(gl_callback_mutex_); - gl_present_callback_ = callback; + gl_present_callback_ = std::move(callback); } uint32_t EmbedderTestContextGL::GLGetFramebuffer(FlutterFrameInfo frame_info) { diff --git a/shell/platform/embedder/tests/embedder_test_context_software.cc b/shell/platform/embedder/tests/embedder_test_context_software.cc index 1bb3bf5c8aea7..cddeed7656b25 100644 --- a/shell/platform/embedder/tests/embedder_test_context_software.cc +++ b/shell/platform/embedder/tests/embedder_test_context_software.cc @@ -4,6 +4,8 @@ #include "flutter/shell/platform/embedder/tests/embedder_test_context_software.h" +#include + #include "flutter/fml/make_copyable.h" #include "flutter/fml/paths.h" #include "flutter/runtime/dart_vm.h" @@ -18,11 +20,11 @@ namespace testing { EmbedderTestContextSoftware::EmbedderTestContextSoftware( std::string assets_path) - : EmbedderTestContext(assets_path) {} + : EmbedderTestContext(std::move(assets_path)) {} EmbedderTestContextSoftware::~EmbedderTestContextSoftware() = default; -bool EmbedderTestContextSoftware::Present(sk_sp image) { +bool EmbedderTestContextSoftware::Present(const sk_sp& image) { software_surface_present_count_++; FireRootSurfacePresentCallbackIfPresent([image] { return image; }); diff --git a/shell/platform/embedder/tests/embedder_test_context_software.h b/shell/platform/embedder/tests/embedder_test_context_software.h index f8690484d0119..d4708e91825be 100644 --- a/shell/platform/embedder/tests/embedder_test_context_software.h +++ b/shell/platform/embedder/tests/embedder_test_context_software.h @@ -21,7 +21,7 @@ class EmbedderTestContextSoftware : public EmbedderTestContext { // |EmbedderTestContext| EmbedderTestContextType GetContextType() const override; - bool Present(sk_sp image); + bool Present(const sk_sp& image); protected: virtual void SetupCompositor() override; diff --git a/shell/platform/embedder/tests/embedder_unittests.cc b/shell/platform/embedder/tests/embedder_unittests.cc index d0e336b98b5b4..74f6c4a563731 100644 --- a/shell/platform/embedder/tests/embedder_unittests.cc +++ b/shell/platform/embedder/tests/embedder_unittests.cc @@ -5,6 +5,7 @@ #define FML_USED_ON_EMBEDDER #include +#include #include #include "embedder.h" @@ -1580,7 +1581,7 @@ static void expectSoftwareRenderingOutputMatches( builder.SetSoftwareRendererConfig(); builder.SetCompositor(); - builder.SetDartEntrypoint(entrypoint); + builder.SetDartEntrypoint(std::move(entrypoint)); builder.SetRenderTargetType( EmbedderTestBackingStoreProducer::RenderTargetType::kSoftwareBuffer2, pixfmt); @@ -1595,7 +1596,9 @@ static void expectSoftwareRenderingOutputMatches( ASSERT_EQ(layers[0]->backing_store->type, kFlutterBackingStoreTypeSoftware2); matches = SurfacePixelDataMatchesBytes( - (SkSurface*)layers[0]->backing_store->software2.user_data, bytes); + static_cast( + layers[0]->backing_store->software2.user_data), + bytes); latch.Signal(); }); @@ -1622,7 +1625,8 @@ static void expectSoftwareRenderingOutputMatches( T pixelvalue) { uint8_t* bytes = reinterpret_cast(&pixelvalue); return expectSoftwareRenderingOutputMatches( - test, entrypoint, pixfmt, std::vector(bytes, bytes + sizeof(T))); + test, std::move(entrypoint), pixfmt, + std::vector(bytes, bytes + sizeof(T))); } #define SW_PIXFMT_TEST_F(dart_entrypoint, pixfmt, matcher) \ diff --git a/shell/platform/embedder/tests/embedder_unittests_metal.mm b/shell/platform/embedder/tests/embedder_unittests_metal.mm index fd221fc736609..296edf7b5723e 100644 --- a/shell/platform/embedder/tests/embedder_unittests_metal.mm +++ b/shell/platform/embedder/tests/embedder_unittests_metal.mm @@ -53,7 +53,7 @@ // ASSERT_TRUE(ImageMatchesFixture("gradient_metal.png", rendered_scene)); } -static sk_sp GetSurfaceFromTexture(sk_sp skia_context, +static sk_sp GetSurfaceFromTexture(const sk_sp& skia_context, SkISize texture_size, void* texture) { GrMtlTextureInfo info; diff --git a/shell/platform/embedder/tests/embedder_unittests_util.cc b/shell/platform/embedder/tests/embedder_unittests_util.cc index ba17af342e2ab..d3f6992d812bd 100644 --- a/shell/platform/embedder/tests/embedder_unittests_util.cc +++ b/shell/platform/embedder/tests/embedder_unittests_util.cc @@ -5,6 +5,7 @@ #define FML_USED_ON_EMBEDDER #include +#include #include "flutter/shell/platform/embedder/tests/embedder_test_backingstore_producer.h" #include "flutter/shell/platform/embedder/tests/embedder_unittests_util.h" @@ -32,7 +33,7 @@ sk_sp CreateRenderSurface(const FlutterLayer& layer, } // Normalizes the color-space, color-type and alpha-type for comparison. -static sk_sp NormalizeImage(sk_sp image) { +static sk_sp NormalizeImage(const sk_sp& image) { // To avoid clipping, convert to a very wide gamut, and a high bit depth. sk_sp norm_colorspace = SkColorSpace::MakeRGB( SkNamedTransferFn::kRec2020, SkNamedGamut::kRec2020); @@ -56,7 +57,7 @@ static sk_sp NormalizeImage(sk_sp image) { return data; } -bool RasterImagesAreSame(sk_sp a, sk_sp b) { +bool RasterImagesAreSame(const sk_sp& a, const sk_sp& b) { if (!a || !b) { return false; } @@ -127,7 +128,7 @@ void ConfigureBackingStore(FlutterBackingStore& backing_store, bool WriteImageToDisk(const fml::UniqueFD& directory, const std::string& name, - sk_sp image) { + const sk_sp& image) { if (!image) { return false; } @@ -144,7 +145,7 @@ bool WriteImageToDisk(const fml::UniqueFD& directory, } bool ImageMatchesFixture(const std::string& fixture_file_name, - sk_sp scene_image) { + const sk_sp& scene_image) { fml::FileMapping fixture_image_mapping(OpenFixture(fixture_file_name)); FML_CHECK(fixture_image_mapping.GetSize() != 0u) @@ -219,7 +220,7 @@ bool SurfacePixelDataMatchesBytes(SkSurface* surface, FML_LOG(ERROR) << "SkImage pixel data didn't match bytes."; { - const uint8_t* addr = (const uint8_t*)pixmap.addr(); + const uint8_t* addr = static_cast(pixmap.addr()); std::stringstream stream; for (size_t i = 0; i < pixmap.computeByteSize(); ++i) { stream << "0x" << std::setfill('0') << std::setw(2) << std::uppercase @@ -255,7 +256,8 @@ void FilterMutationsByType( const FlutterPlatformViewMutation** mutations, size_t count, FlutterPlatformViewMutationType type, - std::function handler) { + const std::function& + handler) { if (mutations == nullptr) { return; } @@ -275,7 +277,7 @@ void FilterMutationsByType( FlutterPlatformViewMutationType type, std::function handler) { return FilterMutationsByType(view->mutations, view->mutations_count, type, - handler); + std::move(handler)); } SkMatrix GetTotalMutationTransformationMatrix( diff --git a/shell/platform/embedder/tests/embedder_unittests_util.h b/shell/platform/embedder/tests/embedder_unittests_util.h index f888aa349068f..c1a146981b781 100644 --- a/shell/platform/embedder/tests/embedder_unittests_util.h +++ b/shell/platform/embedder/tests/embedder_unittests_util.h @@ -23,7 +23,7 @@ namespace testing { sk_sp CreateRenderSurface(const FlutterLayer& layer, GrDirectContext* context); -bool RasterImagesAreSame(sk_sp a, sk_sp b); +bool RasterImagesAreSame(const sk_sp& a, const sk_sp& b); /// @brief Prepends a prefix to the name which is unique to the test /// context type. This is useful for tests that use @@ -61,10 +61,10 @@ void ConfigureBackingStore(FlutterBackingStore& backing_store, bool WriteImageToDisk(const fml::UniqueFD& directory, const std::string& name, - sk_sp image); + const sk_sp& image); bool ImageMatchesFixture(const std::string& fixture_file_name, - sk_sp scene_image); + const sk_sp& scene_image); bool ImageMatchesFixture(const std::string& fixture_file_name, std::future>& scene_image); @@ -79,7 +79,8 @@ void FilterMutationsByType( const FlutterPlatformViewMutation** mutations, size_t count, FlutterPlatformViewMutationType type, - std::function handler); + const std::function& + handler); void FilterMutationsByType( const FlutterPlatformView* view, diff --git a/tools/clang_tidy/lib/clang_tidy.dart b/tools/clang_tidy/lib/clang_tidy.dart index b1af4486dde48..e722c89e0372f 100644 --- a/tools/clang_tidy/lib/clang_tidy.dart +++ b/tools/clang_tidy/lib/clang_tidy.dart @@ -3,7 +3,7 @@ // found in the LICENSE file. import 'dart:convert' show jsonDecode; -import 'dart:io' as io show Directory, File, stderr, stdout; +import 'dart:io' as io show File, stderr, stdout; import 'package:meta/meta.dart'; import 'package:path/path.dart' as path; @@ -134,8 +134,7 @@ class ClangTidy { final _ComputeJobsResult computeJobsResult = await _computeJobs( changedFileBuildCommands, - options.repoPath, - options.checks, + options, ); final int computeResult = computeJobsResult.sawMalformed ? 1 : 0; final List jobs = computeJobsResult.jobs; @@ -193,15 +192,14 @@ class ClangTidy { Future<_ComputeJobsResult> _computeJobs( List commands, - io.Directory repoPath, - String? checks, + Options options, ) async { bool sawMalformed = false; final List jobs = []; for (final Command command in commands) { final String relativePath = path.relative( command.filePath, - from: repoPath.parent.path, + from: options.repoPath.parent.path, ); final LintAction action = await command.lintAction; switch (action) { @@ -217,7 +215,7 @@ class ClangTidy { break; case LintAction.lint: _outSink.writeln('🔶 linting $relativePath'); - jobs.add(command.createLintJob(checks, options.fix)); + jobs.add(command.createLintJob(options)); break; case LintAction.skipThirdParty: _outSink.writeln('🔷 ignoring $relativePath (third_party)'); diff --git a/tools/clang_tidy/lib/src/command.dart b/tools/clang_tidy/lib/src/command.dart index 522e46f39c74e..f3853bc607a70 100644 --- a/tools/clang_tidy/lib/src/command.dart +++ b/tools/clang_tidy/lib/src/command.dart @@ -9,6 +9,8 @@ import 'package:meta/meta.dart'; import 'package:path/path.dart' as path; import 'package:process_runner/process_runner.dart'; +import 'options.dart'; + /// The url prefix for issues that must be attached to the directive in files /// that disables linting. const String issueUrlPrefix = 'https://github.com/flutter/flutter/issues'; @@ -129,12 +131,14 @@ class Command { } /// The job for the process runner for the lint needed for this command. - WorkerJob createLintJob(String? checks, bool fix) { + WorkerJob createLintJob(Options options) { final List args = [ filePath, - if (checks != null) - checks, - if (fix) ...[ + if (options.warningsAsErrors != null) + '--warnings-as-errors="${options.warningsAsErrors}"', + if (options.checks != null) + options.checks!, + if (options.fix) ...[ '--fix', '--format-style=file', ], diff --git a/tools/clang_tidy/lib/src/options.dart b/tools/clang_tidy/lib/src/options.dart index 1b07bcf0deb37..e10ca99b66ad1 100644 --- a/tools/clang_tidy/lib/src/options.dart +++ b/tools/clang_tidy/lib/src/options.dart @@ -10,6 +10,16 @@ import 'package:path/path.dart' as path; // Path to root of the flutter/engine repository containing this script. final String _engineRoot = path.dirname(path.dirname(path.dirname(path.dirname(path.fromUri(io.Platform.script))))); + +/// Adds warnings as errors for only specific runs. This is helpful if migrating one platform at a time. +String? _platformSpecificWarningsAsErrors(ArgResults options) { + if (options['target-variant'] == 'host_debug' && io.Platform.isMacOS) { + return options['mac-host-warnings-as-errors'] as String?; + } + return null; +} + + /// A class for organizing the options to the Engine linter, and the files /// that it operates on. class Options { @@ -23,6 +33,7 @@ class Options { this.lintHead = false, this.fix = false, this.errorMessage, + this.warningsAsErrors, StringSink? errSink, }) : checks = checksArg.isNotEmpty ? '--checks=$checksArg' : null, _errSink = errSink ?? io.stderr; @@ -64,6 +75,7 @@ class Options { lintHead: options['lint-head'] as bool, fix: options['fix'] as bool, errSink: errSink, + warningsAsErrors: _platformSpecificWarningsAsErrors(options), ); } @@ -134,6 +146,9 @@ class Options { valueHelp: 'host_debug|android_debug_unopt|ios_debug|ios_debug_sim_unopt', defaultsTo: 'host_debug', ) + ..addOption('mac-host-warnings-as-errors', + help: + 'checks that will be treated as errors when running debug_host on mac.') ..addOption( 'src-dir', help: 'Path to the engine src directory. Cannot be used with --compile-commands.', @@ -159,11 +174,13 @@ class Options { /// The root of the flutter/engine repository. final io.Directory repoPath = io.Directory(_engineRoot); - /// Arguments to plumb through to clang-tidy formatted as a command line - /// argument. + /// Argument sent as `warnings-as-errors` to clang-tidy. + final String? warningsAsErrors; + + /// Checks argument as supplied to the command-line. final String checksArg; - /// Check arguments to plumb through to clang-tidy. + /// Check argument to be supplied to the clang-tidy subprocess. final String? checks; /// Whether all files should be linted. diff --git a/tools/clang_tidy/test/clang_tidy_test.dart b/tools/clang_tidy/test/clang_tidy_test.dart index 76e8eeab820a0..70ff2cb06dfe8 100644 --- a/tools/clang_tidy/test/clang_tidy_test.dart +++ b/tools/clang_tidy/test/clang_tidy_test.dart @@ -6,6 +6,7 @@ import 'dart:io' as io show File, Platform, stderr; import 'package:clang_tidy/clang_tidy.dart'; import 'package:clang_tidy/src/command.dart'; +import 'package:clang_tidy/src/options.dart'; import 'package:litetest/litetest.dart'; import 'package:process_runner/process_runner.dart'; @@ -228,14 +229,17 @@ Future main(List args) async { expect(commands, isNotEmpty); final Command command = commands.first; expect(command.tidyPath, contains('clang/bin/clang-tidy')); - final WorkerJob jobNoFix = command.createLintJob(null, false); + final Options noFixOptions = Options(buildCommandsPath: io.File('.')); + expect(noFixOptions.fix, isFalse); + final WorkerJob jobNoFix = command.createLintJob(noFixOptions); expect(jobNoFix.command[0], endsWith('../../buildtools/mac-x64/clang/bin/clang-tidy')); expect(jobNoFix.command[1], endsWith(filePath.replaceAll('/', io.Platform.pathSeparator))); expect(jobNoFix.command[2], '--'); expect(jobNoFix.command[3], ''); expect(jobNoFix.command[4], endsWith(filePath)); - final WorkerJob jobWithFix = command.createLintJob(null, true); + final Options fixOptions = Options(buildCommandsPath: io.File('.'), fix: true); + final WorkerJob jobWithFix = command.createLintJob(fixOptions); expect(jobWithFix.command[0], endsWith('../../buildtools/mac-x64/clang/bin/clang-tidy')); expect(jobWithFix.command[1], endsWith(filePath.replaceAll('/', io.Platform.pathSeparator))); expect(jobWithFix.command[2], '--fix');