Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 5 commits into from Jul 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 1 addition & 4 deletions crates/wasmparser/benches/benchmark.rs
Expand Up @@ -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 {
Expand Down
24 changes: 17 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,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")?;
}
}
}
Expand All @@ -75,7 +80,6 @@ impl<'a> Resolver<'a> {
_ => self.type_info.push(TypeInfo::Other),
}

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

Expand Down Expand Up @@ -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) => {
Expand Down
2 changes: 1 addition & 1 deletion crates/wast/src/names.rs
Expand Up @@ -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(())
Expand Down
6 changes: 6 additions & 0 deletions tests/local/gc/gc-subtypes-invalid.wast
Expand Up @@ -129,3 +129,9 @@
)
"type index out of bounds"
)
(assert_invalid
(module
(type (struct (field $vt (mut i32)) (field $vt (mut i64))))
)
"duplicate identifier"
)
5 changes: 5 additions & 0 deletions tests/local/gc/gc-subtypes.wat
Expand Up @@ -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
)
4 changes: 4 additions & 0 deletions tests/snapshots/local/gc/gc-subtypes.wat.print
Expand Up @@ -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)))))
)