patterncppModerate
Brainfuck on-the-fly interpreter in C++
Viewed 0 times
brainfucktheinterpreterfly
Problem
I was bored yesterday morning, so I wrote a brainfuck interpreter.
I know there are a lot, but this one is different. Why? Because it evaluates brainfuck code on the fly, reading from the input file, and eliminates code once it's not reachable anymore, instead of reading the whole program, analyzing it, and evaluating it.
I wrote it in C++ because that way I didn't have to re-write a vector, a stack...
I'm aware of a few bad program design practices:
I've compiled it with g++ in c++11 mode:
To run a program:
I've tested successfully with all the files in here.
I'm concerned about:
The code
```
#include
#include
#include
#include
#define STACK_INITIAL_SIZE 300
#define DEAD_CODE_LIMIT 100
typedef unsigned char byte;
enum class State {
READING,
WHILE_IGNORING
};
int main(int argc, char** argv) {
if ( argc " tokens;
size_t current_token_index = 0;
/** Store the loop entry points */
std::stack entry_points;
/** Store the states, to allow multiple looping */
std::stack states;
/** Start reading */
states.push(State::READING);
while ( true ) {
/** Declare the token */
char token;
/** Get the current state */
State state = states.top();
/** Realloc if we have not enough space, allocate 2 stack_size /
I know there are a lot, but this one is different. Why? Because it evaluates brainfuck code on the fly, reading from the input file, and eliminates code once it's not reachable anymore, instead of reading the whole program, analyzing it, and evaluating it.
I wrote it in C++ because that way I didn't have to re-write a vector, a stack...
I'm aware of a few bad program design practices:
- It's monolithic, even though I find it readable... It's just a brainfuck interpreter though, so instead of using a context struct whith all the program data and split it, I found it easier to continue the main function.
- Use of C functions (
getchar/putchar) in evaluation instead of using C++'s streams. I think this way is more readable though.
I've compiled it with g++ in c++11 mode:
$ g++ -std=c++11 -Wall bf.cc -o bfTo run a program:
$ ./bf program.bfI've tested successfully with all the files in here.
I'm concerned about:
- Performance: I made it to be faster than most of interpreters out there. Any performance implication to make it run faster?
- Readability: I think even though is monolithic, it's readable enough. Is it true?
- Any bug/memory leak I could have missed.
The code
```
#include
#include
#include
#include
#define STACK_INITIAL_SIZE 300
#define DEAD_CODE_LIMIT 100
typedef unsigned char byte;
enum class State {
READING,
WHILE_IGNORING
};
int main(int argc, char** argv) {
if ( argc " tokens;
size_t current_token_index = 0;
/** Store the loop entry points */
std::stack entry_points;
/** Store the states, to allow multiple looping */
std::stack states;
/** Start reading */
states.push(State::READING);
while ( true ) {
/** Declare the token */
char token;
/** Get the current state */
State state = states.top();
/** Realloc if we have not enough space, allocate 2 stack_size /
Solution
Looks great! Just a few (small) suggestions:
Style
Technicalities
Design
This is perhaps the most glaring thing I see in your code:
Preaching time: Manual memory (and resource) management should be incredibly rare (it should be limited pretty much to library-like containers, if even there).
For resources, there's always the RAII approach, and for memory, there's
To way oversimplify it, memory management (or more abstractly resource management) is incredibly difficult to get right in all but the smallest and simplest snippets of code. Luckily classes provide a nice, isolated place to tuck the complications away and more easily reason about behavior.
Style
- It really is quite readable and straightforward, but breaking out a few functions certainly wouldn't hurt.
- Even though you're not using
using namespace std;(woo!), the variable namestackstill makes me a bit uncomfortable (then again, I can't think of a name for it that wouldn't end up being gross).
- If you do end up pulling things into functions, you could orient the program reading around templated iterators (using an istream_iterator to accomplish what you're doing now). That would allow you to stream the program or to store it up front. That doesn't really accomplish much on its own, I suppose, but just figured it was worth noting since breaking it into smaller functions without doing it that way would quickly get quite messy.
STACK_INITIAL_SIZEandDEAD_CODE_LIMITshould beconst intconstants instead of macros.
- Super subjective thing, but alphabetized includes can be a bit easier to mentally scan through.
Technicalities
size_tshould technically bestd::size_t, though it doesn't realistically matter.
- You missed the include for
cstdio.
getcharandputcharshould bestd::getcharandstd::putchar.
Design
This is perhaps the most glaring thing I see in your code:
stack should be a std::vector. In fact, it pretty much already is a manually managed vector. Performance will be the same as long as you're careful, and as a bonus, quite a bit of code should fall away (resizing, zeroing out, etc).Preaching time: Manual memory (and resource) management should be incredibly rare (it should be limited pretty much to library-like containers, if even there).
For resources, there's always the RAII approach, and for memory, there's
std::shared_ptr and std::unique_ptr (you code could have used std::unique_ptr for example).To way oversimplify it, memory management (or more abstractly resource management) is incredibly difficult to get right in all but the smallest and simplest snippets of code. Luckily classes provide a nice, isolated place to tuck the complications away and more easily reason about behavior.
Context
StackExchange Code Review Q#84333, answer score: 12
Revisions (0)
No revisions yet.