remove SkTCast
SkTCast is functionally equivalent to reinterpret_cast.
The comment about SkTCast helping to avoid strict alising issues is not
true. Dereferencing a pointer cast to a pointer of an unrelated type is
always undefined, even if smuggled through a union like in SkTCast.
To really avoid aliasing issues, you need to make a union[1] of the two
value types, or better, memcpy between values. I've had to fix
MatrixText.cpp where switching to reinterpret_cast actually let Clang
notice and warn that we're exploiting undefined behavior, and
GrSwizzle.h and SkCamera.cpp caught by GCC.
I've switched SkTLList over to use SkAlignedSTStorage, which seems
to help convince some GCC versions that fObj is used in a sound way.
[1] The union punning trick is non-standard in C++, but GCC and MSVC
both explicitly support it. I believe Clang does not officially
explicitly support it, but probably does quietly for GCC compatibility.
Change-Id: I71822e82c962f9aaac8be24d3c0f39f4f8b05026
Reviewed-on: https://skia-review.googlesource.com/134947
Commit-Queue: Mike Klein <mtklein@chromium.org>
Commit-Queue: Brian Salomon <bsalomon@google.com>
Auto-Submit: Mike Klein <mtklein@chromium.org>
Reviewed-by: Brian Salomon <bsalomon@google.com>
diff --git a/bench/MathBench.cpp b/bench/MathBench.cpp
index 7f3408e..74b89a3 100644
--- a/bench/MathBench.cpp
+++ b/bench/MathBench.cpp
@@ -76,8 +76,8 @@
int count) = 0;
void performTest(float* SK_RESTRICT dst, const float* SK_RESTRICT src, int count) override {
- uint32_t* d = SkTCast<uint32_t*>(dst);
- const uint32_t* s = SkTCast<const uint32_t*>(src);
+ uint32_t* d = reinterpret_cast<uint32_t*>(dst);
+ const uint32_t* s = reinterpret_cast<const uint32_t*>(src);
this->performITest(d, s, count);
}
private:
diff --git a/bench/RectBench.cpp b/bench/RectBench.cpp
index 386fb43..407c3a9 100644
--- a/bench/RectBench.cpp
+++ b/bench/RectBench.cpp
@@ -185,7 +185,7 @@
for (size_t i = 0; i < sizes; i++) {
paint.setStrokeWidth(gSizes[i]);
this->setupPaint(&paint);
- canvas->drawPoints(fMode, N * 2, SkTCast<SkPoint*>(fRects), paint);
+ canvas->drawPoints(fMode, N * 2, reinterpret_cast<SkPoint*>(fRects), paint);
paint.setColor(fColors[i % N]);
}
}
@@ -263,7 +263,7 @@
this->setupPaint(&paint);
paint.setColor(color);
paint.setAlpha(alpha);
- canvas->drawPoints(fMode, N * 2, SkTCast<SkPoint*>(fRects), paint);
+ canvas->drawPoints(fMode, N * 2, reinterpret_cast<SkPoint*>(fRects), paint);
}
}
}
diff --git a/include/core/SkTLazy.h b/include/core/SkTLazy.h
index a166f05..26ee922 100644
--- a/include/core/SkTLazy.h
+++ b/include/core/SkTLazy.h
@@ -61,7 +61,7 @@
if (this->isValid()) {
fPtr->~T();
}
- fPtr = new (SkTCast<T*>(fStorage.get())) T(std::forward<Args>(args)...);
+ fPtr = new (reinterpret_cast<T*>(fStorage.get())) T(std::forward<Args>(args)...);
return fPtr;
}
@@ -75,7 +75,7 @@
if (this->isValid()) {
*fPtr = src;
} else {
- fPtr = new (SkTCast<T*>(fStorage.get())) T(src);
+ fPtr = new (reinterpret_cast<T*>(fStorage.get())) T(src);
}
return fPtr;
}
@@ -84,7 +84,7 @@
if (this->isValid()) {
*fPtr = std::move(src);
} else {
- fPtr = new (SkTCast<T*>(fStorage.get())) T(std::move(src));
+ fPtr = new (reinterpret_cast<T*>(fStorage.get())) T(std::move(src));
}
return fPtr;
}
diff --git a/include/core/SkTypes.h b/include/core/SkTypes.h
index 262002b..e001bb1 100644
--- a/include/core/SkTypes.h
+++ b/include/core/SkTypes.h
@@ -247,20 +247,6 @@
kExact
};
-///////////////////////////////////////////////////////////////////////////////
-
-/**
- * Use to cast a pointer to a different type, and maintaining strict-aliasing
- */
-template <typename Dst> Dst SkTCast(const void* ptr) {
- union {
- const void* src;
- Dst dst;
- } data;
- data.src = ptr;
- return data.dst;
-}
-
//////////////////////////////////////////////////////////////////////////////
/** \class SkNoncopyable
diff --git a/include/private/GrSwizzle.h b/include/private/GrSwizzle.h
index db645db..da5d3e0 100644
--- a/include/private/GrSwizzle.h
+++ b/include/private/GrSwizzle.h
@@ -38,8 +38,12 @@
, fKey((CToI(c[0]) << 0) | (CToI(c[1]) << 2) | (CToI(c[2]) << 4) | (CToI(c[3]) << 6)) {}
GR_STATIC_ASSERT(sizeof(char[4]) == sizeof(uint32_t));
- uint32_t* asUIntPtr() { return SkTCast<uint32_t*>(fSwiz); }
- uint32_t asUInt() const { return *SkTCast<const uint32_t*>(fSwiz); }
+ uint32_t* asUIntPtr() { return reinterpret_cast<uint32_t*>(fSwiz); }
+ uint32_t asUInt() const {
+ uint32_t v;
+ memcpy(&v, fSwiz, 4);
+ return v;
+ }
public:
GrSwizzle() { *this = RGBA(); }
diff --git a/src/core/SkDraw.cpp b/src/core/SkDraw.cpp
index 2c8eb84..097569f 100644
--- a/src/core/SkDraw.cpp
+++ b/src/core/SkDraw.cpp
@@ -723,11 +723,11 @@
}
static const SkPoint* rect_points(const SkRect& r) {
- return SkTCast<const SkPoint*>(&r);
+ return reinterpret_cast<const SkPoint*>(&r);
}
static SkPoint* rect_points(SkRect& r) {
- return SkTCast<SkPoint*>(&r);
+ return reinterpret_cast<SkPoint*>(&r);
}
static void draw_rect_as_path(const SkDraw& orig, const SkRect& prePaintRect,
diff --git a/src/core/SkScan_AntiPath.cpp b/src/core/SkScan_AntiPath.cpp
index 9c64f32..13ca3c8 100644
--- a/src/core/SkScan_AntiPath.cpp
+++ b/src/core/SkScan_AntiPath.cpp
@@ -497,7 +497,7 @@
if (middleCount >= MIN_COUNT_FOR_QUAD_LOOP) {
// loop until we're quad-byte aligned
- while (SkTCast<intptr_t>(alpha) & 0x3) {
+ while (reinterpret_cast<intptr_t>(alpha) & 0x3) {
alpha[0] = SkToU8(alpha[0] + maxValue);
alpha += 1;
middleCount -= 1;
diff --git a/src/core/SkTLList.h b/src/core/SkTLList.h
index b865538..8680ea2 100644
--- a/src/core/SkTLList.h
+++ b/src/core/SkTLList.h
@@ -8,9 +8,9 @@
#ifndef SkTLList_DEFINED
#define SkTLList_DEFINED
-#include "SkTInternalLList.h"
-
#include "SkMalloc.h"
+#include "SkTInternalLList.h"
+#include "SkTemplates.h"
#include "SkTypes.h"
#include <new>
#include <utility>
@@ -32,7 +32,7 @@
private:
struct Block;
struct Node {
- char fObj[sizeof(T)];
+ SkAlignedSTStorage<1, T> fObj;
SK_DECLARE_INTERNAL_LLIST_INTERFACE(Node);
Block* fBlock; // owning block.
};
@@ -51,7 +51,7 @@
typename NodeList::Iter iter;
Node* node = iter.init(fList, Iter::kHead_IterStart);
while (node) {
- SkTCast<T*>(node->fObj)->~T();
+ reinterpret_cast<T*>(node->fObj.get())->~T();
Block* block = node->fBlock;
node = iter.next();
if (0 == --block->fNodesInUse) {
@@ -71,7 +71,7 @@
Node* node = this->createNode();
fList.addToHead(node);
this->validate();
- return new (node->fObj) T(std::forward<Args>(args)...);
+ return new (node->fObj.get()) T(std::forward<Args>(args)...);
}
/** Adds a new element to the list at the tail. */
@@ -80,7 +80,7 @@
Node* node = this->createNode();
fList.addToTail(node);
this->validate();
- return new (node->fObj) T(std::forward<Args>(args)...);
+ return new (node->fObj.get()) T(std::forward<Args>(args)...);
}
/** Adds a new element to the list before the location indicated by the iterator. If the
@@ -90,7 +90,7 @@
Node* node = this->createNode();
fList.addBefore(node, location.getNode());
this->validate();
- return new (node->fObj) T(std::forward<Args>(args)...);
+ return new (node->fObj.get()) T(std::forward<Args>(args)...);
}
/** Adds a new element to the list after the location indicated by the iterator. If the
@@ -100,7 +100,7 @@
Node* node = this->createNode();
fList.addAfter(node, location.getNode());
this->validate();
- return new (node->fObj) T(std::forward<Args>(args)...);
+ return new (node->fObj.get()) T(std::forward<Args>(args)...);
}
/** Convenience methods for getting an iterator initialized to the head/tail of the list. */
@@ -133,7 +133,7 @@
void remove(T* t) {
this->validate();
Node* node = reinterpret_cast<Node*>(t);
- SkASSERT(reinterpret_cast<T*>(node->fObj) == t);
+ SkASSERT(reinterpret_cast<T*>(node->fObj.get()) == t);
this->removeNode(node);
this->validate();
}
@@ -210,7 +210,7 @@
T* nodeToObj(Node* node) {
if (node) {
- return reinterpret_cast<T*>(node->fObj);
+ return reinterpret_cast<T*>(node->fObj.get());
} else {
return nullptr;
}
@@ -264,7 +264,7 @@
void removeNode(Node* node) {
SkASSERT(node);
fList.remove(node);
- SkTCast<T*>(node->fObj)->~T();
+ reinterpret_cast<T*>(node->fObj.get())->~T();
Block* block = node->fBlock;
// Don't ever elease the first block, just add its nodes to the free list
if (0 == --block->fNodesInUse && block != &fFirstBlock) {
diff --git a/src/gpu/GrProgramDesc.cpp b/src/gpu/GrProgramDesc.cpp
index c552789..e5da3be 100644
--- a/src/gpu/GrProgramDesc.cpp
+++ b/src/gpu/GrProgramDesc.cpp
@@ -65,7 +65,7 @@
if (0 == word32Count) {
return;
}
- uint16_t* k16 = SkTCast<uint16_t*>(b->add32n(word32Count));
+ uint16_t* k16 = reinterpret_cast<uint16_t*>(b->add32n(word32Count));
int j = 0;
for (int i = 0; i < numTextureSamplers; ++i, ++j) {
const GrResourceIOProcessor::TextureSampler& sampler = proc.textureSampler(i);
diff --git a/src/utils/SkCamera.cpp b/src/utils/SkCamera.cpp
index cb364a5..3475bb5 100644
--- a/src/utils/SkCamera.cpp
+++ b/src/utils/SkCamera.cpp
@@ -218,13 +218,13 @@
fAxis.normalize(&axis);
{
- SkScalar dot = SkUnit3D::Dot(*SkTCast<const SkUnit3D*>(&fZenith), axis);
+ SkScalar dot = SkUnit3D::Dot(SkUnit3D{fZenith.fX, fZenith.fY, fZenith.fZ}, axis);
zenith.fX = fZenith.fX - dot * axis.fX;
zenith.fY = fZenith.fY - dot * axis.fY;
zenith.fZ = fZenith.fZ - dot * axis.fZ;
- SkTCast<SkPoint3D*>(&zenith)->normalize(&zenith);
+ SkPoint3D{zenith.fX, zenith.fY, zenith.fZ}.normalize(&zenith);
}
SkUnit3D::Cross(axis, zenith, &cross);
@@ -276,8 +276,8 @@
diff.fY = quilt.fOrigin.fY - fLocation.fY;
diff.fZ = quilt.fOrigin.fZ - fLocation.fZ;
- dot = SkUnit3D::Dot(*SkTCast<const SkUnit3D*>(&diff),
- *SkTCast<const SkUnit3D*>(SkTCast<const SkScalar*>(&fOrientation) + 6));
+ dot = SkUnit3D::Dot(SkUnit3D{diff.fX, diff.fY, diff.fZ},
+ SkUnit3D{mapPtr[6], mapPtr[7], mapPtr[8]});
// This multiplies fOrientation by the matrix [quilt.fU quilt.fV diff] -- U, V, and diff are
// column vectors in the matrix -- then divides by the length of the projection of diff onto
diff --git a/tests/MatrixTest.cpp b/tests/MatrixTest.cpp
index 5dfe2ca..03661dc 100644
--- a/tests/MatrixTest.cpp
+++ b/tests/MatrixTest.cpp
@@ -27,6 +27,12 @@
return true;
}
+static int float_bits(float f) {
+ int result;
+ memcpy(&result, &f, 4);
+ return result;
+}
+
static bool are_equal(skiatest::Reporter* reporter,
const SkMatrix& a,
const SkMatrix& b) {
@@ -38,8 +44,8 @@
for (int i = 0; i < 9; ++i) {
float aVal = a.get(i);
float bVal = b.get(i);
- int aValI = *SkTCast<int*>(&aVal);
- int bValI = *SkTCast<int*>(&bVal);
+ int aValI = float_bits(aVal);
+ int bValI = float_bits(bVal);
if (0 == aVal && 0 == bVal && aValI != bValI) {
foundZeroSignDiff = true;
} else {
@@ -52,8 +58,8 @@
for (int i = 0; i < 9; ++i) {
float aVal = a.get(i);
float bVal = b.get(i);
- int aValI = *SkTCast<int*>(&aVal);
- int bValI = *SkTCast<int*>(&bVal);
+ int aValI = float_bits(aVal);
+ int bValI = float_bits(bVal);
if (sk_float_isnan(aVal) && aValI == bValI) {
foundNaN = true;
} else {
diff --git a/tests/PointTest.cpp b/tests/PointTest.cpp
index 76ac3a1..4981e89 100644
--- a/tests/PointTest.cpp
+++ b/tests/PointTest.cpp
@@ -14,8 +14,8 @@
SkPoint p = { 0, 0 };
SkRect r = { 0, 0, 0, 0 };
- const SkScalar* pPtr = SkTCast<const SkScalar*>(&p);
- const SkScalar* rPtr = SkTCast<const SkScalar*>(&r);
+ const SkScalar* pPtr = reinterpret_cast<const SkScalar*>(&p);
+ const SkScalar* rPtr = reinterpret_cast<const SkScalar*>(&r);
REPORTER_ASSERT(reporter, SkPointPriv::AsScalars(p) == pPtr);
REPORTER_ASSERT(reporter, r.asScalars() == rPtr);
diff --git a/tests/RectangleTextureTest.cpp b/tests/RectangleTextureTest.cpp
index 90b723a..e473ed8 100644
--- a/tests/RectangleTextureTest.cpp
+++ b/tests/RectangleTextureTest.cpp
@@ -55,7 +55,7 @@
// The clear color is a GrColor, our readback is to kRGBA_8888, which may be different.
uint32_t expectedColor0 = 0;
- uint8_t* expectedBytes0 = SkTCast<uint8_t*>(&expectedColor0);
+ uint8_t* expectedBytes0 = reinterpret_cast<uint8_t*>(&expectedColor0);
expectedBytes0[0] = GrColorUnpackR(color0);
expectedBytes0[1] = GrColorUnpackG(color0);
expectedBytes0[2] = GrColorUnpackB(color0);
@@ -70,7 +70,7 @@
rtc->clear(&rect, color1, GrRenderTargetContext::CanClearFullscreen::kNo);
uint32_t expectedColor1 = 0;
- uint8_t* expectedBytes1 = SkTCast<uint8_t*>(&expectedColor1);
+ uint8_t* expectedBytes1 = reinterpret_cast<uint8_t*>(&expectedColor1);
expectedBytes1[0] = GrColorUnpackR(color1);
expectedBytes1[1] = GrColorUnpackG(color1);
expectedBytes1[2] = GrColorUnpackB(color1);