From a31d89d6a330565858c1d61421015ceb7bdc3afb Mon Sep 17 00:00:00 2001 From: Ben Clayton Date: Tue, 6 Dec 2022 15:43:40 +0000 Subject: [PATCH] tint/utils: Add support for unsafe pointer downcasts Bug: tint:1779 Change-Id: Icfd27680edf7dfaedbfb70f25641dc762d23f42a Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/113020 Auto-Submit: Ben Clayton Reviewed-by: Dan Sinclair Kokoro: Kokoro Commit-Queue: Ben Clayton --- src/tint/utils/vector.h | 80 +++++++++++++++++++++++++---------- src/tint/utils/vector_test.cc | 52 ++++++++++++++++------- 2 files changed, 94 insertions(+), 38 deletions(-) diff --git a/src/tint/utils/vector.h b/src/tint/utils/vector.h index cefd536a1e..2371136fc4 100644 --- a/src/tint/utils/vector.h +++ b/src/tint/utils/vector.h @@ -106,12 +106,20 @@ struct Slice { auto rend() const { return std::reverse_iterator(begin()); } }; +/// Mode enumerator for ReinterpretSlice +enum class ReinterpretMode { + /// Only upcasts of pointers are permitted + kSafe, + /// Potentially unsafe downcasts of pointers are also permitted + kUnsafe, +}; + namespace detail { /// Private implementation of tint::utils::CanReinterpretSlice. /// Specialized for the case of TO equal to FROM, which is the common case, and avoids inspection of /// the base classes, which can be troublesome if the slice is of an incomplete type. -template +template struct CanReinterpretSlice { /// True if a slice of FROM can be reinterpreted as a slice of TO static constexpr bool value = @@ -122,13 +130,14 @@ struct CanReinterpretSlice { !std::is_const_v>)&& // // TO and FROM are both Castable IsCastable, std::remove_pointer_t> && // - // FROM is of, or derives from TO - traits::IsTypeOrDerived, std::remove_pointer_t>; + // MODE is kUnsafe, or FROM is of, or derives from TO + (MODE == ReinterpretMode::kUnsafe || + traits::IsTypeOrDerived, std::remove_pointer_t>); }; /// Specialization of 'CanReinterpretSlice' for when TO and FROM are equal types. -template -struct CanReinterpretSlice { +template +struct CanReinterpretSlice { /// Always `true` as TO and FROM are the same type. static constexpr bool value = true; }; @@ -140,16 +149,16 @@ struct CanReinterpretSlice { /// CastableBase, and the pointee type of `TO` is of the same type as, or is an ancestor of the /// pointee type of `FROM`. Vectors of non-`const` Castable pointers can be converted to a vector of /// `const` Castable pointers. -template -static constexpr bool CanReinterpretSlice = detail::CanReinterpretSlice::value; +template +static constexpr bool CanReinterpretSlice = detail::CanReinterpretSlice::value; /// Reinterprets `const Slice*` as `const Slice*` /// @param slice a pointer to the slice to reinterpret /// @returns the reinterpreted slice /// @see CanReinterpretSlice -template +template const Slice* ReinterpretSlice(const Slice* slice) { - static_assert(CanReinterpretSlice); + static_assert(CanReinterpretSlice); return Bitcast*>(slice); } @@ -157,9 +166,9 @@ const Slice* ReinterpretSlice(const Slice* slice) { /// @param slice a pointer to the slice to reinterpret /// @returns the reinterpreted slice /// @see CanReinterpretSlice -template +template Slice* ReinterpretSlice(Slice* slice) { - static_assert(CanReinterpretSlice); + static_assert(CanReinterpretSlice); return Bitcast*>(slice); } @@ -230,15 +239,21 @@ class Vector { /// Copy constructor with covariance / const conversion /// @param other the vector to copy /// @see CanReinterpretSlice for rules about conversion - template >> + template >> Vector(const Vector& other) { // NOLINT(runtime/explicit) - Copy(*ReinterpretSlice(&other.impl_.slice)); + Copy(*ReinterpretSlice(&other.impl_.slice)); } /// Move constructor with covariance / const conversion /// @param other the vector to move /// @see CanReinterpretSlice for rules about conversion - template >> + template >> Vector(Vector&& other) { // NOLINT(runtime/explicit) MoveOrCopy(VectorRef(std::move(other))); } @@ -701,6 +716,11 @@ class VectorRef { /// Constructor VectorRef(EmptyType) : slice_(EmptySlice()) {} // NOLINT(runtime/explicit) + /// Constructor from a Slice + /// @param slice the slice + VectorRef(Slice& slice) // NOLINT(runtime/explicit) + : slice_(slice) {} + /// Constructor from a Vector /// @param vector the vector to create a reference of template @@ -729,29 +749,37 @@ class VectorRef { /// Copy constructor with covariance / const conversion /// @param other the other vector reference - template >> + template >> VectorRef(const VectorRef& other) // NOLINT(runtime/explicit) - : slice_(*ReinterpretSlice(&other.slice_)) {} + : slice_(*ReinterpretSlice(&other.slice_)) {} /// Move constructor with covariance / const conversion /// @param other the vector reference - template >> + template >> VectorRef(VectorRef&& other) // NOLINT(runtime/explicit) - : slice_(*ReinterpretSlice(&other.slice_)), can_move_(other.can_move_) {} + : slice_(*ReinterpretSlice(&other.slice_)), + can_move_(other.can_move_) {} /// Constructor from a Vector with covariance / const conversion /// @param vector the vector to create a reference of /// @see CanReinterpretSlice for rules about conversion - template >> + template >> VectorRef(Vector& vector) // NOLINT(runtime/explicit) - : slice_(*ReinterpretSlice(&vector.impl_.slice)) {} + : slice_(*ReinterpretSlice(&vector.impl_.slice)) {} /// Constructor from a moved Vector with covariance / const conversion /// @param vector the vector to create a reference of /// @see CanReinterpretSlice for rules about conversion - template >> + template >> VectorRef(Vector&& vector) // NOLINT(runtime/explicit) - : slice_(*ReinterpretSlice(&vector.impl_.slice)), can_move_(vector.impl_.CanMove()) {} + : slice_(*ReinterpretSlice(&vector.impl_.slice)), + can_move_(vector.impl_.CanMove()) {} /// Index operator /// @param i the element index. Must be less than `len`. @@ -765,6 +793,14 @@ class VectorRef { /// be made size_t Capacity() const { return slice_.cap; } + /// @return a reinterpretation of this VectorRef as elements of type U. + /// @note this is doing a reinterpret_cast of elements. It is up to the caller to ensure that + /// this is a safe operation. + template + VectorRef ReinterpretCast() const { + return {*ReinterpretSlice(&slice_)}; + } + /// @returns true if the vector is empty. bool IsEmpty() const { return slice_.len == 0; } diff --git a/src/tint/utils/vector_test.cc b/src/tint/utils/vector_test.cc index 8e6fa7a51c..74d15b774f 100644 --- a/src/tint/utils/vector_test.cc +++ b/src/tint/utils/vector_test.cc @@ -79,22 +79,30 @@ static_assert(std::is_same_v, const C1*>); static_assert(std::is_same_v, const C1*>); static_assert(std::is_same_v, const C1*>); -static_assert(CanReinterpretSlice, "apply const"); -static_assert(!CanReinterpretSlice, "remove const"); -static_assert(CanReinterpretSlice, "up cast"); -static_assert(CanReinterpretSlice, "up cast"); -static_assert(CanReinterpretSlice, "up cast, apply const"); -static_assert(!CanReinterpretSlice, "up cast, remove const"); -static_assert(!CanReinterpretSlice, "down cast"); -static_assert(!CanReinterpretSlice, "down cast"); -static_assert(!CanReinterpretSlice, "down cast, apply const"); -static_assert(!CanReinterpretSlice, "down cast, remove const"); -static_assert(!CanReinterpretSlice, "down cast, apply const"); -static_assert(!CanReinterpretSlice, "down cast, remove const"); -static_assert(!CanReinterpretSlice, "sideways cast"); -static_assert(!CanReinterpretSlice, "sideways cast"); -static_assert(!CanReinterpretSlice, "sideways cast, apply const"); -static_assert(!CanReinterpretSlice, "sideways cast, remove const"); +static_assert(CanReinterpretSlice, "apply const"); +static_assert(!CanReinterpretSlice, "remove const"); +static_assert(CanReinterpretSlice, "up cast"); +static_assert(CanReinterpretSlice, "up cast"); +static_assert(CanReinterpretSlice, "up cast, apply const"); +static_assert(!CanReinterpretSlice, + "up cast, remove const"); +static_assert(!CanReinterpretSlice, "down cast"); +static_assert(!CanReinterpretSlice, "down cast"); +static_assert(!CanReinterpretSlice, + "down cast, apply const"); +static_assert(!CanReinterpretSlice, + "down cast, remove const"); +static_assert(!CanReinterpretSlice, + "down cast, apply const"); +static_assert(!CanReinterpretSlice, + "down cast, remove const"); +static_assert(!CanReinterpretSlice, "sideways cast"); +static_assert(!CanReinterpretSlice, + "sideways cast"); +static_assert(!CanReinterpretSlice, + "sideways cast, apply const"); +static_assert(!CanReinterpretSlice, + "sideways cast, remove const"); //////////////////////////////////////////////////////////////////////////////// // TintVectorTest @@ -2001,6 +2009,18 @@ TEST(TintVectorRefTest, MoveVector_UpcastAndAddConst) { EXPECT_TRUE(AllExternallyHeld(vec_b)); // Moved, not copied } +TEST(TintVectorRefTest, MoveVector_ReinterpretCast) { + C2a c2a; + C2b c2b; + Vector vec_a{&c2a, &c2b}; + VectorRef vec_ref(std::move(vec_a)); // Move + EXPECT_EQ(vec_ref[0], &c2a); + EXPECT_EQ(vec_ref[1], &c2b); + VectorRef reinterpret = vec_ref.ReinterpretCast(); + EXPECT_EQ(reinterpret[0], &c2a); + EXPECT_EQ(reinterpret[1], &c2b); +} + TEST(TintVectorRefTest, Index) { Vector vec{"one", "two"}; VectorRef vec_ref(vec);