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

gaaclarke · 2022-11-08T22:29:36Z

WARNING: If this fails in a post submit check, do not revert without considering that violations were introduced between this being authored and the post submit check.

This was previously just turned on for the mac host lint while the migration happened to all targets.

issue: flutter/flutter#113848

Pre-launch Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
I listed at least one issue that this PR fixes in the description above.
I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See testing the engine for instructions on writing and running engine tests.
I updated/added relevant documentation (doc comments with ///).
I signed the CLA.
All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

gaaclarke · 2022-11-08T22:56:38Z

impeller/renderer/backend/vulkan/command_buffer_vk.cc

@@ -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)),


Things like this will get wiped out when I turn on the const move linter.

skia-gold · 2022-11-09T01:45:29Z

Gold has detected about 72 new digest(s) on patchset 4.
View them at https://flutter-engine-gold.skia.org/cl/github/37447

gaaclarke · 2022-11-09T18:41:02Z

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) {


Looks like there are a few stray flags that are not turned on as errors still.

gaaclarke · 2022-11-09T18:59:14Z

shell/platform/android/android_external_texture_gl.cc

@@ -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)),


I'm not quite sure why sometimes it's okay with value arguments like this, but at least it's still a net positive because it's finding move locations.

gaaclarke · 2022-11-09T19:03:14Z

ci/lint.sh


+FLUTTER_LINT_PRINT_FIX=1


This needs to be removed before landing.

I just didn't turn it off yet since I want this round of ci to complete.

zanderso

lgtm w/ noted cleanups

skia-gold · 2022-11-09T19:35:29Z

Gold has detected about 72 new digest(s) on patchset 6.
View them at https://flutter-engine-gold.skia.org/cl/github/37447

…lutter/engine#37447)

…115025) * 6daa75ad4 Roll Skia from 0c8127b3dd7d to 6a056a26b3c5 (16 revisions) (flutter/engine#37438) * 6517ee8ee Roll Skia from 6a056a26b3c5 to 3cecb22c5827 (1 revision) (flutter/engine#37439) * c1200c44f Sdk roll failure - add facet to allowlist test_manager and other packages. (flutter/engine#37395) * 179154ec1 Revert "Sdk roll failure - add facet to allowlist test_manager and other packages. (#37395)" (flutter/engine#37449) * c171f27c4 Revert "Add rects to accumulator rather than bounds (#37435)" (flutter/engine#37444) * 583c71ce4 Clang-tidy: Fixed math on shard-id validator. (flutter/engine#37433) * d224648fc Roll Skia from 3cecb22c5827 to 94450cd1df4e (4 revisions) (flutter/engine#37445) * 5df42a87b Oops, accidentally compiled the non-null-safe platform file as null safe. (flutter/engine#37453) * 05455ac0a Roll Skia from 94450cd1df4e to 22526b55be02 (6 revisions) (flutter/engine#37455) * 9628070ee Roll Skia from 22526b55be02 to 511aa59461f3 (1 revision) (flutter/engine#37458) * fc7293441 [Impeller] Don't double-convert include path encodings in ImpellerC (flutter/engine#37408) * d82a2df8a Roll Skia from 511aa59461f3 to 790aedd91cd8 (2 revisions) (flutter/engine#37463) * be54e3ea0 Roll Skia from 790aedd91cd8 to fe143a0dc368 (3 revisions) (flutter/engine#37467) * 410f270e6 Roll Dart SDK from c15cdb978761 to 874a662f7507 (3 revisions) (flutter/engine#37466) * e778ca4e0 Roll Fuchsia Mac SDK from d4l6A1aPw6Z0YjxmA... to vEw8iuSZYmlfk1JiE... (flutter/engine#37468) * f8048be39 [Reland] Add rects to accumulator rather than bounds (#37435) (flutter/engine#37451) * 214b84137 Roll Dart SDK from 874a662f7507 to 4fcefaf03496 (1 revision) (flutter/engine#37470) * 8ecb1c913 [macOS] Remove OpenGL rendering backend (flutter/engine#37448) * 1a8888b51 Roll Skia from fe143a0dc368 to 0854badd5783 (1 revision) (flutter/engine#37471) * ca303bfd4 Roll Dart SDK from 4fcefaf03496 to fa6586382e5d (1 revision) (flutter/engine#37473) * 5d0df764b Use traingle strip for difference clip (flutter/engine#37469) * eeba9b3c8 Support HLSL ingestion in ImpellerC (flutter/engine#37461) * 5f88ee42b Use IndexError.check instead of RangeError.checkValidIndex in buffers. (flutter/engine#37450) * b65ac4ac4 Roll expat to 2.5.0 (flutter/engine#37383) * ddde8300e [sdk_roll_failure] add facet to allowlist test_manager and other packages. (flutter/engine#37479) * 62b581ee0 Roll CanvasKit to 0.37.1 (flutter/engine#37454) * aa4b3ea2f Turned on performance-unnecessary-value-param everywhere. (flutter/engine#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

…lutter#115025) * 6daa75ad4 Roll Skia from 0c8127b3dd7d to 6a056a26b3c5 (16 revisions) (flutter/engine#37438) * 6517ee8ee Roll Skia from 6a056a26b3c5 to 3cecb22c5827 (1 revision) (flutter/engine#37439) * c1200c44f Sdk roll failure - add facet to allowlist test_manager and other packages. (flutter/engine#37395) * 179154ec1 Revert "Sdk roll failure - add facet to allowlist test_manager and other packages. (flutter#37395)" (flutter/engine#37449) * c171f27c4 Revert "Add rects to accumulator rather than bounds (flutter#37435)" (flutter/engine#37444) * 583c71ce4 Clang-tidy: Fixed math on shard-id validator. (flutter/engine#37433) * d224648fc Roll Skia from 3cecb22c5827 to 94450cd1df4e (4 revisions) (flutter/engine#37445) * 5df42a87b Oops, accidentally compiled the non-null-safe platform file as null safe. (flutter/engine#37453) * 05455ac0a Roll Skia from 94450cd1df4e to 22526b55be02 (6 revisions) (flutter/engine#37455) * 9628070ee Roll Skia from 22526b55be02 to 511aa59461f3 (1 revision) (flutter/engine#37458) * fc7293441 [Impeller] Don't double-convert include path encodings in ImpellerC (flutter/engine#37408) * d82a2df8a Roll Skia from 511aa59461f3 to 790aedd91cd8 (2 revisions) (flutter/engine#37463) * be54e3ea0 Roll Skia from 790aedd91cd8 to fe143a0dc368 (3 revisions) (flutter/engine#37467) * 410f270e6 Roll Dart SDK from c15cdb978761 to 874a662f7507 (3 revisions) (flutter/engine#37466) * e778ca4e0 Roll Fuchsia Mac SDK from d4l6A1aPw6Z0YjxmA... to vEw8iuSZYmlfk1JiE... (flutter/engine#37468) * f8048be39 [Reland] Add rects to accumulator rather than bounds (flutter#37435) (flutter/engine#37451) * 214b84137 Roll Dart SDK from 874a662f7507 to 4fcefaf03496 (1 revision) (flutter/engine#37470) * 8ecb1c913 [macOS] Remove OpenGL rendering backend (flutter/engine#37448) * 1a8888b51 Roll Skia from fe143a0dc368 to 0854badd5783 (1 revision) (flutter/engine#37471) * ca303bfd4 Roll Dart SDK from 4fcefaf03496 to fa6586382e5d (1 revision) (flutter/engine#37473) * 5d0df764b Use traingle strip for difference clip (flutter/engine#37469) * eeba9b3c8 Support HLSL ingestion in ImpellerC (flutter/engine#37461) * 5f88ee42b Use IndexError.check instead of RangeError.checkValidIndex in buffers. (flutter/engine#37450) * b65ac4ac4 Roll expat to 2.5.0 (flutter/engine#37383) * ddde8300e [sdk_roll_failure] add facet to allowlist test_manager and other packages. (flutter/engine#37479) * 62b581ee0 Roll CanvasKit to 0.37.1 (flutter/engine#37454) * aa4b3ea2f Turned on performance-unnecessary-value-param everywhere. (flutter/engine#37447)

gaaclarke force-pushed the unnecessary-value-everywhere branch from 8e4c1f0 to 34af9d1 Compare November 8, 2022 22:54

gaaclarke commented Nov 8, 2022

View reviewed changes

gaaclarke force-pushed the unnecessary-value-everywhere branch from 5731389 to 99893e3 Compare November 9, 2022 00:30

gaaclarke force-pushed the unnecessary-value-everywhere branch from 6d89616 to d252453 Compare November 9, 2022 17:12

gaaclarke added 5 commits November 9, 2022 10:00

Turned on performance-unnecessary-value-param everywhere.

96f2fe5

linux host additions

1a8e09f

ios patch

16db8da

reverted bad fix

7366e97

revert bad fix

148491a

gaaclarke force-pushed the unnecessary-value-everywhere branch from a9549e5 to 148491a Compare November 9, 2022 18:34

gaaclarke commented Nov 9, 2022

View reviewed changes

another ios patch

d61c68f

flutter-dashboard bot added platform-android platform-ios platform-linux labels Nov 9, 2022

gaaclarke commented Nov 9, 2022

View reviewed changes

gaaclarke requested a review from zanderso November 9, 2022 19:14

gaaclarke marked this pull request as ready for review November 9, 2022 19:14

zanderso approved these changes Nov 9, 2022

View reviewed changes

removed lint fix printer

45a5dd8

gaaclarke added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 9, 2022

auto-submit bot merged commit aa4b3ea into flutter:main Nov 9, 2022

engine-flutter-autoroll mentioned this pull request Nov 9, 2022

Roll Flutter Engine from c76035429c36 to aa4b3ea2f733 (27 revisions) flutter/flutter#115022

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 9, 2022

aa4b3ea2f Turned on performance-unnecessary-value-param everywhere. (f…

eaaa46a

…lutter/engine#37447)

engine-flutter-autoroll mentioned this pull request Nov 9, 2022

Roll Flutter Engine from c76035429c36 to aa4b3ea2f733 (27 revisions) flutter/flutter#115025

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 9, 2022

aa4b3ea2f Turned on performance-unnecessary-value-param everywhere. (f…

45c4462

…lutter/engine#37447)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

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

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

Uh oh!

gaaclarke commented Nov 8, 2022 •

edited

Loading

Uh oh!

gaaclarke Nov 8, 2022

Uh oh!

skia-gold commented Nov 9, 2022

Uh oh!

gaaclarke Nov 9, 2022

Uh oh!

gaaclarke Nov 9, 2022

Uh oh!

gaaclarke Nov 9, 2022

Uh oh!

gaaclarke Nov 9, 2022

Uh oh!

gaaclarke Nov 9, 2022

Uh oh!

zanderso left a comment

Uh oh!

skia-gold commented Nov 9, 2022

Uh oh!

Uh oh!


		FLUTTER_LINT_PRINT_FIX=1

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

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

Uh oh!

Conversation

gaaclarke commented Nov 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

gaaclarke Nov 8, 2022

Choose a reason for hiding this comment

Uh oh!

skia-gold commented Nov 9, 2022

Uh oh!

gaaclarke Nov 9, 2022

Choose a reason for hiding this comment

Uh oh!

gaaclarke Nov 9, 2022

Choose a reason for hiding this comment

Uh oh!

gaaclarke Nov 9, 2022

Choose a reason for hiding this comment

Uh oh!

gaaclarke Nov 9, 2022

Choose a reason for hiding this comment

Uh oh!

gaaclarke Nov 9, 2022

Choose a reason for hiding this comment

Uh oh!

zanderso left a comment

Choose a reason for hiding this comment

Uh oh!

skia-gold commented Nov 9, 2022

Uh oh!

Uh oh!

gaaclarke commented Nov 8, 2022 •

edited

Loading