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 bytecodealliance#1092.

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,41 @@ 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")?;
+                        if let Some(parent_type_index) = ty
+                            .parent
+                            .map(|index| self.types.get_index(&index))
+                            .flatten()
+                        {
+                            if let Some(parent_field_ns) = self.fields.get(&parent_type_index) {
+                                let field_idx =
+                                    parent_field_ns.resolve(&mut Index::Id(id), "field")?;
+                                if field_idx != i as u32 {
+                                    return Err(Error::new(
+                                        id.span(),
+                                        format!(
+                                            "field index mismatch for `{}`: expected {}, got {}",
+                                            id.name(),
+                                            field_idx,
+                                            i
+                                        ),
+                                    ));
+                                }
+                            }
+                        }
+                        self.fields
+                            .entry(type_index)
+                            .or_insert(Namespace::default())
+                            .register_specific(id, i as u32, "field")?;
                     }
                 }
             }
@@ -75,7 +101,6 @@ impl<'a> Resolver<'a> {
             _ => self.type_info.push(TypeInfo::Other),
         }
 
-        self.types.register(ty.id, "type")?;
         Ok(())
     }
 
@@ -610,8 +635,12 @@ 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)?;
+                self.resolver
+                    .fields
+                    .get(&type_index)
+                    .unwrap()
+                    .resolve(&mut s.field, "field")?;
             }
 
             ArrayNewFixed(a) => {

crates/wast/src/names.rs

@@ -51,11 +51,13 @@ impl<'a> Namespace<'a> {
     }
 
     pub fn register_specific(&mut self, name: Id<'a>, index: u32, desc: &str) -> Result<(), Error> {
-        if let Some(_prev) = self.names.insert(name, index) {
-            return Err(Error::new(
-                name.span(),
-                format!("duplicate identifier for {}", desc),
-            ));
+        if let Some(prev) = self.names.insert(name, index) {
+            if prev != index {
+                return Err(Error::new(
+                    name.span(),
+                    format!("duplicate identifier for {}", desc),
+                ));
+            }
         }
         Ok(())
     }
@@ -71,6 +73,14 @@ impl<'a> Namespace<'a> {
         }
         Err(resolve_error(*id, desc))
     }
+
+    pub fn get_index(&self, idx: &Index<'a>) -> Option<u32> {
+        let id = match idx {
+            Index::Num(n, _) => return Some(*n),
+            Index::Id(id) => id,
+        };
+        self.names.get(id).copied()
+    }
 }
 
 pub fn resolve_error(id: Id<'_>, ns: &str) -> Error {

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

@@ -129,3 +129,17 @@
   )
   "type index out of bounds"
 )
+(assert_invalid
+  (module
+    (type $A (struct (field $vt (mut i32))))
+    (type $C (sub $A (struct (field $tv (mut i32)))))
+  )
+  "unknown field"
+)
+(assert_invalid
+  (module
+    (type $A (struct (field $vt (mut i32))))
+    (type $C (sub $A (struct (field (mut i32)) (field $vt (mut i32)))))
+  )
+  "field index mismatch"
+)

tests/local/gc/gc-subtypes.wat

@@ -97,4 +97,7 @@
   (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)))))
 )

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

@@ -68,4 +68,6 @@
   (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)))))
 )