From 4ea2d96cfb697079d240db5ed4ad41fbd0c7730d Mon Sep 17 00:00:00 2001 From: Jim Blandy Date: Thu, 30 Sep 2021 05:56:09 -0700 Subject: [PATCH] Various doc fixes. (#1440) --- src/back/msl/writer.rs | 16 ++++++++++++++++ src/lib.rs | 35 +++++++++++++++++++++-------------- src/proc/namer.rs | 15 +++++++++++++++ src/proc/typifier.rs | 4 ++-- 4 files changed, 54 insertions(+), 16 deletions(-) diff --git a/src/back/msl/writer.rs b/src/back/msl/writer.rs index a34fbfdd0e..efe58d3dd5 100644 --- a/src/back/msl/writer.rs +++ b/src/back/msl/writer.rs @@ -14,6 +14,9 @@ use std::{ type BackendResult = Result<(), Error>; const NAMESPACE: &str = "metal"; +// The name of the array member of the Metal struct types we generate to +// represent Naga `Array` types. See the comments in `Writer::write_type_defs` +// for details. const WRAPPED_ARRAY_FIELD: &str = "inner"; // This is a hack: we need to pass a pointer to an atomic, // but generally the backend isn't putting "&" in front of every pointer. @@ -1771,6 +1774,19 @@ impl Writer { } let name = &self.names[&NameKey::Type(handle)]; match ty.inner { + // Naga IR can pass around arrays by value, but Metal, following + // C++, performs an array-to-pointer conversion (C++ [conv.array]) + // on expressions of array type, so assigning the array by value + // isn't possible. However, Metal *does* assign structs by + // value. So in our Metal output, we wrap all array types in + // synthetic struct types: + // + // struct type1 { + // float inner[10] + // }; + // + // Then we carefully include `.inner` (`WRAPPED_ARRAY_FIELD`) in + // any expression that actually wants access to the array. crate::TypeInner::Array { base, size, diff --git a/src/lib.rs b/src/lib.rs index bc0681d9f6..97b5b9f52c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -573,6 +573,10 @@ pub enum TypeInner { Atomic { kind: ScalarKind, width: Bytes }, /// Pointer to another type. /// + /// Pointers to scalars and vectors should be treated as equivalent to + /// [`ValuePointer`] types. Use the [`TypeInner::equivalent`] method to + /// compare types in a way that treats pointers correctly. + /// /// ## Pointers to non-`SIZED` types /// /// The `base` type of a pointer may be a non-[`SIZED`] type like a @@ -590,6 +594,7 @@ pub enum TypeInner { /// [`DATA`]: valid::TypeFlags::DATA /// [`Array`]: TypeInner::Array /// [`Struct`]: TypeInner::Struct + /// [`ValuePointer`]: TypeInner::ValuePointer /// [`GlobalVariable`]: Expression::GlobalVariable /// [`AccessIndex`]: Expression::AccessIndex Pointer { @@ -597,11 +602,17 @@ pub enum TypeInner { class: StorageClass, }, - /// Pointer to a value. + /// Pointer to a scalar or vector. /// - /// A `ValuePointer` type is equivalent to a `Pointer` to a `Scalar` or `Vector`. - /// This is for use in [`TypeResolution::Value`] variants. + /// A `ValuePointer` type is equivalent to a `Pointer` whose `base` is a + /// `Scalar` or `Vector` type. This is for use in [`TypeResolution::Value`] + /// variants; see the documentation for [`TypeResolution`] for details. /// + /// Use the [`TypeInner::equivalent`] method to compare types that could be + /// pointers, to ensure that `Pointer` and `ValuePointer` types are + /// recognized as equivalent. + /// + /// [`TypeResolution`]: proc::TypeResolution /// [`TypeResolution::Value`]: proc::TypeResolution::Value ValuePointer { size: Option, @@ -1008,17 +1019,12 @@ pub enum Expression { /// Indexing a [`Vector`] or [`Array`] produces a value of its element type. /// Indexing a [`Matrix`] produces a [`Vector`]. /// - /// Indexing a [`Pointer`] to an [`Array`] produces a [`Pointer`] to its - /// `base` type, taking on the `Pointer`'s storage class. - /// - /// Indexing a [`Pointer`] to a [`Vector`] produces a [`ValuePointer`] whose - /// size is `None`, taking on the [`Vector`]'s scalar kind and width and the - /// [`Pointer`]'s storage class. - /// - /// Indexing a [`Pointer`] to a [`Matrix`] produces a [`ValuePointer`] for a - /// column of the matrix: its size is the matrix's height, its `kind` is - /// [`Float`], and it inherits the [`Matrix`]'s width and the [`Pointer`]'s - /// storage class. + /// Indexing a [`Pointer`] to any of the above produces a pointer to the + /// element/component type, in the same [`class`]. In the case of [`Array`], + /// the result is an actual [`Pointer`], but for vectors and matrices, there + /// may not be any type in the arena representing the component's type, so + /// those produce [`ValuePointer`] types equivalent to the appropriate + /// [`Pointer`]. /// /// ## Dynamic indexing restrictions /// @@ -1043,6 +1049,7 @@ pub enum Expression { /// [`Matrix`]: TypeInner::Matrix /// [`Array`]: TypeInner::Array /// [`Pointer`]: TypeInner::Pointer + /// [`class`]: TypeInner::Pointer::class /// [`ValuePointer`]: TypeInner::ValuePointer /// [`Float`]: ScalarKind::Float Access { diff --git a/src/proc/namer.rs b/src/proc/namer.rs index 0c2038cbfc..90c91e503d 100644 --- a/src/proc/namer.rs +++ b/src/proc/namer.rs @@ -28,6 +28,12 @@ pub struct Namer { } impl Namer { + /// Return a form of `string` suitable for use as the base of an identifier. + /// + /// Retain only alphanumeric and `_` characters. Drop leading digits. Ensure + /// that the string does not end with a digit, so we can attach numeric + /// suffixes without merging. Avoid prefixes in + /// [`Namer::reserved_prefixes`]. fn sanitize(&self, string: &str) -> String { let mut base = string .chars() @@ -50,6 +56,15 @@ impl Namer { base } + /// Return a new identifier based on `label_raw`. + /// + /// The result: + /// - is a valid identifier even if `label_raw` is not + /// - conflicts with no keywords listed in `Namer::keywords`, and + /// - is different from any identifier previously constructed by this + /// `Namer`. + /// + /// Guarantee uniqueness by applying a numeric suffix when necessary. pub fn call(&mut self, label_raw: &str) -> String { let base = self.sanitize(label_raw); match self.unique.entry(base) { diff --git a/src/proc/typifier.rs b/src/proc/typifier.rs index 9bca13e481..217eeb1e2d 100644 --- a/src/proc/typifier.rs +++ b/src/proc/typifier.rs @@ -4,8 +4,8 @@ use thiserror::Error; /// The result of computing an expression's type. /// -/// This is the type returned by [`ResolveContext::resolve`] to represent the type -/// it ascribes to some expression. +/// This is the (Rust) type returned by [`ResolveContext::resolve`] to represent +/// the (Naga) type it ascribes to some expression. /// /// You might expect such a function to simply return a `Handle`. However, /// we want type resolution to be a read-only process, and that would limit the