Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
144 changes: 126 additions & 18 deletions bft/bft_interp/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
use std::io::{Read, Write};
use std::num::NonZeroUsize;

use bft_types::{DecoratedInstruction, DecoratedProgram, PositionedInstruction};
use bft_types::{DecoratedInstruction, DecoratedProgram, PositionedInstruction, RawInstruction};

use thiserror::Error;

Expand All @@ -19,6 +19,8 @@ pub trait CellKind: std::clone::Clone + Default {
fn set_value(&mut self, value: u8);
/// Gets the cell's value as a single byte
fn get_value(&self) -> u8;
/// Returns whether the cell's value is equal to zero
fn is_zero(&self) -> bool;
}

impl CellKind for u8 {
Expand All @@ -35,6 +37,10 @@ impl CellKind for u8 {
fn get_value(&self) -> u8 {
*self
}

fn is_zero(&self) -> bool {
*self == 0
}
}
/// A brainfuck virtual machine
///
Expand Down Expand Up @@ -91,6 +97,10 @@ impl<'a, T> Machine<'a, T> {
self.prog().decorated_instructions()[self.instruction_pointer]
}

fn next_instruction(&self) -> usize {
self.instruction_pointer + 1
}

Comment on lines +100 to +103
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function feels a little "extra" but I don't hate it.

/// Decrements the memory pointer
///
/// If doing so would cause the memory pointer to become negative, it instead returns a [VMError::SeekTooLow]
Expand Down Expand Up @@ -118,14 +128,14 @@ impl<'a, T> Machine<'a, T> {
/// ```
/// TODO! Come back here when moving the head is more useful
/// TODO! Once I can run programs, decide whether I want to allow external mutation of program state
pub fn seek_left(&mut self) -> Result<(), VMError> {
pub fn seek_left(&mut self) -> Result<usize, VMError> {
if self.head == 0 {
Err(VMError::SeekTooLow(
self.current_instruction().instruction(),
))
} else {
self.head -= 1;
Ok(())
Ok(self.next_instruction())
}
}
}
Expand Down Expand Up @@ -203,7 +213,7 @@ where
/// ```
/// TODO! Come back here when moving the head is more useful
/// TODO! Once I can run programs, decide whether I want to allow external mutation of program state
pub fn seek_right(&mut self) -> Result<(), VMError> {
pub fn seek_right(&mut self) -> Result<usize, VMError> {
if self.head + 1 == self.cells.len() {
if !self.may_grow {
return Err(VMError::SeekTooHigh(
Expand All @@ -214,7 +224,11 @@ where
}
}
self.head += 1;
Ok(())
Ok(self.next_instruction())
}

pub fn current_cell(&mut self) -> &mut T {
&mut self.cells[self.head]
}
Comment on lines +230 to 232
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not clear why you need this function, using this can cause borrow problems in functions in your impl blocks, because to call it, everything inside the instance must be consistent and unborrowed - I imagine you're not having a problem right now, but in general I try and avoid this kind of function unless it's critical to your API. Do you need this to be public? If not, perhaps it need not exist.

If you're not quite sure why I'm saying this is a potential problem; then think more about what it means to be able to call a method on a struct in terms of the borrows. If you're still stuck, poke me IRL and I'll try to explain it another way.


/// Increase the value of the cell at the data pointer
Expand All @@ -231,8 +245,9 @@ where
/// interp.increment_cell();
/// assert_eq!(interp.cells()[0], 1);
/// ```
pub fn increment_cell(&mut self) {
self.cells[self.head].increment()
pub fn increment_cell(&mut self) -> Result<usize, VMError> {
self.current_cell().increment();
Ok(self.next_instruction())
}

/// Decrease the value of the cell at the data pointer
Expand All @@ -249,8 +264,9 @@ where
/// interp.decrement_cell();
/// assert_eq!(interp.cells()[0], 255);
/// ```
pub fn decrement_cell(&mut self) {
self.cells[self.head].decrement()
pub fn decrement_cell(&mut self) -> Result<usize, VMError> {
self.current_cell().decrement();
Ok(self.next_instruction())
}

/// Read a value from `file` into memory at the memory pointer
Expand All @@ -270,12 +286,12 @@ where
/// assert_eq!(interp.cells()[0], 7);
/// ```
/// TODO: More examples?
pub fn read_value(&mut self, file: &mut impl Read) -> Result<(), VMError> {
pub fn read_value(&mut self, file: &mut impl Read) -> Result<usize, VMError> {
let mut buffer: [u8; 1] = [0; 1];
match file.read_exact(&mut buffer) {
Ok(()) => {
self.cells[self.head].set_value(buffer[0]);
Ok(())
self.current_cell().set_value(buffer[0]);
Ok(self.next_instruction())
}
Err(ioerror) => Err(VMError::IOError {
instruction: self.current_instruction().instruction(),
Expand Down Expand Up @@ -305,13 +321,82 @@ where
/// interp.write_value(&mut data);
/// assert_eq!(data.get_ref()[1], 7);
/// ```
pub fn write_value(&mut self, file: &mut impl Write) -> Result<(), VMError> {
pub fn write_value(&mut self, file: &mut impl Write) -> Result<usize, VMError> {
let mut buffer: [u8; 1] = [0; 1];
buffer[0] = self.cells[self.head].get_value();
file.write_all(&buffer).map_err(|e| VMError::IOError {
instruction: self.current_instruction().instruction(),
source: e,
})
buffer[0] = self.current_cell().get_value();
file.write_all(&buffer)
.and_then(|_| file.flush())
.map_err(|e| VMError::IOError {
instruction: self.current_instruction().instruction(),
source: e,
})
.and(Ok(self.next_instruction()))
Comment on lines +327 to +333
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a really nice literate handling of the write/flush/map_err process, well done.

}

pub fn open_loop(&mut self) -> Result<usize, VMError> {
if self.current_cell().is_zero() {
assert!(matches!(
self.current_instruction(),
DecoratedInstruction::OpenLoop { .. }
));
Comment on lines +338 to +341
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Surely this function won't be called unless the current instruction is an openloop?

match self.current_instruction() {
DecoratedInstruction::OpenLoop {
instruction: _,
closer,
} => Ok(self
.prog
.position_to_index(closer.line(), closer.character())
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a conversion which could have been precomputed and stored in the instruction stream.

While I wouldn't always advocate precomputing things, in this instance the use of this data will be frequent and on the critical path of most BF programs.

+ 1),
_ => unreachable!(),
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The assert/matches above is also handled here since unreachable!() will panic if it's ever reached.

}
} else {
Ok(self.next_instruction())
}
}

pub fn close_loop(&mut self) -> Result<usize, VMError> {
assert!(matches!(
self.current_instruction(),
DecoratedInstruction::CloseLoop { .. }
));
Comment on lines +358 to +361
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comments as above re: assert/matches and the unreachable below


match self.current_instruction() {
DecoratedInstruction::CloseLoop {
instruction: _,
opener,
} => Ok(self
.prog()
.position_to_index(opener.line(), opener.character())),
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And for this computation step

_ => unreachable!(),
}
}

pub fn interpret(
&mut self,
input: &mut impl Read,
output: &mut impl Write,
) -> Result<(), VMError> {
while self.instruction_pointer < self.prog().decorated_instructions().len() {
self.instruction_pointer = self.interpret_current_instruction(input, output)?
}
Ok(())
}

fn interpret_current_instruction(
&mut self,
input: &mut impl Read,
output: &mut impl Write,
) -> Result<usize, VMError> {
match self.current_instruction().instruction().instruction() {
RawInstruction::OpenLoop => self.open_loop(),
RawInstruction::CloseLoop => self.close_loop(),
RawInstruction::DecrementDataPointer => self.seek_left(),
RawInstruction::IncrementDataPointer => self.seek_right(),
RawInstruction::IncrementByte => self.increment_cell(),
RawInstruction::DecrementByte => self.decrement_cell(),
RawInstruction::GetByte => self.read_value(input),
RawInstruction::PutByte => self.write_value(output),
}
Comment on lines +380 to +399
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally I'd not have had a separate method for interpreting a single instruction, though this isn't entirely bad.

}
}

Expand All @@ -328,3 +413,26 @@ pub enum VMError {
source: std::io::Error,
},
}

#[cfg(test)]
mod tests {
use super::*;

/// Test that a simple "hello world" program works
#[test]
fn test_hello_world() {
use bft_types::{DecoratedProgram, Program};
let hello_world_text =
">++++++++[<+++++++++>-]<.>++++[<+++++++>-]<+.+++++++..+++.>>++++++[<+++++++>-]<+
+.------------.>++++++[<+++++++++>-]<+.<.+++.------.--------.>>>++++[<++++++++>-
]<+.";
let mut input = std::io::Cursor::new(Vec::new());
let mut output = std::io::Cursor::new(Vec::new());
let prog = Program::new("<no program>", &hello_world_text);
let decorated = DecoratedProgram::from_program(&prog).unwrap();
let mut machine: Machine<u8> = Machine::new(None, false, &decorated);
let result = machine.interpret(&mut input, &mut output);
assert!(result.is_ok());
assert_eq!(output.into_inner(), "Hello, World!".as_bytes());
}
}
23 changes: 22 additions & 1 deletion bft/bft_types/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,14 @@ impl DecoratedInstruction {
Self::PlaceholderOpenBracket => unreachable!(),
}
}

pub fn line(&self) -> usize {
self.instruction().line()
}

pub fn character(&self) -> usize {
self.instruction().character()
}
}

impl fmt::Display for DecoratedInstruction {
Expand All @@ -138,7 +146,20 @@ pub struct DecoratedProgram {
file: PathBuf,
decorated_instructions: Vec<DecoratedInstruction>,
}

impl DecoratedProgram {
pub fn position_to_index(&self, line: usize, character: usize) -> usize {
self.decorated_instructions
.binary_search_by(|instruction| {
instruction
.line()
.cmp(&line)
.then(instruction.character().cmp(&character))
})
.unwrap()
// >:| I can't just search for the DecoratedInstruction because I don't store it
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not store an index -> index map instead (perhaps as a Vec<usize>)?

//
}
}
impl fmt::Display for DecoratedProgram {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
for instruction in self.decorated_instructions() {
Expand Down
1 change: 1 addition & 0 deletions bft/cat.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
,[.,]
22 changes: 22 additions & 0 deletions bft/hello-wait.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
This is your input file

Anything which is not one of the important characters will be ignored
by the program

The program is as follows:

+[-
[<<
[+
[--->]
-[<<<]
]
]>>>-
]

I'm guessing that you know where we're going with all this

>-.---.>..>.<<<<-.<+.>>>>>.>.<<.<-.

But perhaps you're as lost as a little lamb?
,.,.,.,.
9 changes: 7 additions & 2 deletions bft/src/cli.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
use clap::Parser;
use std::{num::NonZeroUsize, path::PathBuf};
use std::{
io::{stdin, stdout},
num::NonZeroUsize,
path::PathBuf,
};

use bft_interp::Machine;
use bft_types::{DecoratedProgram, Program};
Expand All @@ -18,6 +22,7 @@ pub(crate) fn run_bft() -> Result<(), Box<dyn std::error::Error>> {
let args = Cli::parse();
let prog = Program::from_file(&args.program)?;
let decorated = DecoratedProgram::from_program(&prog)?;
let _machine: Machine<u8> = Machine::new(args.cells, args.extensible, &decorated);
let mut machine: Machine<u8> = Machine::new(args.cells, args.extensible, &decorated);
machine.interpret(&mut stdin(), &mut stdout())?;
Ok(())
}