A first impl of CharTable #106

hesampakdaman · 2025-01-29T19:42:26Z

Fixes #105

This is a first iteration of char table to get things tarted. It is obviously not what we want in the end, especially since this implementation uses std::collections::HashMap under the hood. There's also functionality still missing, such as

setting purpose
there are no extra slots

hesampakdaman · 2025-01-29T19:44:53Z

src/data.rs

@@ -335,6 +335,9 @@ pub(crate) fn aset<'ob>(
                Err(anyhow!("index {idx} is out of bounds. Length was {len}"))
            }
        }
+        ObjectType::CharTable(_) => {


I'm not too sure how to get a mutable view of LispCharTable.

for now, you can just use a RefCell.

Do you mean that we should have something like

#[derive(PartialEq, Eq, Trace, Debug)] pub(crate) struct CharTable(GcHeap<RefCell<CharTableInner<'static>>>);

In this case I get into issues with Trace not being implemented for RefCell. Of course we could add this

impl<T: Trace> Trace for RefCell<T> { fn trace(&self, state: &mut GcState) { self.borrow().trace(state); } }

I committed this. Let me know what you think.

I am saying you can do this

pub struct CharTableInner<'ob> { parent: Option<Slot<&'ob CharTable>>, data: RefCell<HashMap<usize, Object<'ob>>>, init: Option<Slot<Object<'ob>>>, }

Slot has a get and set method to modify it. So we only need RefCell for data.

hesampakdaman · 2025-01-29T19:45:35Z

src/data.rs

@@ -392,6 +399,7 @@ fn type_of(object: Object) -> Object {
        ObjectType::String(_) | ObjectType::ByteString(_) => sym::STRING.into(),
        ObjectType::SubrFn(_) => sym::SUBR.into(),
        ObjectType::Buffer(_) => sym::BUFFER.into(),
+        ObjectType::CharTable(_) => todo!(),


I tried changing the sym module, but it doesn't quite work as I was pointed to a file outside of the project.

the sym module is created as part of build.rs. you can declare a new symbol with the defsym macro and it will be added to the sym module (example).

Got it! This should now be fixed.

hesampakdaman · 2025-01-29T20:02:10Z

src/data.rs

@@ -374,6 +377,10 @@ pub(crate) fn aref<'ob>(array: Object<'ob>, idx: usize, cx: &'ob Context) -> Res
            Some(x) => Ok(x),
            None => Err(anyhow!("index {idx} is out of bounds")),
        },
+        ObjectType::CharTable(chartab) => match chartab.get(&idx) {
+            Some(x) => Ok(x),
+            None => Err(anyhow!("index {idx} is out of bounds")),


This will actually never happen in the current implementation since we currently return NIL. Maybe we don't want that?

we can return NIL on the None branch (example).

I changed it so that in chartab.get(..) we return None instead of NIL. When we match against None here, we now return NIL instead.

CeleritasCelery · 2025-01-29T23:38:33Z

src/data.rs

@@ -392,6 +399,7 @@ fn type_of(object: Object) -> Object {
        ObjectType::String(_) | ObjectType::ByteString(_) => sym::STRING.into(),
        ObjectType::SubrFn(_) => sym::SUBR.into(),
        ObjectType::Buffer(_) => sym::BUFFER.into(),
+        ObjectType::CharTable(_) => todo!(),


the sym module is created as part of build.rs. you can declare a new symbol with the defsym macro and it will be added to the sym module (example).

CeleritasCelery · 2025-01-29T23:39:40Z

src/data.rs

@@ -374,6 +377,10 @@ pub(crate) fn aref<'ob>(array: Object<'ob>, idx: usize, cx: &'ob Context) -> Res
            Some(x) => Ok(x),
            None => Err(anyhow!("index {idx} is out of bounds")),
        },
+        ObjectType::CharTable(chartab) => match chartab.get(&idx) {
+            Some(x) => Ok(x),
+            None => Err(anyhow!("index {idx} is out of bounds")),


we can return NIL on the None branch (example).

CeleritasCelery · 2025-01-29T23:50:34Z

src/data.rs

@@ -335,6 +335,9 @@ pub(crate) fn aset<'ob>(
                Err(anyhow!("index {idx} is out of bounds. Length was {len}"))
            }
        }
+        ObjectType::CharTable(_) => {


for now, you can just use a RefCell.

CeleritasCelery · 2025-01-29T23:52:23Z

src/core/object/chartab.rs

+        Self(GcHeap::new(table, constant))
+    }
+
+    pub(super) fn display_walk(


display_walk is used when we have recursive data structures like lists or vectors. For something like a char-table which is not recursive, we can just Implement Display.

This is now fixed!

Just noticed that we want these sorted. I just do that in the display function itself.

CeleritasCelery · 2025-01-29T23:57:12Z

src/core/object/chartab.rs

+}
+
+#[derive(PartialEq, Eq, Trace, Debug)]
+pub(crate) struct LispCharTable(GcHeap<CharTable<'static>>);


I generally only use the Lisp prefix for thing that already have a rust analog (i.e. String, Vec, HashMap, etc). For this we can change LispCharTable to CharTable and CharTable to something like CharTableData or CharTableInner. We can even make the inner struct private and only expose methods on the top level struct if we want.

Renamed LispCharTable to CharTable. I opted for the inner struct to CharTableInner since we have that same name pattern in other places.

I tried making the CharTableInner private. However, the IntoObject trait is implemented in tagged.rs hinders me from doing so.

CeleritasCelery · 2025-01-31T20:33:21Z

Thanks!

hesampakdaman and others added 5 commits January 29, 2025 19:24

A first implementation of CharTable

3fad910

Implement Trace, CloneIn, and IntoObject for CharTable

f7008e3

Implement display_walk for LispChartable

9813607

Create todo!() aset/aref for ObjectType::CharTable

ed485fe

Implemented aref for LispCharTable

15deeb7

hesampakdaman commented Jan 29, 2025

View reviewed changes

Fix clippy elided lifetime

e31bf1f

hesampakdaman commented Jan 29, 2025

View reviewed changes

CeleritasCelery reviewed Jan 29, 2025

View reviewed changes

hesampakdaman added 4 commits January 30, 2025 21:21

Rename LispCharTable and CharTable

04cfa51

Return NIL in data.rs instead of get in chartab

ffbfb87

defsym! CHAR_TABLE

817be8d

Implement Display for CharTable and remove display_walk

532874f

hesampakdaman requested a review from CeleritasCelery January 30, 2025 22:05

Wrap data of CharTableInner with RefCell

273473c

hesampakdaman force-pushed the master branch from 226c65a to 273473c Compare January 31, 2025 10:47

CeleritasCelery merged commit a97f4eb into CeleritasCelery:master Jan 31, 2025
10 checks passed

hesampakdaman deleted the master branch January 31, 2025 22:17

hesampakdaman restored the master branch January 31, 2025 22:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

A first impl of CharTable #106

A first impl of CharTable #106

hesampakdaman commented Jan 29, 2025 •

edited

Loading

hesampakdaman Jan 29, 2025

CeleritasCelery Jan 29, 2025

hesampakdaman Jan 30, 2025

CeleritasCelery Jan 30, 2025

hesampakdaman Jan 31, 2025

hesampakdaman Jan 29, 2025

CeleritasCelery Jan 29, 2025

hesampakdaman Jan 30, 2025

hesampakdaman Jan 29, 2025

CeleritasCelery Jan 29, 2025

hesampakdaman Jan 30, 2025

CeleritasCelery Jan 29, 2025

CeleritasCelery Jan 29, 2025

CeleritasCelery Jan 29, 2025

CeleritasCelery Jan 29, 2025

hesampakdaman Jan 30, 2025

hesampakdaman Jan 31, 2025

CeleritasCelery Jan 29, 2025

hesampakdaman Jan 30, 2025 •

edited

Loading

CeleritasCelery commented Jan 31, 2025

A first impl of CharTable #106

A first impl of CharTable #106

Conversation

hesampakdaman commented Jan 29, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hesampakdaman Jan 30, 2025 • edited Loading

Choose a reason for hiding this comment

CeleritasCelery commented Jan 31, 2025

hesampakdaman commented Jan 29, 2025 •

edited

Loading

hesampakdaman Jan 30, 2025 •

edited

Loading