1362: feat(interface-types) Remove allocator index from `string.lower_memory` r=Hywan a=Hywan

This PR updates `string.lower_memory` to remove the allocator index. Indeed, the string pointer is assumed to be present on the stack.
Also, this PR updates `string.size` to pop, and not to peek, the string to compute the length from.

That way, it matches the WIT proposal.

Co-authored-by: Ivan Enderlin <ivan@mnt.io>
This commit is contained in:
bors[bot] 2020-04-09 08:05:50 +00:00 committed by GitHub
commit ad6f939e85
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 45 additions and 150 deletions

View File

@ -232,16 +232,7 @@ fn instruction<'input, E: ParseError<&'input [u8]>>(
0x21 => (input, Instruction::I64FromU64),
0x22 => (input, Instruction::StringLiftMemory),
0x23 => {
consume!((input, argument_0) = uleb(input)?);
(
input,
Instruction::StringLowerMemory {
allocator_index: argument_0 as u32,
},
)
}
0x23 => (input, Instruction::StringLowerMemory),
0x24 => (input, Instruction::StringSize),
0x25 => {
@ -764,7 +755,7 @@ mod tests {
0x20, // I64FromU32
0x21, // I64FromU64
0x22, // StringLiftMemory
0x23, 0x01, // StringLowerMemory { allocator_index: 1 }
0x23, // StringLowerMemory
0x24, // StringSize
0x25, 0x01, // RecordLift { type_index: 1 },
0x26, 0x01, // RecordLower { type_index: 1 },
@ -808,7 +799,7 @@ mod tests {
Instruction::I64FromU32,
Instruction::I64FromU64,
Instruction::StringLiftMemory,
Instruction::StringLowerMemory { allocator_index: 1 },
Instruction::StringLowerMemory,
Instruction::StringSize,
Instruction::RecordLift { type_index: 1 },
Instruction::RecordLower { type_index: 1 },

View File

@ -309,9 +309,7 @@ impl<'a> Parse<'a> for Instruction {
} else if lookahead.peek::<keyword::string_lower_memory>() {
parser.parse::<keyword::string_lower_memory>()?;
Ok(Instruction::StringLowerMemory {
allocator_index: parser.parse()?,
})
Ok(Instruction::StringLowerMemory)
} else if lookahead.peek::<keyword::string_size>() {
parser.parse::<keyword::string_size>()?;
@ -770,7 +768,7 @@ mod tests {
"i64.from_u32",
"i64.from_u64",
"string.lift_memory",
"string.lower_memory 42",
"string.lower_memory",
"string.size",
"record.lift 42",
"record.lower 42",
@ -811,9 +809,7 @@ mod tests {
Instruction::I64FromU32,
Instruction::I64FromU64,
Instruction::StringLiftMemory,
Instruction::StringLowerMemory {
allocator_index: 42,
},
Instruction::StringLowerMemory,
Instruction::StringSize,
Instruction::RecordLift { type_index: 42 },
Instruction::RecordLower { type_index: 42 },

View File

@ -331,10 +331,7 @@ where
Instruction::I64FromU64 => 0x21_u8.to_bytes(writer)?,
Instruction::StringLiftMemory => 0x22_u8.to_bytes(writer)?,
Instruction::StringLowerMemory { allocator_index } => {
0x23_u8.to_bytes(writer)?;
(*allocator_index as u64).to_bytes(writer)?;
}
Instruction::StringLowerMemory => 0x23_u8.to_bytes(writer)?,
Instruction::StringSize => 0x24_u8.to_bytes(writer)?,
Instruction::RecordLift { type_index } => {
@ -688,7 +685,7 @@ mod tests {
Instruction::I64FromU32,
Instruction::I64FromU64,
Instruction::StringLiftMemory,
Instruction::StringLowerMemory { allocator_index: 1 },
Instruction::StringLowerMemory,
Instruction::StringSize,
Instruction::RecordLift { type_index: 1 },
Instruction::RecordLower { type_index: 1 },
@ -730,7 +727,7 @@ mod tests {
0x20, // I64FromU32
0x21, // I64FromU64
0x22, // StringLiftMemory
0x23, 0x01, // StringLowerMemory { allocator_index: 1 }
0x23, // StringLowerMemory
0x24, // StringSize
0x025, 0x01, // RecordLift { type_index: 1 }
0x026, 0x01, // RecordLower { type_index: 1 }

View File

@ -135,9 +135,7 @@ impl ToString for &Instruction {
Instruction::I64FromU32 => "i64.from_u32".into(),
Instruction::I64FromU64 => "i64.from_u64".into(),
Instruction::StringLiftMemory => "string.lift_memory".into(),
Instruction::StringLowerMemory { allocator_index } => {
format!("string.lower_memory {}", allocator_index)
}
Instruction::StringLowerMemory => "string.lower_memory".into(),
Instruction::StringSize => "string.size".into(),
Instruction::RecordLift { type_index } => format!("record.lift {}", type_index),
Instruction::RecordLower { type_index } => format!("record.lower {}", type_index),
@ -462,10 +460,7 @@ mod tests {
(&Instruction::I64FromU32).to_string(),
(&Instruction::I64FromU64).to_string(),
(&Instruction::StringLiftMemory).to_string(),
(&Instruction::StringLowerMemory {
allocator_index: 42,
})
.to_string(),
(&Instruction::StringLowerMemory).to_string(),
(&Instruction::StringSize).to_string(),
(&Instruction::RecordLift { type_index: 42 }).to_string(),
(&Instruction::RecordLower { type_index: 42 }).to_string(),
@ -506,7 +501,7 @@ mod tests {
"i64.from_u32",
"i64.from_u64",
"string.lift_memory",
"string.lower_memory 42",
"string.lower_memory",
"string.size",
"record.lift 42",
"record.lower 42",

View File

@ -130,10 +130,7 @@ pub enum Instruction {
StringLiftMemory,
/// The `string.lower_memory` instruction.
StringLowerMemory {
/// The allocator function index.
allocator_index: u32,
},
StringLowerMemory,
/// The `string.size` instruction.
StringSize,

View File

@ -2,13 +2,7 @@ use super::to_native;
use crate::{
ast::InterfaceType,
errors::{InstructionError, InstructionErrorKind},
interpreter::{
wasm::{
structures::{FunctionIndex, TypedIndex},
values::InterfaceValue,
},
Instruction,
},
interpreter::{wasm::values::InterfaceValue, Instruction},
};
use std::{cell::Cell, convert::TryInto};
@ -75,37 +69,20 @@ executable_instruction!(
);
executable_instruction!(
string_lower_memory(allocator_index: u32, instruction: Instruction) -> _ {
string_lower_memory(instruction: Instruction) -> _ {
move |runtime| -> _ {
let instance = &mut runtime.wasm_instance;
let index = FunctionIndex::new(allocator_index as usize);
let allocator = instance.local_or_import(index).ok_or_else(|| {
let inputs = runtime.stack.pop(2).ok_or_else(|| {
InstructionError::new(
instruction,
InstructionErrorKind::LocalOrImportIsMissing { function_index: allocator_index },
InstructionErrorKind::StackIsTooSmall { needed: 2 },
)
})?;
if allocator.inputs() != [InterfaceType::I32] || allocator.outputs() != [InterfaceType::I32] {
return Err(InstructionError::new(
instruction,
InstructionErrorKind::LocalOrImportSignatureMismatch {
function_index: allocator_index,
expected: (vec![InterfaceType::I32], vec![InterfaceType::I32]),
received: (allocator.inputs().to_vec(), allocator.outputs().to_vec()),
},
))
}
let string = runtime.stack.pop1().ok_or_else(|| {
InstructionError::new(
instruction,
InstructionErrorKind::StackIsTooSmall { needed: 1 },
)
})?;
let string: String = to_native(&string, instruction)?;
let string_pointer: usize = to_native::<i32>(&inputs[0], instruction)?
.try_into()
.map_err(|e| (e, "pointer").into())
.map_err(|k| InstructionError::new(instruction, k))?;
let string: String = to_native(&inputs[1], instruction)?;
let string_bytes = string.as_bytes();
let string_length: i32 = string_bytes.len().try_into().map_err(|_| {
InstructionError::new(
@ -114,19 +91,7 @@ executable_instruction!(
)
})?;
let outputs = allocator.call(&[InterfaceValue::I32(string_length)]).map_err(|_| {
InstructionError::new(
instruction,
InstructionErrorKind::LocalOrImportCall { function_index: allocator_index },
)
})?;
let string_pointer: u32 = to_native::<i32>(&outputs[0], instruction)?.try_into().map_err(|_| {
InstructionError::new(
instruction,
InstructionErrorKind::NegativeValue { subject: "string_pointer" },
)
})?;
let instance = &mut runtime.wasm_instance;
let memory_index: u32 = 0;
let memory_view = instance
.memory(memory_index as usize)
@ -153,7 +118,7 @@ executable_instruction!(
executable_instruction!(
string_size(instruction: Instruction) -> _ {
move |runtime| -> _ {
match runtime.stack.peek1() {
match runtime.stack.pop1() {
Some(InterfaceValue::String(string)) => {
let length = string.len() as i32;
runtime.stack.push(InterfaceValue::I32(length));
@ -165,7 +130,7 @@ executable_instruction!(
instruction,
InstructionErrorKind::InvalidValueOnTheStack {
expected_type: InterfaceType::String,
received_type: value.into(),
received_type: (&value).into(),
},
)),
@ -313,7 +278,11 @@ mod tests {
test_string_lower_memory =
instructions: [
Instruction::ArgumentGet { index: 0 },
Instruction::StringLowerMemory { allocator_index: 43 },
Instruction::StringSize,
Instruction::CallCore { function_index: 43 },
Instruction::ArgumentGet { index: 0 },
Instruction::StringLowerMemory,
],
invocation_inputs: [InterfaceValue::String("Hello, World!".into())],
instance: Instance::new(),
@ -329,7 +298,10 @@ mod tests {
test_string__roundtrip =
instructions: [
Instruction::ArgumentGet { index: 0 },
Instruction::StringLowerMemory { allocator_index: 43 },
Instruction::StringSize,
Instruction::CallCore { function_index: 43 },
Instruction::ArgumentGet { index: 0 },
Instruction::StringLowerMemory,
Instruction::StringLiftMemory,
],
invocation_inputs: [InterfaceValue::String("Hello, World!".into())],
@ -337,70 +309,14 @@ mod tests {
stack: [InterfaceValue::String("Hello, World!".into())],
);
test_executable_instruction!(
test_string_lower_memory__allocator_does_not_exist =
instructions: [Instruction::StringLowerMemory { allocator_index: 43 }],
invocation_inputs: [],
instance: Instance { ..Default::default() },
error: r#"`string.lower_memory 43` the local or import function `43` doesn't exist"#,
);
test_executable_instruction!(
test_string_lower_memory__stack_is_too_small =
instructions: [
Instruction::StringLowerMemory { allocator_index: 43 }
// ^^ `43` expects 1 value on the stack, none is present
Instruction::StringLowerMemory,
],
invocation_inputs: [InterfaceValue::String("Hello, World!".into())],
invocation_inputs: [],
instance: Instance::new(),
error: r#"`string.lower_memory 43` needed to read `1` value(s) from the stack, but it doesn't contain enough data"#,
);
test_executable_instruction!(
test_string_lower_memory__failure_when_calling_the_allocator =
instructions: [
Instruction::ArgumentGet { index: 0 },
Instruction::StringLowerMemory { allocator_index: 153 }
],
invocation_inputs: [InterfaceValue::String("Hello, World!".into())],
instance: {
let mut instance = Instance::new();
instance.locals_or_imports.insert(
153,
LocalImport {
inputs: vec![InterfaceType::I32],
outputs: vec![InterfaceType::I32],
function: |_| Err(()),
// ^^^^^^^ function fails
},
);
instance
},
error: r#"`string.lower_memory 153` failed while calling the local or import function `153`"#,
);
test_executable_instruction!(
test_string_lower_memory__invalid_allocator_signature =
instructions: [
Instruction::ArgumentGet { index: 0 },
Instruction::StringLowerMemory { allocator_index: 153 }
],
invocation_inputs: [InterfaceValue::String("Hello, World!".into())],
instance: {
let mut instance = Instance::new();
instance.locals_or_imports.insert(
153,
LocalImport {
inputs: vec![InterfaceType::I32, InterfaceType::I32],
outputs: vec![],
function: |_| Err(()),
},
);
instance
},
error: r#"`string.lower_memory 153` the local or import function `153` has the signature `[I32] -> [I32]` but it received values of kind `[I32, I32] -> []`"#,
error: r#"`string.lower_memory` needed to read `2` value(s) from the stack, but it doesn't contain enough data"#,
);
test_executable_instruction!(
@ -411,7 +327,7 @@ mod tests {
],
invocation_inputs: [InterfaceValue::String("Hello, World!".into())],
instance: Instance::new(),
stack: [InterfaceValue::String("Hello, World!".into()), InterfaceValue::I32(13)],
stack: [InterfaceValue::I32(13)],
);
test_executable_instruction!(

View File

@ -230,9 +230,7 @@ where
Instruction::I64FromU64 => instructions::i64_from_u64(*instruction),
Instruction::StringLiftMemory => instructions::string_lift_memory(*instruction),
Instruction::StringLowerMemory { allocator_index } => {
instructions::string_lower_memory(*allocator_index, *instruction)
}
Instruction::StringLowerMemory => instructions::string_lower_memory(*instruction),
Instruction::StringSize => instructions::string_size(*instruction),
Instruction::RecordLift { type_index } => {

View File

@ -111,7 +111,12 @@ macro_rules! test_executable_instruction {
let mut instance = $instance;
let run = interpreter.run(&invocation_inputs, &mut instance);
assert!(run.is_ok());
let err = match &run {
Ok(_) => "".to_string(),
Err(e) => e.to_string(),
};
assert!(run.is_ok(), err);
let stack = run.unwrap();