Enable ClangTidy check performance-unnecessary-copy-initialization.
https://clang.llvm.org/extra/clang-tidy/checks/performance-unnecessary-copy-initialization.html
Finds local variable declarations that are initialized using the copy
constructor of a non-trivially-copyable type but it would suffice to
obtain a const reference.
The check is only applied if it is safe to replace the copy by a const
reference. This is the case when the variable is const qualified or when
it is only used as a const, i.e. only const methods or operators are
invoked on it, or it is used as const reference or value argument in
constructors or function calls.
Change-Id: I1261410deccd8ea64e85edec53fbd5360940e587
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/308759
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
diff --git a/.clang-tidy b/.clang-tidy
index 3d38c4e..5160dd6 100644
--- a/.clang-tidy
+++ b/.clang-tidy
@@ -1,4 +1,4 @@
-Checks: '-*,bugprone-use-after-move,bugprone-unused-raii,bugprone-undelegated-constructor,bugprone-argument-comment,performance-for-range-copy,bugprone-bool-pointer-implicit-conversion,readability-redundant-preprocessor,misc-definitions-in-headers,modernize-make-unique,llvm-namespace-comment,readability-static-accessed-through-instance,readability-const-return-type,google-build-namespaces'
+Checks: '-*,bugprone-use-after-move,bugprone-unused-raii,bugprone-undelegated-constructor,bugprone-argument-comment,bugprone-bool-pointer-implicit-conversion,performance-unnecessary-copy-initialization,performance-for-range-copy,readability-redundant-preprocessor,misc-definitions-in-headers,modernize-make-unique,llvm-namespace-comment,readability-static-accessed-through-instance,readability-const-return-type,google-build-namespaces'
CheckOptions:
- key: llvm-namespace-comment.SpacesBeforeComments
value: 2
diff --git a/modules/skparagraph/src/TextLine.cpp b/modules/skparagraph/src/TextLine.cpp
index d366199..377380a 100644
--- a/modules/skparagraph/src/TextLine.cpp
+++ b/modules/skparagraph/src/TextLine.cpp
@@ -989,7 +989,7 @@
break;
}
case RectHeightStyle::kStrut: {
- auto strutStyle = paragraphStyle.getStrutStyle();
+ const auto& strutStyle = paragraphStyle.getStrutStyle();
if (strutStyle.getStrutEnabled()
&& strutStyle.getFontSize() > 0) {
auto strutMetrics = fOwner->strutMetrics();
diff --git a/samplecode/SampleRegion.cpp b/samplecode/SampleRegion.cpp
index 7c16c01..2b898cc 100644
--- a/samplecode/SampleRegion.cpp
+++ b/samplecode/SampleRegion.cpp
@@ -218,9 +218,7 @@
this->build_rgn(&rgn, op);
{
- SkRegion tmp, tmp2(rgn);
-
- tmp = tmp2;
+ SkRegion tmp = rgn;
tmp.translate(5, -3);
{
diff --git a/src/gpu/ops/GrAAConvexPathRenderer.cpp b/src/gpu/ops/GrAAConvexPathRenderer.cpp
index 834e8da..5fe5792 100644
--- a/src/gpu/ops/GrAAConvexPathRenderer.cpp
+++ b/src/gpu/ops/GrAAConvexPathRenderer.cpp
@@ -928,7 +928,7 @@
GR_DRAW_OP_TEST_DEFINE(AAConvexPathOp) {
SkMatrix viewMatrix = GrTest::TestMatrixInvertible(random);
- SkPath path = GrTest::TestPathConvex(random);
+ const SkPath& path = GrTest::TestPathConvex(random);
const GrUserStencilSettings* stencilSettings = GrGetRandomStencil(random, context);
return AAConvexPathOp::Make(context, std::move(paint), viewMatrix, path, stencilSettings);
}
diff --git a/src/gpu/ops/GrAAHairLinePathRenderer.cpp b/src/gpu/ops/GrAAHairLinePathRenderer.cpp
index 8216e97..afe7105 100644
--- a/src/gpu/ops/GrAAHairLinePathRenderer.cpp
+++ b/src/gpu/ops/GrAAHairLinePathRenderer.cpp
@@ -1306,7 +1306,7 @@
GR_DRAW_OP_TEST_DEFINE(AAHairlineOp) {
SkMatrix viewMatrix = GrTest::TestMatrix(random);
- SkPath path = GrTest::TestPath(random);
+ const SkPath& path = GrTest::TestPath(random);
SkIRect devClipBounds;
devClipBounds.setEmpty();
return AAHairlineOp::Make(context, std::move(paint), viewMatrix, path,
diff --git a/src/gpu/ops/GrAALinearizingConvexPathRenderer.cpp b/src/gpu/ops/GrAALinearizingConvexPathRenderer.cpp
index 9c67fd8..fdd4162 100644
--- a/src/gpu/ops/GrAALinearizingConvexPathRenderer.cpp
+++ b/src/gpu/ops/GrAALinearizingConvexPathRenderer.cpp
@@ -415,7 +415,7 @@
GR_DRAW_OP_TEST_DEFINE(AAFlatteningConvexPathOp) {
SkMatrix viewMatrix = GrTest::TestMatrixPreservesRightAngles(random);
- SkPath path = GrTest::TestPathConvex(random);
+ const SkPath& path = GrTest::TestPathConvex(random);
SkStrokeRec::Style styles[3] = { SkStrokeRec::kFill_Style,
SkStrokeRec::kStroke_Style,
diff --git a/src/gpu/ops/GrDefaultPathRenderer.cpp b/src/gpu/ops/GrDefaultPathRenderer.cpp
index 17ddfa8..d711fdb 100644
--- a/src/gpu/ops/GrDefaultPathRenderer.cpp
+++ b/src/gpu/ops/GrDefaultPathRenderer.cpp
@@ -734,7 +734,7 @@
// For now just hairlines because the other types of draws require two ops.
// TODO we should figure out a way to combine the stencil and cover steps into one op.
GrStyle style(SkStrokeRec::kHairline_InitStyle);
- SkPath path = GrTest::TestPath(random);
+ const SkPath& path = GrTest::TestPath(random);
// Compute srcSpaceTol
SkRect bounds = path.getBounds();
diff --git a/src/gpu/ops/GrTriangulatingPathRenderer.cpp b/src/gpu/ops/GrTriangulatingPathRenderer.cpp
index 7e696c4..64429f0 100644
--- a/src/gpu/ops/GrTriangulatingPathRenderer.cpp
+++ b/src/gpu/ops/GrTriangulatingPathRenderer.cpp
@@ -433,7 +433,7 @@
GR_DRAW_OP_TEST_DEFINE(TriangulatingPathOp) {
SkMatrix viewMatrix = GrTest::TestMatrixInvertible(random);
- SkPath path = GrTest::TestPath(random);
+ const SkPath& path = GrTest::TestPath(random);
SkIRect devClipBounds = SkIRect::MakeLTRB(
random->nextU(), random->nextU(), random->nextU(), random->nextU());
devClipBounds.sort();
diff --git a/src/pdf/SkPDFBitmap.cpp b/src/pdf/SkPDFBitmap.cpp
index fcd03b1..bee6336 100644
--- a/src/pdf/SkPDFBitmap.cpp
+++ b/src/pdf/SkPDFBitmap.cpp
@@ -258,7 +258,7 @@
return;
}
SkBitmap bm = to_pixels(img);
- SkPixmap pm = bm.pixmap();
+ const SkPixmap& pm = bm.pixmap();
bool isOpaque = pm.isOpaque() || pm.computeIsOpaque();
if (encodingQuality <= 100 && isOpaque) {
sk_sp<SkData> data = img->encodeToData(SkEncodedImageFormat::kJPEG, encodingQuality);
diff --git a/src/pdf/SkPDFType1Font.cpp b/src/pdf/SkPDFType1Font.cpp
index 5bcb3f8..4914300 100644
--- a/src/pdf/SkPDFType1Font.cpp
+++ b/src/pdf/SkPDFType1Font.cpp
@@ -282,7 +282,7 @@
void SkPDFEmitType1Font(const SkPDFFont& pdfFont, SkPDFDocument* doc) {
SkTypeface* typeface = pdfFont.typeface();
- const std::vector<SkString> glyphNames = type_1_glyphnames(doc, typeface);
+ const std::vector<SkString>& glyphNames = type_1_glyphnames(doc, typeface);
SkGlyphID firstGlyphID = pdfFont.firstGlyphID();
SkGlyphID lastGlyphID = pdfFont.lastGlyphID();
diff --git a/tests/GrStyledShapeTest.cpp b/tests/GrStyledShapeTest.cpp
index 9f781c6..bd5df5a 100644
--- a/tests/GrStyledShapeTest.cpp
+++ b/tests/GrStyledShapeTest.cpp
@@ -1535,9 +1535,9 @@
REPORTER_ASSERT(reporter, fillInvertedEmptyCase.appliedPathEffectShape().inverseFilled());
REPORTER_ASSERT(reporter, fillInvertedEmptyCase.appliedFullStyleShape().inverseFilled());
- Key emptyKey(fillEmptyCase.baseKey());
+ const Key& emptyKey = fillEmptyCase.baseKey();
REPORTER_ASSERT(reporter, emptyKey.count());
- Key inverseEmptyKey(fillInvertedEmptyCase.baseKey());
+ const Key& inverseEmptyKey = fillInvertedEmptyCase.baseKey();
REPORTER_ASSERT(reporter, inverseEmptyKey.count());
TestCase::SelfExpectations expectations;
expectations.fStrokeApplies = false;
diff --git a/tests/PathTest.cpp b/tests/PathTest.cpp
index 669c713..6c130f7 100644
--- a/tests/PathTest.cpp
+++ b/tests/PathTest.cpp
@@ -1056,7 +1056,8 @@
if (expected == kDontCheckDir) {
return;
}
- SkPath copy(path); // we make a copy so that we don't cache the result on the passed in path.
+ // We make a copy so that we don't cache the result on the passed in path.
+ SkPath copy(path); // NOLINT(performance-unnecessary-copy-initialization)
SkPathPriv::FirstDirection dir;
if (SkPathPriv::CheapComputeFirstDirection(copy, &dir)) {
@@ -1313,7 +1314,8 @@
static void check_convexity(skiatest::Reporter* reporter, const SkPath& path,
SkPathConvexityType expected) {
- SkPath copy(path); // we make a copy so that we don't cache the result on the passed in path.
+ // We make a copy so that we don't cache the result on the passed in path.
+ SkPath copy(path); // NOLINT(performance-unnecessary-copy-initialization)
SkPathConvexityType c = copy.getConvexityType();
REPORTER_ASSERT(reporter, c == expected);
@@ -1619,7 +1621,8 @@
check_direction(reporter, path, gRec[i].fExpectedDirection);
// check after setting the initial convex and direction
if (kDontCheckDir != gRec[i].fExpectedDirection) {
- SkPath copy(path);
+ // We make a copy so that we don't cache the result on the passed in path.
+ SkPath copy(path); // NOLINT(performance-unnecessary-copy-initialization)
SkPathPriv::FirstDirection dir;
bool foundDir = SkPathPriv::CheapComputeFirstDirection(copy, &dir);
REPORTER_ASSERT(reporter, (gRec[i].fExpectedDirection == SkPathPriv::kUnknown_FirstDirection)
@@ -1700,7 +1703,8 @@
if (curveSelect == 0 || curveSelect == 1 || curveSelect == 2 || curveSelect == 5) {
check_convexity(reporter, path, SkPathConvexityType::kConvex);
} else {
- SkPath copy(path); // we make a copy so that we don't cache the result on the passed in path.
+ // We make a copy so that we don't cache the result on the passed in path.
+ SkPath copy(path); // NOLINT(performance-unnecessary-copy-initialization)
SkPathConvexityType c = copy.getConvexityType();
REPORTER_ASSERT(reporter, SkPathConvexityType::kUnknown == c
|| SkPathConvexityType::kConcave == c);
@@ -1735,7 +1739,8 @@
if (curveSelect == 0) {
check_convexity(reporter, path, SkPathConvexityType::kConvex);
} else {
- SkPath copy(path); // we make a copy so that we don't cache the result on the passed in path.
+ // We make a copy so that we don't cache the result on the passed in path.
+ SkPath copy(path); // NOLINT(performance-unnecessary-copy-initialization)
SkPathConvexityType c = copy.getConvexityType();
REPORTER_ASSERT(reporter, SkPathConvexityType::kUnknown == c
|| SkPathConvexityType::kConcave == c);
diff --git a/tests/ProxyRefTest.cpp b/tests/ProxyRefTest.cpp
index e590f76..12e1af3 100644
--- a/tests/ProxyRefTest.cpp
+++ b/tests/ProxyRefTest.cpp
@@ -50,7 +50,7 @@
{
sk_sp<GrTextureProxy> proxy((*make)(dContext));
if (proxy) {
- sk_sp<GrTextureProxy> extraRef(proxy);
+ sk_sp<GrTextureProxy> extraRef(proxy); // NOLINT(performance-unnecessary-copy-initialization)
int backingRefs = proxy->isInstantiated() ? 1 : -1;
@@ -88,7 +88,7 @@
{
sk_sp<GrTextureProxy> proxy((*make)(dContext));
if (proxy) {
- sk_sp<GrTextureProxy> firstExtraRef(proxy);
+ sk_sp<GrTextureProxy> firstExtraRef(proxy); // NOLINT(performance-unnecessary-copy-initialization)
int backingRefs = proxy->isInstantiated() ? 1 : -1;
@@ -98,7 +98,7 @@
CheckSingleThreadedProxyRefs(reporter, proxy.get(), 2, 1);
- sk_sp<GrTextureProxy> secondExtraRef(proxy);
+ sk_sp<GrTextureProxy> secondExtraRef(proxy); // NOLINT(performance-unnecessary-copy-initialization)
CheckSingleThreadedProxyRefs(reporter, proxy.get(), 3, 1);
}
CheckSingleThreadedProxyRefs(reporter, proxy.get(), 1, 1);
diff --git a/tests/StringTest.cpp b/tests/StringTest.cpp
index c22f961..a299ccf 100644
--- a/tests/StringTest.cpp
+++ b/tests/StringTest.cpp
@@ -274,7 +274,7 @@
std::thread threads[5];
for (auto& thread : threads) {
thread = std::thread([&] {
- SkString copy = str;
+ SkString copy = str; // NOLINT(performance-unnecessary-copy-initialization)
(void)copy.equals("test");
});
}
diff --git a/tests/SurfaceTest.cpp b/tests/SurfaceTest.cpp
index cf88858..6029ff2 100644
--- a/tests/SurfaceTest.cpp
+++ b/tests/SurfaceTest.cpp
@@ -440,10 +440,10 @@
#define EXPECT_COPY_ON_WRITE(command) \
{ \
sk_sp<SkImage> imageBefore = surface->makeImageSnapshot(); \
- sk_sp<SkImage> aur_before(imageBefore); \
+ sk_sp<SkImage> aur_before(imageBefore); /*NOLINT*/ \
canvas-> command ; \
sk_sp<SkImage> imageAfter = surface->makeImageSnapshot(); \
- sk_sp<SkImage> aur_after(imageAfter); \
+ sk_sp<SkImage> aur_after(imageAfter); /*NOLINT*/ \
REPORTER_ASSERT(reporter, imageBefore != imageAfter); \
}
@@ -608,10 +608,10 @@
// Verifies the robustness of SkSurface for handling use cases where calls
// are made before a canvas is created.
sk_sp<SkImage> image1 = surface->makeImageSnapshot();
- sk_sp<SkImage> aur_image1(image1);
+ sk_sp<SkImage> aur_image1(image1); // NOLINT(performance-unnecessary-copy-initialization)
surface->notifyContentWillChange(mode);
sk_sp<SkImage> image2 = surface->makeImageSnapshot();
- sk_sp<SkImage> aur_image2(image2);
+ sk_sp<SkImage> aur_image2(image2); // NOLINT(performance-unnecessary-copy-initialization)
REPORTER_ASSERT(reporter, image1 != image2);
}
DEF_TEST(SurfaceNoCanvas, reporter) {
diff --git a/tests/TArrayTest.cpp b/tests/TArrayTest.cpp
index 4baedb8..1f84db7 100644
--- a/tests/TArrayTest.cpp
+++ b/tests/TArrayTest.cpp
@@ -149,12 +149,12 @@
}
{
SkTArray<int> a;
- SkTArray<int> b(a);
+ SkTArray<int> b(a); // NOLINT(performance-unnecessary-copy-initialization)
REPORTER_ASSERT(reporter, b.allocCntForTest() == 0);
}
{
SkSTArray<10, int> a;
- SkTArray<int> b(a);
+ SkTArray<int> b(a); // NOLINT(performance-unnecessary-copy-initialization)
REPORTER_ASSERT(reporter, b.allocCntForTest() == 0);
}
{
diff --git a/tests/TLazyTest.cpp b/tests/TLazyTest.cpp
index ef88930..abc2817 100644
--- a/tests/TLazyTest.cpp
+++ b/tests/TLazyTest.cpp
@@ -15,7 +15,7 @@
REPORTER_ASSERT(r, lazy.getMaybeNull() == nullptr);
{
- SkTLazy<int> lazy_copy(lazy);
+ SkTLazy<int> lazy_copy(lazy); // NOLINT(performance-unnecessary-copy-initialization)
REPORTER_ASSERT(r, !lazy_copy.isValid());
REPORTER_ASSERT(r, lazy_copy.getMaybeNull() == nullptr);
}
@@ -26,7 +26,7 @@
REPORTER_ASSERT(r, 42 == *lazy.get());
{
- SkTLazy<int> lazy_copy(lazy);
+ SkTLazy<int> lazy_copy(lazy); // NOLINT(performance-unnecessary-copy-initialization)
REPORTER_ASSERT(r, lazy_copy.isValid());
REPORTER_ASSERT(r, 42 == *lazy_copy.get());
REPORTER_ASSERT(r, lazy.get() != lazy_copy.get());
diff --git a/tests/WritePixelsTest.cpp b/tests/WritePixelsTest.cpp
index b6837ef..ec8b136 100644
--- a/tests/WritePixelsTest.cpp
+++ b/tests/WritePixelsTest.cpp
@@ -226,7 +226,7 @@
return false;
}
- const SkImageInfo bmInfo = bitmap.info();
+ const SkImageInfo& bmInfo = bitmap.info();
SkIRect writeRect = SkIRect::MakeXYWH(writeX, writeY, bitmap.width(), bitmap.height());
for (int cy = 0; cy < DEV_H; ++cy) {
diff --git a/tools/DDLPromiseImageHelper.cpp b/tools/DDLPromiseImageHelper.cpp
index 0ae1df7..35e8ed8 100644
--- a/tools/DDLPromiseImageHelper.cpp
+++ b/tools/DDLPromiseImageHelper.cpp
@@ -413,7 +413,7 @@
}
#endif
} else {
- GrBackendFormat backendFormat = curImage.backendFormat(0);
+ const GrBackendFormat& backendFormat = curImage.backendFormat(0);
SkASSERT(backendFormat.isValid());
// Each DDL recorder gets its own ref on the promise callback context for the