tint: Clean up legacy type cruft
Fix TODOs dating back to when types were an AST / SEM hybrid concept.
Bring the `arch.md` to reflect how things work today.
Bug: tint:724
Change-Id: I6bf4174158cf490f2839aeed78164b66e3410f27
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/96141
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
diff --git a/docs/tint/arch.md b/docs/tint/arch.md
index 204ff64..704e7c4 100644
--- a/docs/tint/arch.md
+++ b/docs/tint/arch.md
@@ -16,13 +16,13 @@
┏━━━━━━━━━━━━━━━━━┻━━━━━━━━━━━━━━━━━┓
┃ ProgramBuilder ┃
┃ (mutable) ┃
- ┏━━━━━━━━━━━━►┫ ┏━━━━━┓ ┏━━━━━━━┓ ┏━━━━━━━━━┓ ┃
- ┃ ┃ ┃ AST ┃ ┃ Types ┃ ┃ Symbols ┃ ┃
- ┃ ┃ ┗━━━━━┛ ┗━━━━━━━┛ ┗━━━━━━━━━┛ ┃
+ ┏━━━━━━━━━━━━►┫ ┏━━━━━┓ ┏━━━━━━━━━┓ ┃
+ ┃ ┃ ┃ AST ┃ ┃ Symbols ┃ ┃
+ ┃ ┃ ┗━━━━━┛ ┗━━━━━━━━━┛ ┃
┃ ┗━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━┛
┃ ▼
┃ ┌┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┃┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┐
- ▲ ┆ Build ▼ ┆
+ ▲ ┆ Resolve ▼ ┆
┏━━━┻━━━┓ ┆ ┏━━━━━━━━┻━━━━━━━━┓ ┆
┃ Clone ┃ ┆ ┃ Resolver ┃ ┆
┗━━━┳━━━┛ ┆ ┗━━━━━━━━━━━━━━━━━┛ ┆
@@ -31,9 +31,9 @@
┃ ┏━━━━━━━━━━━━━━━━━━━━━━━┻━━━━━━━━━━━━━━━━━━━━━━┓
┃ ┃ Program ┃
┃ ┃ (immutable) ┃
- ┣━━━━━━◄┫ ┏━━━━━┓ ┏━━━━━━━┓ ┏━━━━━━━━━━┓ ┏━━━━━━━━━┓ ┃
- ┃ ┃ ┃ AST ┃ ┃ Types ┃ ┃ Semantic ┃ ┃ Symbols ┃ ┃
- ┃ ┃ ┗━━━━━┛ ┗━━━━━━━┛ ┗━━━━━━━━━━┛ ┗━━━━━━━━━┛ ┃
+ ┣━━━━━━◄┫ ┏━━━━━┓ ┏━━━━━━━━━━┓ ┏━━━━━━━━━┓ ┃
+ ┃ ┃ ┃ AST ┃ ┃ Semantic ┃ ┃ Symbols ┃ ┃
+ ┃ ┃ ┗━━━━━┛ ┗━━━━━━━━━━┛ ┗━━━━━━━━━┛ ┃
┃ ┗━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━┛
▲ ▼
┏━━━━━┻━━━━━┓ ┃ ┏━━━━━━━━━━━┓
@@ -56,16 +56,16 @@
## Reader
Readers are responsible for parsing a shader program and populating a
-`ProgramBuilder` with the parsed AST, type and symbol information.
+`ProgramBuilder` with the parsed AST and symbol information.
The WGSL reader is a recursive descent parser. It closely follows the WGSL
grammar in the naming of the parse methods.
## ProgramBuilder
-A `ProgramBuilder` is the primary interface to construct an immutable `Program`.
-There are a number of methods exposed which make creating of the `Program`
-simpler. A `ProgramBuilder` can only be used once, and must be discarded after
+A `ProgramBuilder` is the interface to construct an immutable `Program`.
+There are a large number of helper methods for simplifying the creation of the
+AST nodes. A `ProgramBuilder` can only be used once, and must be discarded after
the `Program` is constructed.
A `Program` is built from the `ProgramBuilder` by `std::move()`ing the
@@ -73,8 +73,9 @@
so the produced `Program` will contain all the needed semantic information.
At any time before building the `Program`, `ProgramBuilder::IsValid()` may be
-called to ensure the AST is **structurally** correct. This checks that things
-like `if` statements have a condition and body attached.
+called to ensure that no error diagnostics have been raised during the
+construction of the AST. This includes parser syntax errors, but not semantic
+validation which happens during the `Resolve` phase.
If further changes to the `Program` are needed (say via a `Transform`) then a
new `ProgramBuilder` can be produced by cloning the `Program` into a new
@@ -88,30 +89,14 @@
encode the syntactic structure of the WGSL program.
The root of the AST is the `ast::Module` class which holds each of the declared
-functions, variables and user defined types (type aliases and structures).
+functions, variables and user declared types (type aliases and structures).
Each `ast::Node` represents a **single** part of the program's source, and so
`ast::Node`s are not shared.
The AST does not perform any verification of its content. For example, the
-`ast::StrideAttribute` node has numeric stride parameter, which is a count of
-the number of bytes from the start of one array element to the start of the
-next. The AST node itself does not constrain the set of stride values that you
-can set, aside from storing it as an unsigned integer.
-
-## Types
-
-Types are constructed during the Reader and resolution phases, and are
-held by the `Program` or `ProgramBuilder`. AST and semantic nodes can both
-reference types.
-
-Each `type::Type` node **uniquely** represents a particular spelling of a WGSL
-type within the program, so you can compare `type::Type*` pointers to check for
-equivalence of type expressions.
-For example, there is only one `type::Type` node for the `i32` type, no matter
-how many times it is mentioned in the source program.
-However, if `MyI32` is a type alias for `i32`, then they will have two different
-type nodes.
+`ast::Array` node has numeric size parameter, which is not validated to be
+within the WGSL specification limits until validation of the `Resolver`.
## Semantic information
@@ -133,6 +118,32 @@
Unlike `ast::Node`s, semantic nodes may not necessarily form a directed acyclic
graph, and the semantic graph may contain diamonds.
+## Types
+
+AST types are regular AST nodes, in that they uniquely represent a single part
+of the parsed source code. Unlike semantic types, identical AST types are not
+de-duplicated as they refer to the source usage of the type.
+
+Semantic types are constructed during the `Resolver` phase, and are held by
+the `Program` or `ProgramBuilder`.
+
+Each `sem::Type` node **uniquely** represents a particular WGSL type within the
+program, so you can compare `type::Type*` pointers to check for type
+equivalence. For example, a `Program` will only hold one instance of the
+`sem::I32` semantic type, no matter how many times an `i32` is mentioned in the
+source program.
+
+WGSL type aliases resolve to their target semantic type. For example, given:
+
+```wgsl
+type MyI32 = i32;
+const a : i32 = 1;
+const b : MyI32 = 2;
+```
+
+The **semantic** types for the variables `a` and `b` will both be the same
+`sem::I32` node pointer.
+
## Symbols
Symbols represent a unique string identifier in the source program. These string
@@ -157,9 +168,9 @@
`ProgramBuilder` along with semantic information generated by the
`Resolver`.
-Like `ProgramBuilder`, `Program::IsValid()` may be called to ensure the AST is
-structurally correct and semantically valid, and that the `Resolver` did not
-report any errors.
+`Program::IsValid()` may be called to ensure the program is structurally correct
+**and** semantically valid, and that the `Resolver` did not report any errors
+during validation.
Unlike the `ProgramBuilder`, a `Program` is fully immutable, and is part of the
public Tint API. The immutable nature of `Program`s make these entirely safe
diff --git a/src/tint/sem/info.h b/src/tint/sem/info.h
index 41321cf..d9fb7b2 100644
--- a/src/tint/sem/info.h
+++ b/src/tint/sem/info.h
@@ -37,10 +37,9 @@
/// Resolves to the return type of the Get() method given the desired sementic
/// type and AST type.
- template <typename SEM, typename AST_OR_TYPE>
- using GetResultType = std::conditional_t<std::is_same<SEM, InferFromAST>::value,
- SemanticNodeTypeFor<AST_OR_TYPE>,
- SEM>;
+ template <typename SEM, typename AST>
+ using GetResultType =
+ std::conditional_t<std::is_same<SEM, InferFromAST>::value, SemanticNodeTypeFor<AST>, SEM>;
/// Constructor
Info();
@@ -56,36 +55,36 @@
/// @return this Program
Info& operator=(Info&& rhs);
- /// Get looks up the semantic information for the AST or type node `node`.
- /// @param node the AST or type node
+ /// Get looks up the semantic information for the AST node `node`.
+ /// @param ast_node the AST node
/// @returns a pointer to the semantic node if found, otherwise nullptr
template <typename SEM = InferFromAST,
- typename AST_OR_TYPE = CastableBase,
- typename RESULT = GetResultType<SEM, AST_OR_TYPE>>
- const RESULT* Get(const AST_OR_TYPE* node) const {
- auto it = map_.find(node);
+ typename AST = CastableBase,
+ typename RESULT = GetResultType<SEM, AST>>
+ const RESULT* Get(const AST* ast_node) const {
+ auto it = map_.find(ast_node);
if (it == map_.end()) {
return nullptr;
}
return As<RESULT>(it->second);
}
- /// Add registers the semantic node `sem_node` for the AST or type node `node`.
- /// @param node the AST or type node
+ /// Add registers the semantic node `sem_node` for the AST node `node`.
+ /// @param ast_node the AST node
/// @param sem_node the semantic node
- template <typename AST_OR_TYPE>
- void Add(const AST_OR_TYPE* node, const SemanticNodeTypeFor<AST_OR_TYPE>* sem_node) {
+ template <typename AST>
+ void Add(const AST* ast_node, const SemanticNodeTypeFor<AST>* sem_node) {
// Check there's no semantic info already existing for the node
- TINT_ASSERT(Semantic, Get(node) == nullptr);
- map_.emplace(node, sem_node);
+ TINT_ASSERT(Semantic, Get(ast_node) == nullptr);
+ map_.emplace(ast_node, sem_node);
}
- /// Replace replaces any existing semantic node `sem_node` for the AST or type node `node`.
- /// @param node the AST or type node
+ /// Replace replaces any existing semantic node `sem_node` for the AST node `node`.
+ /// @param ast_node the AST node
/// @param sem_node the new semantic node
- template <typename AST_OR_TYPE>
- void Replace(const AST_OR_TYPE* node, const SemanticNodeTypeFor<AST_OR_TYPE>* sem_node) {
- map_[node] = sem_node;
+ template <typename AST>
+ void Replace(const AST* ast_node, const SemanticNodeTypeFor<AST>* sem_node) {
+ map_[ast_node] = sem_node;
}
/// Wrap returns a new Info created with the contents of `inner`.
@@ -110,9 +109,8 @@
const sem::Module* Module() const { return module_; }
private:
- // TODO(crbug.com/tint/724): Once finished, this map should be:
- // std::unordered_map<const ast::Node*, const sem::Node*>
- std::unordered_map<const CastableBase*, const CastableBase*> map_;
+ // The map of AST node to semantic node
+ std::unordered_map<const ast::Node*, const sem::Node*> map_;
// The semantic module
sem::Module* module_ = nullptr;
};
