[WGSL] Variable redeclaration should not assert

https://bugs.webkit.org/show_bug.cgi?id=259727
rdar://113257297

Reviewed by Dan Glastonbury.

Currently, when inserting a new value or type into the type checker context we
assert that the context doesn't already have another entry with the same name.
Instead, we should report an error informing that it's invalid to redeclare
that value/type.

* Source/WebGPU/WGSL/ContextProvider.h:
* Source/WebGPU/WGSL/ContextProviderInlines.h:
(WGSL::ContextProvider<Value>::Context::add):
(WGSL::ContextProvider<Value>::introduceVariable const):
* Source/WebGPU/WGSL/MangleNames.cpp:
(WGSL::NameManglerVisitor::introduceVariable):
* Source/WebGPU/WGSL/TypeCheck.cpp:
(WGSL::TypeChecker::TypeChecker):
(WGSL::TypeChecker::introduceType):
(WGSL::TypeChecker::introduceValue):
* Source/WebGPU/WGSL/tests/invalid/redeclaration.wgsl: Added.

Canonical link: https://commits.webkit.org/266635@main

Source/WebGPU/WGSL/ContextProvider.h

@@ -53,7 +53,7 @@ class ContextProvider {
 
     ContextProvider();
 
-    const ContextValue& introduceVariable(const String&, const ContextValue&);
+    const ContextValue* introduceVariable(const String&, const ContextValue&);
     const ContextValue* readVariable(const String&) const;
 
 private:
@@ -62,7 +62,7 @@ class ContextProvider {
         Context(const Context *const parent);
 
         const ContextValue* lookup(const String&) const;
-        const ContextValue& add(const String&, const ContextValue&);
+        const ContextValue* add(const String&, const ContextValue&);
 
     private:
         const Context* m_parent { nullptr };

Source/WebGPU/WGSL/ContextProviderInlines.h

@@ -48,11 +48,12 @@ const Value* ContextProvider<Value>::Context::lookup(const String& name) const
 }
 
 template<typename Value>
-const Value& ContextProvider<Value>::Context::add(const String& name, const Value& value)
+const Value* ContextProvider<Value>::Context::add(const String& name, const Value& value)
 {
     auto result = m_map.add(name, value);
-    ASSERT(result.isNewEntry);
-    return result.iterator->value;
+    if (UNLIKELY(!result.isNewEntry))
+        return nullptr;
+    return &result.iterator->value;
 }
 
 template<typename Value>
@@ -80,7 +81,7 @@ ContextProvider<Value>::ContextProvider()
 }
 
 template<typename Value>
-auto ContextProvider<Value>::introduceVariable(const String& name, const Value& value) -> const Value&
+auto ContextProvider<Value>::introduceVariable(const String& name, const Value& value) -> const Value*
 {
     return m_context->add(name, value);
 }

Source/WebGPU/WGSL/MangleNames.cpp

@@ -226,8 +226,9 @@ void NameManglerVisitor::visit(AST::NamedTypeName& type)
 
 void NameManglerVisitor::introduceVariable(AST::Identifier& name, MangledName::Kind kind)
 {
-    const auto& mangledName = ContextProvider::introduceVariable(name, makeMangledName(name, kind));
-    m_callGraph.ast().replace(&name, AST::Identifier::makeWithSpan(name.span(), mangledName.toString()));
+    const auto* mangledName = ContextProvider::introduceVariable(name, makeMangledName(name, kind));
+    ASSERT(mangledName);
+    m_callGraph.ast().replace(&name, AST::Identifier::makeWithSpan(name.span(), mangledName->toString()));
 }
 
 MangledName NameManglerVisitor::makeMangledName(const String& name, MangledName::Kind kind)

Source/WebGPU/WGSL/TypeCheck.cpp

@@ -129,8 +129,8 @@ class TypeChecker : public AST::Visitor, public ContextProvider<Binding> {
     void inferred(const Type*);
     bool unify(const Type*, const Type*) WARN_UNUSED_RETURN;
     bool isBottom(const Type*) const;
-    void introduceType(const String&, const Type*);
-    void introduceValue(const String&, const Type*);
+    void introduceType(const AST::Identifier&, const Type*);
+    void introduceValue(const AST::Identifier&, const Type*);
 
     template<typename CallArguments>
     const Type* chooseOverload(const char*, const SourceSpan&, const String&, CallArguments&& valueArguments, const Vector<const Type*>& typeArguments);
@@ -148,12 +148,12 @@ TypeChecker::TypeChecker(ShaderModule& shaderModule)
     : m_shaderModule(shaderModule)
     , m_types(shaderModule.types())
 {
-    introduceType("bool"_s, m_types.boolType());
-    introduceType("i32"_s, m_types.i32Type());
-    introduceType("u32"_s, m_types.u32Type());
-    introduceType("f32"_s, m_types.f32Type());
-    introduceType("sampler"_s, m_types.samplerType());
-    introduceType("texture_external"_s, m_types.textureExternalType());
+    introduceType(AST::Identifier::make("bool"_s), m_types.boolType());
+    introduceType(AST::Identifier::make("i32"_s), m_types.i32Type());
+    introduceType(AST::Identifier::make("u32"_s), m_types.u32Type());
+    introduceType(AST::Identifier::make("f32"_s), m_types.f32Type());
+    introduceType(AST::Identifier::make("sampler"_s), m_types.samplerType());
+    introduceType(AST::Identifier::make("texture_external"_s), m_types.textureExternalType());
 
     // This file contains the declarations generated from `TypeDeclarations.rb`
 #include "TypeDeclarations.h" // NOLINT
@@ -1053,14 +1053,16 @@ bool TypeChecker::isBottom(const Type* type) const
     return type == m_types.bottomType();
 }
 
-void TypeChecker::introduceType(const String& name, const Type* type)
+void TypeChecker::introduceType(const AST::Identifier& name, const Type* type)
 {
-    introduceVariable(name, { Binding::Type, type });
+    if (!introduceVariable(name, { Binding::Type, type }))
+        typeError(InferBottom::No, name.span(), "redeclaration of '", name, "'");
 }
 
-void TypeChecker::introduceValue(const String& name, const Type* type)
+void TypeChecker::introduceValue(const AST::Identifier& name, const Type* type)
 {
-    introduceVariable(name, { Binding::Value, type });
+    if (!introduceVariable(name, { Binding::Value, type }))
+        typeError(InferBottom::No, name.span(), "redeclaration of '", name, "'");
 }
 
 template<typename... Arguments>

Source/WebGPU/WGSL/tests/invalid/redeclaration.wgsl

@@ -0,0 +1,16 @@
+// RUN: %not %wgslc | %check
+
+struct f {
+}
+
+// CHECK-L: redeclaration of 'f'
+override f = 1;
+
+// CHECK-L: redeclaration of 'f'
+fn f() {
+    let x = 1;
+    // CHECK-L: redeclaration of 'x'
+    var x = 2;
+    // CHECK-L: redeclaration of 'x'
+    const x = 2;
+}