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>

src/tint/inspector/inspector.cc

@@ -532,7 +532,7 @@ std::vector<ResourceBinding> Inspector::GetExternalTextureResourceBindings(
                                       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) {

src/tint/inspector/inspector.h

@@ -126,7 +126,7 @@ class Inspector {
     /// @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

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 @@ using namespace tint::number_suffixes;  // NOLINT
 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 @@ fn main(@location(0) fragUV: vec2<f32>,
 })";
 
     Inspector& inspector = Initialize(shader);
-    auto result = inspector.GetSamplerTextureUses("foo");
+    inspector.GetSamplerTextureUses("foo");
     ASSERT_TRUE(inspector.has_error()) << inspector.error();
 }
 
@@ -3224,7 +3222,8 @@ fn main(@location(0) fragUV: vec2<f32>,
     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 @@ fn main(@location(0) fragUV: vec2<f32>,
 })";
 
     Inspector& inspector = Initialize(shader);
-    auto result = inspector.GetSamplerTextureUses("main");
+    inspector.GetSamplerTextureUses("main");
 }
 
 }  // namespace

src/tint/ir/builder.cc

@@ -77,7 +77,7 @@ Switch* Builder::CreateSwitch(const ast::SwitchStatement* stmt) {
     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;

src/tint/ir/builder.h

@@ -78,7 +78,7 @@ class Builder {
     /// @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

src/tint/ir/switch.h

@@ -33,7 +33,7 @@ class Switch : public Castable<Switch, FlowNode> {
     /// 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;
     };

src/tint/resolver/resolver.cc

@@ -1745,7 +1745,7 @@ const sem::Type* Resolver::ConcreteType(const sem::Type* ty,
             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;

src/tint/resolver/validator.cc

@@ -1811,7 +1811,7 @@ bool Validator::Matrix(const sem::Matrix* ty, const Source& source) const {
     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 @@ bool Validator::PipelineStages(const utils::VectorRef<sem::Function*> entry_poin
     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 @@ bool Validator::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 {
     if (!AddressSpaceLayout(store_ty, address_space, source)) {
         return false;

src/tint/resolver/validator.h

@@ -135,12 +135,12 @@ class Validator {
     /// 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 @@ class Validator {
     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_;

src/tint/sem/function.h

@@ -135,9 +135,7 @@ class Function final : public Castable<Function, CallTarget> {
 
     /// @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

src/tint/sem/module.h

@@ -39,7 +39,7 @@ class Module final : public Castable<Module, Node> {
     ~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_;
     }
 

src/tint/sem/struct.h

@@ -163,7 +163,7 @@ class Struct final : public Castable<Struct, Type> {
     /// @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_;

src/tint/sem/type.cc

@@ -239,7 +239,7 @@ uint32_t Type::ConversionRank(const Type* from, const Type* to) {
             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);

src/tint/transform/direct_variable_access.cc

@@ -228,7 +228,8 @@ struct DirectVariableAccess::State {
         // 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);

src/tint/utils/transform.h

@@ -91,8 +91,7 @@ auto Transform(const Vector<IN, N>& in, TRANSFORMER&& transform)
 /// @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 @@ auto Transform(const VectorRef<IN>& in, TRANSFORMER&& transform)
 /// @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;

src/tint/utils/unique_vector.h

@@ -87,8 +87,8 @@ struct UniqueVector {
     /// @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

src/tint/utils/unique_vector_test.cc

@@ -94,11 +94,11 @@ TEST(UniqueVectorTest, AsVector) {
     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++;
     }

src/tint/utils/vector.h

@@ -680,11 +680,11 @@ template <typename... Ts>
 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(Ts...) -> Vector<VectorCommonType<Ts...>, sizeof...(Ts)>;
 ///        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 @@ class VectorRef {
     }
 
   public:
+    /// Type of `T`.
+    using value_type = T;
+
     /// Constructor - empty reference
     VectorRef() : slice_(EmptySlice()) {}
 
@@ -804,39 +809,21 @@ class VectorRef {
     /// @returns true if the vector is empty.
     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(); }
 
@@ -907,7 +894,7 @@ inline std::ostream& operator<<(std::ostream& o, const utils::Vector<T, N>& vec)
 /// @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) {

src/tint/utils/vector_test.cc

@@ -2072,15 +2072,6 @@ TEST(TintVectorRefTest, IsEmpty) {
 }
 
 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())>>);
@@ -2090,15 +2081,6 @@ TEST(TintVectorRefTest, ConstFrontBack) {
 }
 
 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())>>);

src/tint/writer/spirv/builder_type_test.cc

@@ -442,8 +442,8 @@ TEST_F(BuilderTest_Type, GenerateStruct_DecoratedMembers_ArraysOfMatrix) {
     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{