Fix "duplicate identifier for field" for subtype fields (#1117)

* Fix "duplicate identifier for field" for subtype fields

Field namespaces are per struct now. Inherited fields' names must be exactly the same as in the super-type.

Fixes #1092.

* Remove some unused imports

* Address review comments

* Address review comments

* Fix a broken test

crates/wasmparser/benches/benchmark.rs

@@ -4,10 +4,7 @@ use once_cell::unsync::Lazy;
 use std::fs;
 use std::path::Path;
 use std::path::PathBuf;
-use wasmparser::{
-    DataKind, ElementKind, HeapType, Parser, Payload, ValType, Validator, VisitOperator,
-    WasmFeatures,
-};
+use wasmparser::{DataKind, ElementKind, Parser, Payload, Validator, VisitOperator, WasmFeatures};
 
 /// A benchmark input.
 pub struct BenchmarkInput {

crates/wast/src/core/resolve/names.rs

@@ -3,6 +3,7 @@ use crate::core::*;
 use crate::names::{resolve_error, Namespace};
 use crate::token::{Id, Index};
 use crate::Error;
+use std::collections::HashMap;
 
 pub fn resolve<'a>(fields: &mut Vec<ModuleField<'a>>) -> Result<Resolver<'a>, Error> {
     let mut resolver = Resolver::default();
@@ -25,7 +26,7 @@ pub struct Resolver<'a> {
     tags: Namespace<'a>,
     datas: Namespace<'a>,
     elems: Namespace<'a>,
-    fields: Namespace<'a>,
+    fields: HashMap<u32, Namespace<'a>>,
     type_info: Vec<TypeInfo<'a>>,
 }
 
@@ -46,16 +47,20 @@ impl<'a> Resolver<'a> {
     }
 
     fn register_type(&mut self, ty: &Type<'a>) -> Result<(), Error> {
+        let type_index = self.types.register(ty.id, "type")?;
+
         match &ty.def {
             // For GC structure types we need to be sure to populate the
             // field namespace here as well.
             //
-            // The field namespace is global, but the resolved indices
-            // are relative to the struct they are defined in
+            // The field namespace is relative to the struct fields are defined in
             TypeDef::Struct(r#struct) => {
                 for (i, field) in r#struct.fields.iter().enumerate() {
                     if let Some(id) = field.id {
-                        self.fields.register_specific(id, i as u32, "field")?;
+                        self.fields
+                            .entry(type_index)
+                            .or_insert(Namespace::default())
+                            .register_specific(id, i as u32, "field")?;
                     }
                 }
             }
@@ -75,7 +80,6 @@ impl<'a> Resolver<'a> {
             _ => self.type_info.push(TypeInfo::Other),
         }
 
-        self.types.register(ty.id, "type")?;
         Ok(())
     }
 
@@ -610,8 +614,14 @@ impl<'a, 'b> ExprResolver<'a, 'b> {
             }
 
             StructSet(s) | StructGet(s) | StructGetS(s) | StructGetU(s) => {
-                self.resolver.resolve(&mut s.r#struct, Ns::Type)?;
-                self.resolver.fields.resolve(&mut s.field, "field")?;
+                let type_index = self.resolver.resolve(&mut s.r#struct, Ns::Type)?;
+                if let Index::Id(field_id) = s.field {
+                    self.resolver
+                        .fields
+                        .get(&type_index)
+                        .ok_or(Error::new(field_id.span(), format!("accessing a named field `{}` in a struct without named fields, type index {}", field_id.name(), type_index)))?
+                        .resolve(&mut s.field, "field")?;
+                }
             }
 
             ArrayNewFixed(a) => {

crates/wast/src/names.rs

@@ -54,7 +54,7 @@ impl<'a> Namespace<'a> {
         if let Some(_prev) = self.names.insert(name, index) {
             return Err(Error::new(
                 name.span(),
-                format!("duplicate identifier for {}", desc),
+                format!("duplicate identifier `{}` for {}", name.name(), desc),
             ));
         }
         Ok(())

tests/local/gc/gc-subtypes-invalid.wast

@@ -129,3 +129,9 @@
   )
   "type index out of bounds"
 )
+(assert_invalid
+  (module
+    (type (struct (field $vt (mut i32)) (field $vt (mut i64))))
+  )
+  "duplicate identifier"
+)

tests/local/gc/gc-subtypes.wat

@@ -97,4 +97,9 @@
   (type $y0 (sub $v0 (array (ref noextern))))
   (type $y01 (sub $u0 (array (ref noextern))))
   (type $z0 (sub $u0 (array nullexternref)))
+
+  (type $A (struct (field $vt (mut i32))))
+  (type $B (sub $A (struct (field $vt (mut i32)))))
+  (type (sub $A (struct (field $tv (mut i32))))) ;; same field, different name
+  (type (sub $A (struct (field (mut i32)) (field $vt (mut i64))))) ;; different field, same name
 )

tests/snapshots/local/gc/gc-subtypes.wat.print

@@ -68,4 +68,8 @@
   (type $y0 (;66;) (sub $v0 (;65;) (array (ref noextern))))
   (type $y01 (;67;) (sub $u0 (;64;) (array (ref noextern))))
   (type $z0 (;68;) (sub $u0 (;64;) (array nullexternref)))
+  (type $A (;69;) (struct (field (mut i32))))
+  (type $B (;70;) (sub $A (;69;) (struct (field (mut i32)))))
+  (type (;71;) (sub $A (;69;) (struct (field (mut i32)))))
+  (type (;72;) (sub $A (;69;) (struct (field (mut i32)) (field (mut i64)))))
 )