Skip to content

Commit

Permalink
feat: do not add duplicate string constants
Browse files Browse the repository at this point in the history
This means the string_equality benchmark does not crash rlox anymore
(but it crashes clox).
  • Loading branch information
tom-anders committed Mar 10, 2024
1 parent 7ca9de7 commit 4de7ada
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 3 deletions.
7 changes: 5 additions & 2 deletions lib/compiler/src/compiler.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::{fmt::Display, iter::Peekable, mem::size_of, unreachable};
use std::{fmt::Display, iter::Peekable, mem::size_of, ops::Deref, unreachable};

use cursor::Line;
use gc::{Chunk, Closure, ClosureRef, Function, Heap, Object, ObjectRef, Value};
Expand Down Expand Up @@ -1327,7 +1327,10 @@ impl<'a, 'b> Compiler<'a, 'b> {

fn add_identifier_constant(&mut self, token: &Token<'a>) -> Result<u8> {
let name = self.interner.intern(token.lexeme());
self.add_object_constant(name, token)
match self.current_chunk().constants().iter().position(|v| v.equals_string(&name)) {
Some(index) => Ok(index.try_into().unwrap()),
None => self.add_object_constant(name, token),
}
}

fn add_object_constant(&mut self, obj: impl Into<Object>, token: &Token<'a>) -> Result<u8> {
Expand Down
12 changes: 11 additions & 1 deletion lib/gc/src/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ mod closure;
pub use closure::*;

mod upvalue;
use strings::string_interner::StringInterner;
use strings::string_interner::{StringInterner, InternedString};
pub use upvalue::*;

mod class;
Expand Down Expand Up @@ -66,6 +66,16 @@ impl Value {
}
}

pub fn equals_string(&self, s: &InternedString) -> bool {
match self {
Value::Object(o) => match o.deref() {
Object::String(string) => string == s,
_ => false,
},
_ => false,
}
}

pub fn less_than(&self, other: &Value) -> Option<Value> {
match (self, other) {
(Value::Number(a), Value::Number(b)) => Some(Value::Boolean(a < b)),
Expand Down

0 comments on commit 4de7ada

Please sign in to comment.