tint: Have ast::TypeName use ast::Identifier

Consistency with the other AST nodes.

Change-Id: I8db3d237c27fea44c80101ed3d24b62832d45c18
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/118360
Commit-Queue: Ben Clayton <bclayton@chromium.org>
Reviewed-by: James Price <jrprice@google.com>
Reviewed-by: Dan Sinclair <dsinclair@chromium.org>
Kokoro: Ben Clayton <bclayton@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>

src/tint/BUILD.gn

@@ -1361,6 +1361,7 @@ if (tint_build_unittests) {
       "ast/test_helper.h",
       "ast/texture_test.cc",
       "ast/traverse_expressions_test.cc",
+      "ast/type_name_test.cc",
       "ast/u32_test.cc",
       "ast/unary_op_expression_test.cc",
       "ast/variable_decl_statement_test.cc",

src/tint/CMakeLists.txt

@@ -882,6 +882,7 @@ if(TINT_BUILD_TESTS)
     ast/test_helper.h
     ast/texture_test.cc
     ast/traverse_expressions_test.cc
+    ast/type_name_test.cc
     ast/u32_test.cc
     ast/unary_op_expression_test.cc
     ast/variable_decl_statement_test.cc

src/tint/ast/type_name.cc

@@ -20,7 +20,7 @@ TINT_INSTANTIATE_TYPEINFO(tint::ast::TypeName);
 
 namespace tint::ast {
 
-TypeName::TypeName(ProgramID pid, NodeID nid, const Source& src, Symbol n)
+TypeName::TypeName(ProgramID pid, NodeID nid, const Source& src, const Identifier* n)
     : Base(pid, nid, src), name(n) {}
 
 TypeName::~TypeName() = default;
@@ -28,7 +28,7 @@ TypeName::~TypeName() = default;
 TypeName::TypeName(TypeName&&) = default;
 
 std::string TypeName::FriendlyName(const SymbolTable& symbols) const {
-    return symbols.NameFor(name);
+    return symbols.NameFor(name->symbol);
 }
 
 const TypeName* TypeName::Clone(CloneContext* ctx) const {

src/tint/ast/type_name.h

@@ -19,6 +19,11 @@
 
 #include "src/tint/ast/type.h"
 
+// Forward declarations
+namespace tint::ast {
+class Identifier;
+}  // namespace tint::ast
+
 namespace tint::ast {
 
 /// A named type (i.e. struct or alias)
@@ -29,7 +34,7 @@ class TypeName final : public Castable<TypeName, Type> {
     /// @param nid the unique node identifier
     /// @param src the source of this node
     /// @param name the type name
-    TypeName(ProgramID pid, NodeID nid, const Source& src, Symbol name);
+    TypeName(ProgramID pid, NodeID nid, const Source& src, const Identifier* name);
     /// Move constructor
     TypeName(TypeName&&);
     /// Destructor
@@ -46,7 +51,7 @@ class TypeName final : public Castable<TypeName, Type> {
     const TypeName* Clone(CloneContext* ctx) const override;
 
     /// The type name
-    Symbol name;
+    Identifier const* const name;
 };
 
 }  // namespace tint::ast

src/tint/ast/type_name_test.cc

@@ -0,0 +1,61 @@
+// Copyright 2023 The Tint Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "gtest/gtest-spi.h"
+
+#include "src/tint/ast/test_helper.h"
+
+namespace tint::ast {
+namespace {
+
+using namespace tint::number_suffixes;  // NOLINT
+using TypeNameTest = TestHelper;
+
+TEST_F(TypeNameTest, Creation_NonTemplated) {
+    auto* t = ty.type_name("ty");
+    ASSERT_NE(t->name, nullptr);
+    EXPECT_EQ(t->name->symbol, Symbols().Get("ty"));
+}
+
+TEST_F(TypeNameTest, Creation_WithSource) {
+    auto* t = ty.type_name(Source{{20, 2}}, "ty");
+    ASSERT_NE(t->name, nullptr);
+    EXPECT_EQ(t->name->symbol, Symbols().Get("ty"));
+
+    auto src = t->source;
+    EXPECT_EQ(src.range.begin.line, 20u);
+    EXPECT_EQ(src.range.begin.column, 2u);
+}
+
+TEST_F(TypeNameTest, Assert_InvalidSymbol) {
+    EXPECT_FATAL_FAILURE(
+        {
+            ProgramBuilder b;
+            b.ty.type_name("");
+        },
+        "internal compiler error");
+}
+
+TEST_F(TypeNameTest, Assert_DifferentProgramID_Symbol) {
+    EXPECT_FATAL_FAILURE(
+        {
+            ProgramBuilder b1;
+            ProgramBuilder b2;
+            b1.ty.type_name(b2.Sym("b2"));
+        },
+        "internal compiler error");
+}
+
+}  // namespace
+}  // namespace tint::ast

src/tint/program_builder.h

@@ -907,7 +907,7 @@ class ProgramBuilder {
         /// @returns the type name
         template <typename NAME>
         const ast::TypeName* type_name(NAME&& name) const {
-            return builder->create<ast::TypeName>(builder->Sym(std::forward<NAME>(name)));
+            return builder->create<ast::TypeName>(builder->Ident(std::forward<NAME>(name)));
         }
 
         /// Creates a type name
@@ -916,7 +916,7 @@ class ProgramBuilder {
         /// @returns the type name
         template <typename NAME>
         const ast::TypeName* type_name(const Source& source, NAME&& name) const {
-            return builder->create<ast::TypeName>(source, builder->Sym(std::forward<NAME>(name)));
+            return builder->create<ast::TypeName>(source, builder->Ident(std::forward<NAME>(name)));
         }
 
         /// Creates an alias type

src/tint/reader/wgsl/parser_impl.cc

@@ -1213,7 +1213,7 @@ Maybe<const ast::Type*> ParserImpl::type_specifier() {
     auto& t = peek();
     Source source;
     if (match(Token::Type::kIdentifier, &source)) {
-        return builder_.create<ast::TypeName>(source, builder_.Symbols().Register(t.to_str()));
+        return builder_.ty.type_name(source, t.to_str());
     }
 
     return type_specifier_without_ident();

src/tint/reader/wgsl/parser_impl_global_decl_test.cc

@@ -129,7 +129,7 @@ alias B = A;)");
     EXPECT_EQ(alias->name, program.Symbols().Get("B"));
     auto* tn = alias->type->As<ast::TypeName>();
     EXPECT_NE(tn, nullptr);
-    EXPECT_EQ(tn->name, str->name);
+    EXPECT_EQ(tn->name->symbol, str->name);
 }
 
 // TODO(crbug.com/tint/1812): DEPRECATED
@@ -165,7 +165,7 @@ type B = A;)");
     EXPECT_EQ(alias->name, program.Symbols().Get("B"));
     auto* tn = alias->type->As<ast::TypeName>();
     EXPECT_NE(tn, nullptr);
-    EXPECT_EQ(tn->name, str->name);
+    EXPECT_EQ(tn->name->symbol, str->name);
 }
 
 TEST_F(ParserImplTest, GlobalDecl_TypeAlias_MissingSemicolon) {

src/tint/reader/wgsl/parser_impl_type_decl_test.cc

@@ -40,7 +40,7 @@ TEST_F(ParserImplTest, TypeDecl_Identifier) {
     ASSERT_NE(t.value, nullptr) << p->error();
     auto* type_name = t.value->As<ast::TypeName>();
     ASSERT_NE(type_name, nullptr);
-    EXPECT_EQ(p->builder().Symbols().Get("A"), type_name->name);
+    EXPECT_EQ(p->builder().Symbols().Get("A"), type_name->name->symbol);
     EXPECT_EQ(type_name->source.range, (Source::Range{{1u, 1u}, {1u, 2u}}));
 }
 

src/tint/resolver/dependency_graph.cc

@@ -392,7 +392,7 @@ class DependencyScanner {
                 TraverseType(ptr->type);
             },
             [&](const ast::TypeName* tn) {  //
-                AddDependency(tn, tn->name, "type", "references");
+                AddDependency(tn, tn->name->symbol, "type", "references");
             },
             [&](const ast::Vector* vec) {  //
                 TraverseType(vec->type);

src/tint/resolver/resolver.cc

@@ -318,8 +318,18 @@ type::Type* Resolver::Type(const ast::Type* ty) {
             return nullptr;
         },
         [&](const ast::ExternalTexture*) { return builder_->create<type::ExternalTexture>(); },
-        [&](Default) {
-            auto* resolved = sem_.ResolvedSymbol(ty);
+        [&](const ast::TypeName* t) -> type::Type* {
+            Mark(t->name);
+
+            auto* resolved = sem_.ResolvedSymbol(t);
+            if (resolved == nullptr) {
+                if (IsBuiltin(t->name->symbol)) {
+                    auto name = builder_->Symbols().NameFor(t->name->symbol);
+                    AddError("cannot use builtin '" + name + "' as type", ty->source);
+                    return nullptr;
+                }
+                return ShortName(t->name->symbol, t->source);
+            }
             return Switch(
                 resolved,  //
                 [&](type::Type* type) { return type; },
@@ -336,20 +346,17 @@ type::Type* Resolver::Type(const ast::Type* ty) {
                     return nullptr;
                 },
                 [&](Default) -> type::Type* {
-                    if (auto* tn = ty->As<ast::TypeName>()) {
-                        if (IsBuiltin(tn->name)) {
-                            auto name = builder_->Symbols().NameFor(tn->name);
-                            AddError("cannot use builtin '" + name + "' as type", ty->source);
-                            return nullptr;
-                        }
-                        return ShortName(tn->name, tn->source);
-                    }
                     TINT_UNREACHABLE(Resolver, diagnostics_)
                         << "Unhandled resolved type '"
                         << (resolved ? resolved->TypeInfo().name : "<null>")
                         << "' resolved from ast::Type '" << ty->TypeInfo().name << "'";
                     return nullptr;
                 });
+        },
+        [&](Default) {
+            TINT_UNREACHABLE(Resolver, diagnostics_)
+                << "Unhandled type: '" << ty->TypeInfo().name << "'";
+            return nullptr;
         });
 
     if (s) {

src/tint/resolver/resolver_test_helper.h

@@ -565,7 +565,7 @@ struct DataType<alias<T, ID>> {
             auto* type = DataType<T>::AST(b);
             b.AST().AddTypeDecl(b.ty.alias(name, type));
         }
-        return b.create<ast::TypeName>(name);
+        return b.ty.type_name(name);
     }
     /// @param b the ProgramBuilder
     /// @return the semantic aliased type

src/tint/transform/clamp_frag_depth.cc

@@ -164,8 +164,9 @@ Transform::ApplyResult ClampFragDepth::Apply(const Program* src, const DataMap&,
             auto* struct_ty = sem.Get(fn)->ReturnType()->As<sem::Struct>()->Declaration();
             auto helper = io_structs_clamp_helpers.GetOrCreate(struct_ty, [&] {
                 auto* return_ty = fn->return_type;
-                auto fn_sym = b.Symbols().New("clamp_frag_depth_" +
-                                              sym.NameFor(return_ty->As<ast::TypeName>()->name));
+                auto fn_sym =
+                    b.Symbols().New("clamp_frag_depth_" +
+                                    sym.NameFor(return_ty->As<ast::TypeName>()->name->symbol));
 
                 utils::Vector<const ast::Expression*, 8u> initializer_args;
                 for (auto* member : struct_ty->members) {

src/tint/transform/renamer.cc

@@ -1263,8 +1263,6 @@ Transform::ApplyResult Renamer::Apply(const Program* src,
 
     // Identifiers that need to keep their symbols preserved.
     utils::Hashset<const ast::Identifier*, 8> preserved_identifiers;
-    // Type names that need to keep their symbols preserved.
-    utils::Hashset<const ast::TypeName*, 8> preserved_type_names;
 
     auto is_type_short_name = [&](const Symbol& symbol) {
         auto name = src->Symbols().NameFor(symbol);
@@ -1317,8 +1315,8 @@ Transform::ApplyResult Renamer::Apply(const Program* src,
                 preserved_identifiers.Add(diagnostic->rule_name);
             },
             [&](const ast::TypeName* type_name) {
-                if (is_type_short_name(type_name->name)) {
-                    preserved_type_names.Add(type_name);
+                if (is_type_short_name(type_name->name->symbol)) {
+                    preserved_identifiers.Add(type_name->name);
                 }
             });
     }
@@ -1386,16 +1384,6 @@ Transform::ApplyResult Renamer::Apply(const Program* src,
         return nullptr;  // Clone ident. Uses the symbol remapping above.
     });
 
-    ctx.ReplaceAll([&](const ast::TypeName* type_name) -> const ast::TypeName* {
-        if (preserved_type_names.Contains(type_name)) {
-            auto sym_in = type_name->name;
-            auto str = src->Symbols().NameFor(sym_in);
-            auto sym_out = b.Symbols().Register(str);
-            return ctx.dst->create<ast::TypeName>(ctx.Clone(type_name->source), sym_out);
-        }
-        return nullptr;  // Clone ident. Uses the symbol remapping above.
-    });
-
     ctx.Clone();  // Must come before the std::move()
 
     outputs.Add<Data>(std::move(remappings));

src/tint/transform/std140.cc

@@ -405,7 +405,7 @@ struct Std140::State {
             ty,  //
             [&](const sem::Struct* str) -> const ast::Type* {
                 if (auto std140 = std140_structs.Find(str)) {
-                    return b.create<ast::TypeName>(*std140);
+                    return b.ty.type_name(*std140);
                 }
                 return nullptr;
             },

src/tint/transform/transform.cc

@@ -138,7 +138,7 @@ const ast::Type* Transform::CreateASTTypeFor(CloneContext& ctx, const type::Type
         return ctx.dst->ty.array(el, u32(count.value()), std::move(attrs));
     }
     if (auto* s = ty->As<sem::Struct>()) {
-        return ctx.dst->create<ast::TypeName>(ctx.Clone(s->Declaration()->name));
+        return ctx.dst->ty.type_name(ctx.Clone(s->Declaration()->name));
     }
     if (auto* s = ty->As<type::Reference>()) {
         return CreateASTTypeFor(ctx, s->StoreType());

src/tint/transform/transform_test.cc

@@ -116,7 +116,7 @@ TEST_F(CreateASTTypeForTest, AliasedArrayWithComplexOverrideLength) {
     CloneContext ctx(&ast_type_builder, &program, false);
     auto* ast_ty = tint::As<ast::TypeName>(CreateASTTypeFor(ctx, arr_ty));
     ASSERT_NE(ast_ty, nullptr);
-    EXPECT_EQ(ast_type_builder.Symbols().NameFor(ast_ty->name), "A");
+    EXPECT_EQ(ast_type_builder.Symbols().NameFor(ast_ty->name->symbol), "A");
 }
 
 TEST_F(CreateASTTypeForTest, Struct) {
@@ -126,7 +126,7 @@ TEST_F(CreateASTTypeForTest, Struct) {
                                      4u /* size */, 4u /* size_no_padding */);
     });
     ASSERT_TRUE(str->Is<ast::TypeName>());
-    EXPECT_EQ(ast_type_builder.Symbols().NameFor(str->As<ast::TypeName>()->name), "S");
+    EXPECT_EQ(ast_type_builder.Symbols().NameFor(str->As<ast::TypeName>()->name->symbol), "S");
 }
 
 }  // namespace

src/tint/writer/wgsl/generator_impl.cc

@@ -595,7 +595,7 @@ bool GeneratorImpl::EmitType(std::ostream& out, const ast::Type* ty) {
             return true;
         },
         [&](const ast::TypeName* tn) {
-            out << program_->Symbols().NameFor(tn->name);
+            out << program_->Symbols().NameFor(tn->name->symbol);
             return true;
         },
         [&](Default) {