From b79894af1a6e4cd67700b505546cf05550d69a3d Mon Sep 17 00:00:00 2001 From: Ivan Mikushin Date: Fri, 30 Jun 2023 18:20:13 -0700 Subject: [PATCH] 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. --- crates/wast/src/core/resolve/names.rs | 43 ++++++++++++++++--- crates/wast/src/names.rs | 20 ++++++--- tests/local/gc/gc-subtypes-invalid.wast | 14 ++++++ tests/local/gc/gc-subtypes.wat | 3 ++ .../snapshots/local/gc/gc-subtypes.wat.print | 2 + 5 files changed, 70 insertions(+), 12 deletions(-) diff --git a/crates/wast/src/core/resolve/names.rs b/crates/wast/src/core/resolve/names.rs index 03b4d851d6..33bdcaf022 100644 --- a/crates/wast/src/core/resolve/names.rs +++ b/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>) -> Result, 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>, type_info: Vec>, } @@ -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) => { diff --git a/crates/wast/src/names.rs b/crates/wast/src/names.rs index 7cbfc5d9ca..f190a77024 100644 --- a/crates/wast/src/names.rs +++ b/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 { + 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 { diff --git a/tests/local/gc/gc-subtypes-invalid.wast b/tests/local/gc/gc-subtypes-invalid.wast index c143d45bf5..178cc9dfc7 100644 --- a/tests/local/gc/gc-subtypes-invalid.wast +++ b/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" +) diff --git a/tests/local/gc/gc-subtypes.wat b/tests/local/gc/gc-subtypes.wat index 86057600f8..61c935dcf5 100644 --- a/tests/local/gc/gc-subtypes.wat +++ b/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))))) ) diff --git a/tests/snapshots/local/gc/gc-subtypes.wat.print b/tests/snapshots/local/gc/gc-subtypes.wat.print index 2e102903bb..4e4fad3c41 100644 --- a/tests/snapshots/local/gc/gc-subtypes.wat.print +++ b/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))))) ) \ No newline at end of file