Skip to content

Commit

Permalink
Fix "duplicate identifier for field" for subtype fields
Browse files Browse the repository at this point in the history
Field namespaces are per struct now. Inherited fields' names must be exactly the same as in the super-type.

Fixes #1092.
  • Loading branch information
imikushin committed Jul 10, 2023
1 parent 6dae59e commit b79894a
Show file tree
Hide file tree
Showing 5 changed files with 70 additions and 12 deletions.
43 changes: 36 additions & 7 deletions crates/wast/src/core/resolve/names.rs
Expand Up @@ -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();
Expand All @@ -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>>,
}

Expand All @@ -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")?;
}
}
}
Expand All @@ -75,7 +101,6 @@ impl<'a> Resolver<'a> {
_ => self.type_info.push(TypeInfo::Other),
}

self.types.register(ty.id, "type")?;
Ok(())
}

Expand Down Expand Up @@ -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) => {
Expand Down
20 changes: 15 additions & 5 deletions crates/wast/src/names.rs
Expand Up @@ -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(())
}
Expand All @@ -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 {
Expand Down
14 changes: 14 additions & 0 deletions tests/local/gc/gc-subtypes-invalid.wast
Expand Up @@ -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"
)
3 changes: 3 additions & 0 deletions tests/local/gc/gc-subtypes.wat
Expand Up @@ -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)))))
)
2 changes: 2 additions & 0 deletions tests/snapshots/local/gc/gc-subtypes.wat.print
Expand Up @@ -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)))))
)

0 comments on commit b79894a

Please sign in to comment.