improve optimisation thoroughness

README.md

@@ -30,15 +30,15 @@ cargo install --git https://git.disroot.org/phyto/braintal
 
 ## Features/completeness
 
-Braintal is _probably_ able to convert any arbitrary brainfuck code into uxn, so long as it doesn't use up enough memory as to corrupt its own code, and the code can fit into a uxn rom. It should warn you in the latter case (though not the former, so be careful with how much memory your programs use).
+Braintal is able to convert any arbitrary brainfuck code into uxn, so long as it doesn't use up enough memory as to corrupt its own code, and the code can fit into a uxn rom. It should warn you in the latter case (though not the former, so be careful with how much memory your programs use).
 
-This is all still a bit up in the air, though.
+If you can find a piece of brainfuck code that runs natively but not as uxn from this program, and doesn't exceed 30,000 bytes of memory, and you don't get warned about it being too large, then please make an issue or contact me!
 
-Some of the optimisations probably __will remove loops that hang indefinitely__ (e.g. `+[+-]` would become `+`).
+Some of the optimisations probably __will remove loops that hang indefinitely__ (e.g. `+[+-]` would become `+`). However, they should not otherwise mangle the functionality of the code.
 
 ## Speed
 
-The resulting uxn is inherently quite slow, though some optimisations are applied to the brainfuck in an attempt to lessen this.
+The resulting uxn is inherently quite slow, especially when faced off against an optimising bf interpreter, though some optimisations are applied to the brainfuck in an attempt to lessen this.
 
 # Todo
 - [ ] Optional uxn minification (sacrifice speed for space)

src/bf/optimise.rs

@@ -2,46 +2,49 @@ use super::Op;
 
 /// Do the bare minimum optimisation needed for conversion to Tal
 pub fn minimum(ops: &mut Vec<Op>) {
-	sanitise(ops)
+	sanitise(ops);
 }
 
 /// Apply reasonable optimisations
 pub fn normal(ops: &mut Vec<Op>) {
-	while merge_adjacent_adds(ops) {}
-	while merge_adjacent_seeks(ops) {}
-	remove_trailing_deadweight(ops);
+	while merge_adjacent_adds(ops) ||
+		merge_adjacent_seeks(ops) ||
+		remove_trailing_deadweight(ops) ||
 
 	//TODO:
 	// Remove comments: [-][ whateve ] => [-]
 	// Copy, Mul...
 
-	purge_comments(ops);
+	set(ops) ||
+	purge_comments(ops) ||
 
-	set(ops);
 	// Cleanup
-	sanitise(ops);
+	sanitise(ops)
+	{}
 }
 
 pub fn experimental(ops: &mut Vec<Op>) {
 	normal(ops);
 
-	set(ops);
-
 	sanitise(ops);
 }
 
-fn set(ops: &mut Vec<Op>) {
+fn set(ops: &mut Vec<Op>) -> bool {
+	let mut acted = false;
 	for i in 0..ops.len() - 2 {
 		use Op::*;
 		if let [LoopStart(..), Add(..), LoopEnd(..)] = ops[i..i + 3] {
 			ops[i] = Set(0);
 			ops[i + 1] = Nop;
 			ops[i + 2] = Nop;
+			acted = true;
 		}
 	}
+	acted
 }
 
-fn remove_trailing_deadweight(ops: &mut Vec<Op>) {
+fn remove_trailing_deadweight(ops: &mut Vec<Op>) -> bool {
+	let mut acted = false;
 	let mut count = 0;
 	for v in ops.iter().rev() {
 		use Op::*;
@@ -52,10 +55,13 @@ fn remove_trailing_deadweight(ops: &mut Vec<Op>) {
 	}
 	for _ in 0..count {
 		ops.pop();
+		acted = true;
 	}
+	acted
 }
 
-fn purge_comments(ops: &mut [Op]) {
+fn purge_comments(ops: &mut [Op]) -> bool {
+	let mut acted = false;
 	use Op::*;
 	// Remove starting comment
 	if let LoopStart(..) = ops[0] {
@@ -64,11 +70,26 @@ fn purge_comments(ops: &mut [Op]) {
 				break;
 			}
 			*v = Nop;
+			acted = true;
 		}
 	}
+
+	// Remove known-zero comments
+	for i in 0..ops.len() - 1 {
+		use Op::*;
+		if let [Set(0), LoopStart(x)] = ops[i..i + 2] {
+			dbg!(i);
+			for j in i + 1..=x {
+				ops[j] = Nop;
+				acted = true;
+			}
+		}
+	}
+	acted
 }
 
-fn remove_nops(ops: &mut Vec<Op>) {
+fn remove_nops(ops: &mut Vec<Op>) -> bool {
+	let mut acted = false;
 	let mut len = ops.len();
 	let mut i = 0;
 	loop {
@@ -80,10 +101,12 @@ fn remove_nops(ops: &mut Vec<Op>) {
 		if let Nop | Add(0) | Seek(0) = ops[i] {
 			ops.remove(i);
 			len -= 1;
+			acted = true;
 		} else {
 			i += 1;
 		}
 	}
+	acted
 }
 
 fn merge_adjacent_adds(ops: &mut Vec<Op>) -> bool {
@@ -128,8 +151,8 @@ fn merge_adjacent_seeks(ops: &mut Vec<Op>) -> bool {
 /// Given a list of instructions, this function will set the adress of each [LoopStart] and [LoopEnd] to the corresponding one.
 /// Also removes any Nop instructions.
 /// Here be dragons.
-fn sanitise(ops: &mut Vec<Op>) {
-	remove_nops(ops);
+fn sanitise(ops: &mut Vec<Op>) -> bool {
+	let acted = remove_nops(ops);
 	let mut newops = ops.clone();
 	//FIXME: this could do it in less passes
 	'outer: for (i, op) in ops.iter().enumerate() {
@@ -154,4 +177,5 @@ fn sanitise(ops: &mut Vec<Op>) {
 	}
 
 	*ops = newops;
+	acted
 }

tests/samples/simple.bf

@@ -1 +1,3 @@
--[-.]
+-[-.]++
+[-][i'm a comment, and i wont do anythnig - ill probably be removed though!]
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++.