tint/utils: Remove non-const accessors on VectorRef
Nothing uses these, and the mutability of these breaks
const-correctness.
Switch functions that used to return `const utils::Vector<T, N>&`
to returning `utils::VectorRef<T>`. Removes the templated size from the
public interface.
Replace all `const utils::VectorRef<T>&` with `utils::Vector<T>`,
there's no point in using yet another level of pointer indirection.
Change-Id: Ib96e3171500606d9afffbb13f40023552a74fffc
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/113021
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Dan Sinclair <dsinclair@chromium.org>
diff --git a/src/tint/inspector/inspector.cc b/src/tint/inspector/inspector.cc
index 25cde92..ea5b3ed 100644
--- a/src/tint/inspector/inspector.cc
+++ b/src/tint/inspector/inspector.cc
@@ -532,7 +532,7 @@
ResourceBinding::ResourceType::kExternalTexture);
}
-utils::Vector<sem::SamplerTexturePair, 4> Inspector::GetSamplerTextureUses(
+utils::VectorRef<sem::SamplerTexturePair> Inspector::GetSamplerTextureUses(
const std::string& entry_point) {
auto* func = FindEntryPointByName(entry_point);
if (!func) {
diff --git a/src/tint/inspector/inspector.h b/src/tint/inspector/inspector.h
index 49e4bdf..62a1c73 100644
--- a/src/tint/inspector/inspector.h
+++ b/src/tint/inspector/inspector.h
@@ -126,7 +126,7 @@
/// @param entry_point name of the entry point to get information about.
/// @returns vector of all of the sampler/texture sampling pairs that are used
/// by that entry point.
- utils::Vector<sem::SamplerTexturePair, 4> GetSamplerTextureUses(const std::string& entry_point);
+ utils::VectorRef<sem::SamplerTexturePair> GetSamplerTextureUses(const std::string& entry_point);
/// @param entry_point name of the entry point to get information about.
/// @param placeholder the sampler binding point to use for texture-only
diff --git a/src/tint/inspector/inspector_test.cc b/src/tint/inspector/inspector_test.cc
index 05f3c19..83302e2 100644
--- a/src/tint/inspector/inspector_test.cc
+++ b/src/tint/inspector/inspector_test.cc
@@ -12,7 +12,8 @@
// See the License for the specific language governing permissions and
// limitations under the License.
-#include "gtest/gtest.h"
+#include "gmock/gmock.h"
+
#include "src/tint/ast/call_statement.h"
#include "src/tint/ast/disable_validation_attribute.h"
#include "src/tint/ast/id_attribute.h"
@@ -33,16 +34,13 @@
namespace tint::inspector {
namespace {
-// All the tests that descend from InspectorBuilder are expected to define their
-// test state via building up the AST through InspectorBuilder and then generate
-// the program with ::Build.
-// The returned Inspector from ::Build can then be used to test expecations.
+// All the tests that descend from InspectorBuilder are expected to define their test state via
+// building up the AST through InspectorBuilder and then generate the program with ::Build. The
+// returned Inspector from ::Build can then be used to test expectations.
//
-// All the tests that descend from InspectorRunner are expected to define their
-// test state via a WGSL shader, which will be parsed to generate a Program and
-// Inspector in ::Initialize.
-// The returned Inspector from ::Initialize can then be used to test
-// expecations.
+// All the tests that descend from InspectorRunner are expected to define their test state via a
+// WGSL shader, which will be parsed to generate a Program and Inspector in ::Initialize. The
+// returned Inspector from ::Initialize can then be used to test expectations.
class InspectorGetEntryPointTest : public InspectorBuilder, public testing::Test {};
@@ -3202,7 +3200,7 @@
})";
Inspector& inspector = Initialize(shader);
- auto result = inspector.GetSamplerTextureUses("foo");
+ inspector.GetSamplerTextureUses("foo");
ASSERT_TRUE(inspector.has_error()) << inspector.error();
}
@@ -3224,7 +3222,8 @@
auto result_1 = inspector.GetSamplerTextureUses("main");
ASSERT_FALSE(inspector.has_error()) << inspector.error();
- EXPECT_EQ(result_0, result_1);
+ EXPECT_EQ((utils::Vector<tint::sem::SamplerTexturePair, 4>(result_0)),
+ (utils::Vector<tint::sem::SamplerTexturePair, 4>(result_1)));
}
TEST_F(InspectorGetSamplerTextureUsesTest, BothIndirect) {
@@ -3641,7 +3640,7 @@
})";
Inspector& inspector = Initialize(shader);
- auto result = inspector.GetSamplerTextureUses("main");
+ inspector.GetSamplerTextureUses("main");
}
} // namespace
diff --git a/src/tint/ir/builder.cc b/src/tint/ir/builder.cc
index 0f0200d..35e4f70 100644
--- a/src/tint/ir/builder.cc
+++ b/src/tint/ir/builder.cc
@@ -77,7 +77,7 @@
return ir_switch;
}
-Block* Builder::CreateCase(Switch* s, const utils::VectorRef<const ast::CaseSelector*> selectors) {
+Block* Builder::CreateCase(Switch* s, utils::VectorRef<const ast::CaseSelector*> selectors) {
s->cases.Push(Switch::Case{selectors, CreateBlock()});
Block* b = s->cases.Back().start_target;
diff --git a/src/tint/ir/builder.h b/src/tint/ir/builder.h
index 3f2e011..ceabfa6b 100644
--- a/src/tint/ir/builder.h
+++ b/src/tint/ir/builder.h
@@ -78,7 +78,7 @@
/// @param s the switch to create the case into
/// @param selectors the case selectors for the case statement
/// @returns the start block for the case flow node
- Block* CreateCase(Switch* s, const utils::VectorRef<const ast::CaseSelector*> selectors);
+ Block* CreateCase(Switch* s, utils::VectorRef<const ast::CaseSelector*> selectors);
/// Branches the given block to the given flow node.
/// @param from the block to branch from
diff --git a/src/tint/ir/switch.h b/src/tint/ir/switch.h
index 3ea9c3f..7fff0a0 100644
--- a/src/tint/ir/switch.h
+++ b/src/tint/ir/switch.h
@@ -33,7 +33,7 @@
/// A case label in the struct
struct Case {
/// The case selector for this node
- const utils::VectorRef<const ast::CaseSelector*> selectors;
+ utils::Vector<const ast::CaseSelector*, 4> selectors;
/// The start block for the case block.
Block* start_target;
};
diff --git a/src/tint/resolver/resolver.cc b/src/tint/resolver/resolver.cc
index ca66ac2..628bf45 100644
--- a/src/tint/resolver/resolver.cc
+++ b/src/tint/resolver/resolver.cc
@@ -1745,7 +1745,7 @@
return nullptr;
},
[&](const sem::Struct* s) -> const sem::Type* {
- if (auto& tys = s->ConcreteTypes(); !tys.IsEmpty()) {
+ if (auto tys = s->ConcreteTypes(); !tys.IsEmpty()) {
return target_ty ? target_ty : tys[0];
}
return nullptr;
diff --git a/src/tint/resolver/validator.cc b/src/tint/resolver/validator.cc
index e5c7c1a..dcb9b4c 100644
--- a/src/tint/resolver/validator.cc
+++ b/src/tint/resolver/validator.cc
@@ -1811,7 +1811,7 @@
return true;
}
-bool Validator::PipelineStages(const utils::VectorRef<sem::Function*> entry_points) const {
+bool Validator::PipelineStages(utils::VectorRef<sem::Function*> entry_points) const {
auto backtrace = [&](const sem::Function* func, const sem::Function* entry_point) {
if (func != entry_point) {
TraverseCallChain(diagnostics_, entry_point, func, [&](const sem::Function* f) {
@@ -1906,7 +1906,7 @@
return true;
}
-bool Validator::PushConstants(const utils::VectorRef<sem::Function*> entry_points) const {
+bool Validator::PushConstants(utils::VectorRef<sem::Function*> entry_points) const {
for (auto* entry_point : entry_points) {
// State checked and modified by check_push_constant so that it remembers previously seen
// push_constant variables for an entry-point.
@@ -2422,7 +2422,7 @@
const sem::Type* store_ty,
ast::Access access,
ast::AddressSpace address_space,
- const utils::VectorRef<const tint::ast::Attribute*> attributes,
+ utils::VectorRef<const tint::ast::Attribute*> attributes,
const Source& source) const {
if (!AddressSpaceLayout(store_ty, address_space, source)) {
return false;
diff --git a/src/tint/resolver/validator.h b/src/tint/resolver/validator.h
index 0f183bb..9f0664c 100644
--- a/src/tint/resolver/validator.h
+++ b/src/tint/resolver/validator.h
@@ -135,12 +135,12 @@
/// Validates pipeline stages
/// @param entry_points the entry points to the module
/// @returns true on success, false otherwise.
- bool PipelineStages(const utils::VectorRef<sem::Function*> entry_points) const;
+ bool PipelineStages(utils::VectorRef<sem::Function*> entry_points) const;
/// Validates push_constant variables
/// @param entry_points the entry points to the module
/// @returns true on success, false otherwise.
- bool PushConstants(const utils::VectorRef<sem::Function*> entry_points) const;
+ bool PushConstants(utils::VectorRef<sem::Function*> entry_points) const;
/// Validates aliases
/// @param alias the alias to validate
@@ -508,7 +508,7 @@
bool CheckTypeAccessAddressSpace(const sem::Type* store_ty,
ast::Access access,
ast::AddressSpace address_space,
- const utils::VectorRef<const tint::ast::Attribute*> attributes,
+ utils::VectorRef<const tint::ast::Attribute*> attributes,
const Source& source) const;
SymbolTable& symbols_;
diag::List& diagnostics_;
diff --git a/src/tint/sem/function.h b/src/tint/sem/function.h
index 8f21f74..7b5eaff 100644
--- a/src/tint/sem/function.h
+++ b/src/tint/sem/function.h
@@ -135,9 +135,7 @@
/// @returns the list of texture/sampler pairs that this function uses
/// (directly or indirectly).
- const utils::Vector<VariablePair, 8>& TextureSamplerPairs() const {
- return texture_sampler_pairs_;
- }
+ utils::VectorRef<VariablePair> TextureSamplerPairs() const { return texture_sampler_pairs_; }
/// @returns the list of direct calls to functions / builtins made by this
/// function
diff --git a/src/tint/sem/module.h b/src/tint/sem/module.h
index b451c5b..216a2c4 100644
--- a/src/tint/sem/module.h
+++ b/src/tint/sem/module.h
@@ -39,7 +39,7 @@
~Module() override;
/// @returns the dependency-ordered global declarations for the module
- const utils::Vector<const ast::Node*, 64>& DependencyOrderedDeclarations() const {
+ utils::VectorRef<const ast::Node*> DependencyOrderedDeclarations() const {
return dep_ordered_decls_;
}
diff --git a/src/tint/sem/struct.h b/src/tint/sem/struct.h
index 831cf3e..f878854 100644
--- a/src/tint/sem/struct.h
+++ b/src/tint/sem/struct.h
@@ -163,7 +163,7 @@
/// @returns the conversion-rank ordered concrete versions of this abstract structure, or an
/// empty vector if this structure is not abstract.
/// @note only structures returned by builtins may be abstract (e.g. modf, frexp)
- const utils::Vector<const Struct*, 2>& ConcreteTypes() const { return concrete_types_; }
+ utils::VectorRef<const Struct*> ConcreteTypes() const { return concrete_types_; }
private:
ast::Struct const* const declaration_;
diff --git a/src/tint/sem/type.cc b/src/tint/sem/type.cc
index 3d25e7e..51afc12 100644
--- a/src/tint/sem/type.cc
+++ b/src/tint/sem/type.cc
@@ -239,7 +239,7 @@
return kNoConversion;
},
[&](const Struct* from_str) {
- auto& concrete_tys = from_str->ConcreteTypes();
+ auto concrete_tys = from_str->ConcreteTypes();
for (size_t i = 0; i < concrete_tys.Length(); i++) {
if (concrete_tys[i] == to) {
return static_cast<uint32_t>(i + 1);
diff --git a/src/tint/transform/direct_variable_access.cc b/src/tint/transform/direct_variable_access.cc
index d1c1339..26c4aba 100644
--- a/src/tint/transform/direct_variable_access.cc
+++ b/src/tint/transform/direct_variable_access.cc
@@ -228,7 +228,8 @@
// will have the pointer parameters replaced with an array of u32s, used to perform the
// pointer indexing in the variant.
// Function call pointer arguments are replaced with an array of these dynamic indices.
- for (auto* decl : utils::Reverse(sem.Module()->DependencyOrderedDeclarations())) {
+ auto decls = sem.Module()->DependencyOrderedDeclarations();
+ for (auto* decl : utils::Reverse(decls)) {
if (auto* fn = sem.Get<sem::Function>(decl)) {
auto* fn_info = FnInfoFor(fn);
ProcessFunction(fn, fn_info);
diff --git a/src/tint/utils/transform.h b/src/tint/utils/transform.h
index 2faca46..d96a4bb 100644
--- a/src/tint/utils/transform.h
+++ b/src/tint/utils/transform.h
@@ -91,8 +91,7 @@
/// @tparam N the small-array size of the returned Vector
/// @returns a new vector with each element of the source vector transformed by `transform`.
template <size_t N, typename IN, typename TRANSFORMER>
-auto Transform(const VectorRef<IN>& in, TRANSFORMER&& transform)
- -> Vector<decltype(transform(in[0])), N> {
+auto Transform(VectorRef<IN> in, TRANSFORMER&& transform) -> Vector<decltype(transform(in[0])), N> {
const auto count = in.Length();
Vector<decltype(transform(in[0])), N> result;
result.Reserve(count);
@@ -108,7 +107,7 @@
/// @tparam N the small-array size of the returned Vector
/// @returns a new vector with each element of the source vector transformed by `transform`.
template <size_t N, typename IN, typename TRANSFORMER>
-auto Transform(const VectorRef<IN>& in, TRANSFORMER&& transform)
+auto Transform(VectorRef<IN> in, TRANSFORMER&& transform)
-> Vector<decltype(transform(in[0], 1u)), N> {
const auto count = in.Length();
Vector<decltype(transform(in[0], 1u)), N> result;
diff --git a/src/tint/utils/unique_vector.h b/src/tint/utils/unique_vector.h
index bda090c..6cb8f88 100644
--- a/src/tint/utils/unique_vector.h
+++ b/src/tint/utils/unique_vector.h
@@ -87,8 +87,8 @@
/// @returns an iterator to the end of the reversed vector
auto rend() const { return vector.rend(); }
- /// @returns a const reference to the internal vector
- operator const Vector<T, N>&() const { return vector; }
+ /// @returns a reference to the internal vector
+ operator VectorRef<T>() const { return vector; }
/// @returns the std::move()'d vector.
/// @note The UniqueVector must not be used after calling this method
diff --git a/src/tint/utils/unique_vector_test.cc b/src/tint/utils/unique_vector_test.cc
index 9b015c2..c198615 100644
--- a/src/tint/utils/unique_vector_test.cc
+++ b/src/tint/utils/unique_vector_test.cc
@@ -94,11 +94,11 @@
unique_vec.Add(1);
unique_vec.Add(2);
- const utils::Vector<int, 4>& vec = unique_vec;
- EXPECT_EQ(vec.Length(), 3u);
+ utils::VectorRef<int> ref = unique_vec;
+ EXPECT_EQ(ref.Length(), 3u);
EXPECT_EQ(unique_vec.IsEmpty(), false);
int i = 0;
- for (auto n : vec) {
+ for (auto n : ref) {
EXPECT_EQ(n, i);
i++;
}
diff --git a/src/tint/utils/vector.h b/src/tint/utils/vector.h
index 2371136..7e2c271 100644
--- a/src/tint/utils/vector.h
+++ b/src/tint/utils/vector.h
@@ -680,11 +680,11 @@
Vector(Ts...) -> Vector<VectorCommonType<Ts...>, sizeof...(Ts)>;
/// VectorRef is a weak reference to a Vector, used to pass vectors as parameters, avoiding copies
-/// between the caller and the callee. VectorRef can accept a Vector of any 'N' value, decoupling
-/// the caller's vector internal size from the callee's vector size. A VectorRef tracks the usage of
-/// moves either side of the call. If at the call site, a Vector argument is moved to a VectorRef
-/// parameter, and within the callee, the VectorRef parameter is moved to a Vector, then the Vector
-/// heap allocation will be moved. For example:
+/// between the caller and the callee, or as an non-static sized accessor on a vector. VectorRef can
+/// accept a Vector of any 'N' value, decoupling the caller's vector internal size from the callee's
+/// vector size. A VectorRef tracks the usage of moves either side of the call. If at the call site,
+/// a Vector argument is moved to a VectorRef parameter, and within the callee, the VectorRef
+/// parameter is moved to a Vector, then the Vector heap allocation will be moved. For example:
///
/// ```
/// void func_a() {
@@ -698,6 +698,8 @@
/// Vector<std::string, 2> vec(std::move(vec_ref));
/// }
/// ```
+///
+/// Aside from this move pattern, a VectorRef provides an immutable reference to the Vector.
template <typename T>
class VectorRef {
/// The slice type used by this vector reference
@@ -710,6 +712,9 @@
}
public:
+ /// Type of `T`.
+ using value_type = T;
+
/// Constructor - empty reference
VectorRef() : slice_(EmptySlice()) {}
@@ -805,39 +810,21 @@
bool IsEmpty() const { return slice_.len == 0; }
/// @returns a reference to the first element in the vector
- T& Front() { return slice_.Front(); }
-
- /// @returns a reference to the first element in the vector
const T& Front() const { return slice_.Front(); }
/// @returns a reference to the last element in the vector
- T& Back() { return slice_.Back(); }
-
- /// @returns a reference to the last element in the vector
const T& Back() const { return slice_.Back(); }
/// @returns a pointer to the first element in the vector
- T* begin() { return slice_.begin(); }
-
- /// @returns a pointer to the first element in the vector
const T* begin() const { return slice_.begin(); }
/// @returns a pointer to one past the last element in the vector
- T* end() { return slice_.end(); }
-
- /// @returns a pointer to one past the last element in the vector
const T* end() const { return slice_.end(); }
/// @returns a reverse iterator starting with the last element in the vector
- auto rbegin() { return slice_.rbegin(); }
-
- /// @returns a reverse iterator starting with the last element in the vector
auto rbegin() const { return slice_.rbegin(); }
/// @returns the end for a reverse iterator
- auto rend() { return slice_.rend(); }
-
- /// @returns the end for a reverse iterator
auto rend() const { return slice_.rend(); }
private:
@@ -907,7 +894,7 @@
/// @param vec the vector reference
/// @return the std::ostream so calls can be chained
template <typename T>
-inline std::ostream& operator<<(std::ostream& o, const utils::VectorRef<T>& vec) {
+inline std::ostream& operator<<(std::ostream& o, utils::VectorRef<T> vec) {
o << "[";
bool first = true;
for (auto& el : vec) {
diff --git a/src/tint/utils/vector_test.cc b/src/tint/utils/vector_test.cc
index 74d15b7..ed9a97a 100644
--- a/src/tint/utils/vector_test.cc
+++ b/src/tint/utils/vector_test.cc
@@ -2073,15 +2073,6 @@
TEST(TintVectorRefTest, FrontBack) {
Vector<std::string, 3> vec{"front", "mid", "back"};
- VectorRef<std::string> vec_ref(vec);
- static_assert(!std::is_const_v<std::remove_reference_t<decltype(vec_ref.Front())>>);
- static_assert(!std::is_const_v<std::remove_reference_t<decltype(vec_ref.Back())>>);
- EXPECT_EQ(vec_ref.Front(), "front");
- EXPECT_EQ(vec_ref.Back(), "back");
-}
-
-TEST(TintVectorRefTest, ConstFrontBack) {
- Vector<std::string, 3> vec{"front", "mid", "back"};
const VectorRef<std::string> vec_ref(vec);
static_assert(std::is_const_v<std::remove_reference_t<decltype(vec_ref.Front())>>);
static_assert(std::is_const_v<std::remove_reference_t<decltype(vec_ref.Back())>>);
@@ -2091,15 +2082,6 @@
TEST(TintVectorRefTest, BeginEnd) {
Vector<std::string, 3> vec{"front", "mid", "back"};
- VectorRef<std::string> vec_ref(vec);
- static_assert(!std::is_const_v<std::remove_reference_t<decltype(*vec_ref.begin())>>);
- static_assert(!std::is_const_v<std::remove_reference_t<decltype(*vec_ref.end())>>);
- EXPECT_EQ(vec_ref.begin(), &vec[0]);
- EXPECT_EQ(vec_ref.end(), &vec[0] + 3);
-}
-
-TEST(TintVectorRefTest, ConstBeginEnd) {
- Vector<std::string, 3> vec{"front", "mid", "back"};
const VectorRef<std::string> vec_ref(vec);
static_assert(std::is_const_v<std::remove_reference_t<decltype(*vec_ref.begin())>>);
static_assert(std::is_const_v<std::remove_reference_t<decltype(*vec_ref.end())>>);
diff --git a/src/tint/writer/spirv/builder_type_test.cc b/src/tint/writer/spirv/builder_type_test.cc
index 4377c42..4ab8aa0 100644
--- a/src/tint/writer/spirv/builder_type_test.cc
+++ b/src/tint/writer/spirv/builder_type_test.cc
@@ -442,8 +442,8 @@
auto* arr_arr_mat2x3_f32 =
ty.array(ty.array(ty.mat2x3<f32>(), 1_u), 2_u); // Doubly nested array
auto* arr_arr_mat2x3_f16 =
- ty.array(ty.array(ty.mat2x3<f16>(), 1_u), 2_u); // Doubly nested array
- auto* rtarr_mat4x4 = ty.array(ty.mat4x4<f32>()); // Runtime array
+ ty.array(ty.array(ty.mat2x3<f16>(), 1_u), 2_u); // Doubly nested array
+ auto* rtarr_mat4x4 = ty.array(ty.mat4x4<f32>()); // Runtime array
auto* s = Structure(
"S", utils::Vector{
