Skip to content

Commit fa041b5

Browse files
authored
Refactor pop_maybe_shared_ref with AbstractHeapType (#1726)
This was an idea I had after #1719 landed to statically ensure that the asserts won't trip.
1 parent b8f0db3 commit fa041b5

1 file changed

Lines changed: 24 additions & 21 deletions

File tree

crates/wasmparser/src/validator/operators.rs

Lines changed: 24 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -618,27 +618,30 @@ where
618618
}
619619

620620
/// Pop a reference type from the operand stack, checking if it is a subtype
621-
/// of `expected` or the shared version of `expected`. This function returns
622-
/// the popped reference type and its `shared`-ness, saving extra lookups
623-
/// for concrete types. This function will panic if `expected` is shared or
624-
/// a concrete type.
625-
fn pop_maybe_shared_ref(&mut self, expected: RefType) -> Result<Option<(RefType, bool)>> {
626-
debug_assert!(!expected.is_concrete_type_ref());
627-
assert!(!self.resources.is_shared(expected));
621+
/// of a nullable type of `expected` or the shared version of `expected`.
622+
///
623+
/// This function returns the popped reference type and its `shared`-ness,
624+
/// saving extra lookups for concrete types.
625+
fn pop_maybe_shared_ref(
626+
&mut self,
627+
expected: AbstractHeapType,
628+
) -> Result<Option<(RefType, bool)>> {
628629
let actual = match self.pop_ref()? {
629630
Some(rt) => rt,
630631
None => return Ok(None),
631632
};
632633
// Change our expectation based on whether we're dealing with an actual
633634
// shared or unshared type.
634635
let is_actual_shared = self.resources.is_shared(actual);
635-
let expected = if is_actual_shared {
636-
expected
637-
.shared()
638-
.expect("this only expects abstract heap types")
639-
} else {
640-
expected
641-
};
636+
let expected = RefType::new(
637+
true,
638+
HeapType::Abstract {
639+
shared: is_actual_shared,
640+
ty: expected,
641+
},
642+
)
643+
.unwrap();
644+
642645
// Check (again) that the actual type is a subtype of the expected type.
643646
// Note that `_pop_operand` already does this kind of thing but we leave
644647
// that for a future refactoring (TODO).
@@ -2749,8 +2752,8 @@ where
27492752
Ok(())
27502753
}
27512754
fn visit_ref_eq(&mut self) -> Self::Output {
2752-
let a = self.pop_maybe_shared_ref(RefType::EQ.nullable())?;
2753-
let b = self.pop_maybe_shared_ref(RefType::EQ.nullable())?;
2755+
let a = self.pop_maybe_shared_ref(AbstractHeapType::Eq)?;
2756+
let b = self.pop_maybe_shared_ref(AbstractHeapType::Eq)?;
27542757
match (a, b) {
27552758
(Some((_, is_a_shared)), Some((_, is_b_shared))) => {
27562759
if is_a_shared != is_b_shared {
@@ -4264,7 +4267,7 @@ where
42644267
Ok(())
42654268
}
42664269
fn visit_array_len(&mut self) -> Self::Output {
4267-
self.pop_maybe_shared_ref(RefType::ARRAY.nullable())?;
4270+
self.pop_maybe_shared_ref(AbstractHeapType::Array)?;
42684271
self.push_operand(ValType::I32)
42694272
}
42704273
fn visit_array_fill(&mut self, array_type_index: u32) -> Self::Output {
@@ -4446,7 +4449,7 @@ where
44464449
Ok(())
44474450
}
44484451
fn visit_any_convert_extern(&mut self) -> Self::Output {
4449-
let extern_ref = self.pop_maybe_shared_ref(RefType::EXTERNREF)?;
4452+
let extern_ref = self.pop_maybe_shared_ref(AbstractHeapType::Extern)?;
44504453
let (is_nullable, shared) = if let Some((extern_ref, shared)) = extern_ref {
44514454
(extern_ref.is_nullable(), shared)
44524455
} else {
@@ -4462,7 +4465,7 @@ where
44624465
self.push_operand(any_ref)
44634466
}
44644467
fn visit_extern_convert_any(&mut self) -> Self::Output {
4465-
let any_ref = self.pop_maybe_shared_ref(RefType::ANY.nullable())?;
4468+
let any_ref = self.pop_maybe_shared_ref(AbstractHeapType::Any)?;
44664469
let (is_nullable, shared) = if let Some((any_ref, shared)) = any_ref {
44674470
(any_ref.is_nullable(), shared)
44684471
} else {
@@ -4587,11 +4590,11 @@ where
45874590
))
45884591
}
45894592
fn visit_i31_get_s(&mut self) -> Self::Output {
4590-
self.pop_maybe_shared_ref(RefType::I31REF)?;
4593+
self.pop_maybe_shared_ref(AbstractHeapType::I31)?;
45914594
self.push_operand(ValType::I32)
45924595
}
45934596
fn visit_i31_get_u(&mut self) -> Self::Output {
4594-
self.pop_maybe_shared_ref(RefType::I31REF)?;
4597+
self.pop_maybe_shared_ref(AbstractHeapType::I31)?;
45954598
self.push_operand(ValType::I32)
45964599
}
45974600
fn visit_try(&mut self, mut ty: BlockType) -> Self::Output {

0 commit comments

Comments
 (0)