From 9132eecc060ba66487578bd6dc3df0f51a7c0622 Mon Sep 17 00:00:00 2001 From: Marek Lipert Date: Sat, 30 May 2026 10:31:15 +0200 Subject: [PATCH 1/2] Fix memory management and namespace interning. --- backend/runtime/Namespace.c | 293 +++++++++++++--------- backend/runtime/Namespace.h | 4 +- backend/runtime/PersistentArrayMap.c | 1 + backend/runtime/Var.c | 5 +- backend/runtime/tests/Namespace_test.c | 101 ++++++-- backend/state/ThreadsafeCompilerState.cpp | 41 ++- frontend/resources/rt-classes.edn | 4 +- 7 files changed, 290 insertions(+), 159 deletions(-) diff --git a/backend/runtime/Namespace.c b/backend/runtime/Namespace.c index 2e0c4e2..e541cd4 100644 --- a/backend/runtime/Namespace.c +++ b/backend/runtime/Namespace.c @@ -1,8 +1,11 @@ #include "Namespace.h" #include "ConcurrentHashMap.h" +#include "Ebr.h" #include "Exceptions.h" #include "Object.h" +#include "ObjectProto.h" #include "PersistentArrayMap.h" +#include "RTValue.h" #include "String.h" #include "Symbol.h" #include "Var.h" @@ -54,18 +57,12 @@ Namespace *Namespace_findOrCreate(Symbol *name) { Namespace *ns = (Namespace *)RT_unboxPtr(nsVal); return ns; } - + Ptr_retain(name); Namespace *newNs = Namespace_create(name); - Symbol *keySym = newNs->name; - Ptr_retain(keySym); - RTValue keyVal = RT_boxSymbol((Object *)keySym); - Ptr_retain(newNs); - RTValue valVal = RT_boxPtr((Object *)newNs); - RTValue existing = ConcurrentHashMap_putIfAbsent_preservesSelf( - namespaces, keyVal, valVal, false); + namespaces, RT_boxPtr(name), RT_boxPtr(newNs), false); if (!RT_isNil(existing)) { Ptr_release(newNs); return (Namespace *)RT_unboxPtr(existing); @@ -96,11 +93,11 @@ void Namespace_destroy(Namespace *self, bool deallocateChildren) { } RTValue mappings = atomic_load_explicit(&self->mappings, memory_order_acquire); - release(mappings); + autorelease(mappings); RTValue aliases = atomic_load_explicit(&self->aliases, memory_order_acquire); - release(aliases); + autorelease(aliases); } } @@ -122,13 +119,14 @@ bool Namespace_equals(Namespace *self, Namespace *other) { * `#namespace[name]`. Consumes the `self` argument and returns a new String. */ String *Namespace_toString(Namespace *self) { - String *nameStr = Symbol_toString(RT_boxSymbol(self->name)); + Ptr_retain(self->name); + String *nameStr = toString(RT_boxPtr(self->name)); String *prefix = String_create("#namespace["); String *suffix = String_create("]"); String *res1 = String_concat(prefix, nameStr); String *res2 = String_concat(res1, suffix); - + Ptr_release(self); return res2; } @@ -172,31 +170,18 @@ RTValue Namespace_getAliases(Namespace *self) { * under the exact symbol `sym`. * Consumes `self`, `sym`, and `o` arguments. */ -bool Namespace_isInternedMapping(Namespace *self, Symbol *sym, RTValue o) { +bool Namespace_isInternedMapping(Namespace *self, Symbol *sym, RTValue var) { bool res = false; - if (getType(o) == varType) { - Var *v = (Var *)RT_unboxPtr(o); + if (getType(var) == varType) { + Var *v = (Var *)RT_unboxPtr(var); res = (v->ns == self) && Symbol_equals(v->sym, sym); } Ptr_release(self); Ptr_release(sym); - release(o); + release(var); return res; } -/* - * Validates whether an existing mapping (`oldVal`) can be safely replaced by a - * new mapping (`neuVal`). Throws an IllegalStateException if an attempt is made - * to overwrite a Var interned in this namespace. Consumes all arguments: - * `self`, `sym`, `oldVal`, and `newVal`. - */ -bool Namespace_checkReplacement(Namespace *self, Symbol *sym, RTValue oldVal, - RTValue newVal) { - // In Clojure, once a mapping is established, it cannot be replaced with a - // different value/Var/Class. - return false; -} - /* * Interns a symbol in this namespace, establishing a new Var if one does not * exist, or returning the existing Var if already mapped. @@ -218,95 +203,155 @@ Var *Namespace_intern(Namespace *self, Symbol *sym) { throwIllegalArgumentException_C("Can't intern namespace-qualified symbol"); } - RTValue symVal = RT_boxSymbol((Object *)sym); - Var *v = NULL; - - while (true) { - RTValue mapVal = - atomic_load_explicit(&self->mappings, memory_order_acquire); - PersistentArrayMap *map = (PersistentArrayMap *)RT_unboxPtr(mapVal); + RTValue mapVal = atomic_load_explicit(&self->mappings, memory_order_acquire); + PersistentArrayMap *map = (PersistentArrayMap *)RT_unboxPtr(mapVal); // 0 - Ptr_retain(map); - retain(symVal); - RTValue existingVar = PersistentArrayMap_get(map, symVal); + Var *v = NULL; - if (!RT_isNil(existingVar)) { - Ptr_retain(self); - Ptr_retain(sym); - retain(existingVar); - if (Namespace_isInternedMapping(self, sym, existingVar)) { - if (v != NULL) { - release(RT_boxPtr((Object *)v)); - } - Ptr_release(self); - Ptr_release(sym); - return (Var *)RT_unboxPtr(existingVar); - } - } + Ptr_retain(map); // 1 + Ptr_retain(sym); + RTValue o = PersistentArrayMap_get(map, RT_boxPtr(sym)); // 0 + while (RT_isNil(o)) { if (v == NULL) { Ptr_retain(self); Ptr_retain(sym); v = Var_create_interned(self, sym); } - RTValue newVal = RT_boxPtr((Object *)v); - if (!RT_isNil(existingVar)) { - Ptr_retain(self); - Ptr_retain(sym); - retain(existingVar); - retain(newVal); - if (!Namespace_checkReplacement(self, sym, existingVar, newVal)) { - retain(existingVar); - String *oStr = toString(existingVar); + Ptr_retain(v); + Ptr_retain(map); // 1 + Ptr_retain(sym); + PersistentArrayMap *newMap = + PersistentArrayMap_assoc(map, RT_boxPtr(sym), RT_boxPtr(v)); // 0 + + bool replaced = atomic_compare_exchange_strong_explicit( + &self->mappings, &mapVal, RT_boxPtr(newMap), memory_order_acq_rel, + memory_order_acquire); + if (replaced) { + autorelease(mapVal); // -1 + } else { + Ptr_release(newMap); // 0 + } + mapVal = atomic_load_explicit(&self->mappings, memory_order_acquire); + map = (PersistentArrayMap *)RT_unboxPtr(mapVal); // 0 - Ptr_retain(oStr); - String *flatOStr = String_compactify(oStr); + Ptr_retain(map); // 1 + Ptr_retain(sym); + o = PersistentArrayMap_get(map, RT_boxPtr(sym)); // 0 + } - char buf[512]; - snprintf(buf, sizeof(buf), "%s already refers to: %s in namespace: %s", - String_c_str(sym->name), String_c_str(flatOStr), - String_c_str(self->name->name)); + Ptr_retain(self); + Ptr_retain(sym); + retain(o); + if (Namespace_isInternedMapping(self, sym, o)) { + Ptr_release(self); + Ptr_release(sym); + if (v != NULL) { + Ptr_release(v); + } + return (Var *)RT_unboxPtr(o); + } - Ptr_release(flatOStr); - Ptr_release(oStr); + if (v == NULL) { + Ptr_retain(self); + Ptr_retain(sym); + v = Var_create_interned(self, sym); + } - release(existingVar); - release(newVal); - Ptr_release(sym); - Ptr_release(self); - if (v != NULL) { - release(RT_boxPtr((Object *)v)); - } - release(existingVar); - Ptr_release(self); - Ptr_release(sym); - throwIllegalStateException_C(buf); + Ptr_retain(self); + Ptr_retain(sym); + retain(o); + Ptr_retain(v); + if (Namespace_checkReplacement(self, sym, o, RT_boxPtr(v))) { + bool replaced = false; + while (!replaced) { + Ptr_retain(map); // 1 + Ptr_retain(sym); + Ptr_retain(v); + PersistentArrayMap *newMap = + PersistentArrayMap_assoc(map, RT_boxPtr(sym), RT_boxPtr(v)); // 0 + printf("Trying to replace %lld with %lld\n", RT_boxPtr(map), + RT_boxPtr(newMap)); + + replaced = atomic_compare_exchange_strong_explicit( + &self->mappings, &mapVal, RT_boxPtr(newMap), memory_order_acq_rel, + memory_order_acquire); + if (replaced) { + autorelease(mapVal); // -1 + } else { + Ptr_release(newMap); // 0 } - release(existingVar); - release(newVal); - Ptr_release(sym); - Ptr_release(self); - release(existingVar); + mapVal = atomic_load_explicit(&self->mappings, memory_order_acquire); + map = (PersistentArrayMap *)RT_unboxPtr(mapVal); // 0 } - Ptr_retain(map); - retain(symVal); - retain(newVal); - PersistentArrayMap *newMap = PersistentArrayMap_assoc(map, symVal, newVal); - RTValue newMapVal = RT_boxPtr((Object *)newMap); + Ptr_release(self); + Ptr_release(sym); + release(o); + return v; + } + Ptr_release(self); + Ptr_release(sym); - if (atomic_compare_exchange_strong_explicit(&self->mappings, &mapVal, - newMapVal, memory_order_acq_rel, - memory_order_acquire)) { - autorelease(mapVal); - Ptr_release(self); - Ptr_release(sym); - return v; + if (v != NULL) { + Ptr_release(v); + } + return (Var *)RT_unboxPtr(o); +} + +/* + This method checks if a namespace's mapping is applicable and warns on +problematic cases. It will return a boolean indicating if a mapping is +replaceable. The semantics of what constitutes a legal replacement mapping is +summarized as follows: + +| classification | in namespace ns | newval = anything other than ns/name +| newval = ns/name | +|----------------+------------------------+--------------------------------------+-------------------------------------| +| native mapping | name -> ns/name | no replace, warn-if newval not-core +| no replace, warn-if newval not-core | | alias mapping | name -> +other/whatever | warn + replace | warn + replace | +*/ + +bool Namespace_checkReplacement(Namespace *self, Symbol *sym, RTValue old, + RTValue neu) { + + if (getType(old) == varType) { + // Namespace *ons = ((Var *)RT_unboxPtr(old))->ns; + Namespace *nns = + (getType(neu) == varType) ? ((Var *)RT_unboxPtr(neu))->ns : NULL; + + String *s = String_create("clojure.core"); + bool isCore = (nns != NULL) && String_equals(nns->name->name, s); + Ptr_release(s); + + if (neu) { + release(neu); } - release(newMapVal); + if (Namespace_isInternedMapping(self, sym, old)) { + if (!isCore) { + // TODO: print to screen + // RT.errPrintWriter().println( + // "REJECTED: attempt to replace interned var " + old + " with " + + // neu + " in " + name + ", you must ns-unmap first"); + return false; + } else + return false; + } + } else { + release(old); + Ptr_release(sym); + Ptr_release(self); + release(neu); } + + // TODO: print to screen + // RT.errPrintWriter().println( + // "WARNING: " + sym + " already refers to: " + old + + // " in namespace: " + name + ", being replaced by: " + neu); + return true; } /* @@ -371,10 +416,6 @@ RTValue Namespace_reference(Namespace *self, Symbol *sym, RTValue val) { throwIllegalStateException_C(buf); } release(o); - release(val); - Ptr_release(sym); - Ptr_release(self); - release(o); } Ptr_retain(map); @@ -397,8 +438,8 @@ RTValue Namespace_reference(Namespace *self, Symbol *sym, RTValue val) { } /* - * A convenience wrapper for `Namespace_reference` specifically designed to map - * a symbol to a Class. Consumes `self`, `sym`, and `val`. + * A convenience wrapper for `Namespace_reference` specifically designed to + * map a symbol to a Class. Consumes `self`, `sym`, and `val`. */ RTValue Namespace_referenceClass(Namespace *self, Symbol *sym, Class *val) { // val is a Class pointer, it consumes val. @@ -455,8 +496,8 @@ void Namespace_unmap(Namespace *self, Symbol *sym) { /* * Imports a Class into the namespace under the given symbol, making it - * accessible. Wraps `Namespace_referenceClass` and unboxes the result. Consumes - * `self`, `sym`, and `c` arguments. + * accessible. Wraps `Namespace_referenceClass` and unboxes the result. + * Consumes `self`, `sym`, and `c` arguments. */ Class *Namespace_importClassSym(Namespace *self, Symbol *sym, Class *c) { return (Class *)RT_unboxPtr(Namespace_referenceClass(self, sym, c)); @@ -474,8 +515,8 @@ Var *Namespace_refer(Namespace *self, Symbol *sym, Var *var) { /* * Looks up and retrieves the mapping for the given symbol name in this - * namespace. Returns the mapped value with a retained (+1) refcount, or nil if - * not found. Consumes `self` and `name` arguments. + * namespace. Returns the mapped value with a retained (+1) refcount, or nil + * if not found. Consumes `self` and `name` arguments. */ RTValue Namespace_getMapping(Namespace *self, Symbol *name) { RTValue mapVal = atomic_load_explicit(&self->mappings, memory_order_acquire); @@ -491,7 +532,8 @@ RTValue Namespace_getMapping(Namespace *self, Symbol *name) { /* * Searches for a Var that is explicitly interned within this namespace under * the given symbol. Returns the retained Var (+1 refcount) if found and owned - * by this namespace, or NULL otherwise. Consumes `self` and `symbol` arguments. + * by this namespace, or NULL otherwise. Consumes `self` and `symbol` + * arguments. */ RTValue Namespace_findInternedVar(Namespace *self, Symbol *symbol) { Ptr_retain(self); @@ -515,7 +557,8 @@ RTValue Namespace_findInternedVar(Namespace *self, Symbol *symbol) { /* * Resolves an alias symbol to its corresponding Namespace object. * Returns the mapped Namespace pointer as RTValue with a retained (+1) - * refcount, or RT_boxNil() if not found. Consumes `self` and `alias` arguments. + * refcount, or RT_boxNil() if not found. Consumes `self` and `alias` + * arguments. */ RTValue Namespace_lookupAlias(Namespace *self, Symbol *alias) { RTValue mapVal = atomic_load_explicit(&self->aliases, memory_order_acquire); @@ -527,10 +570,10 @@ RTValue Namespace_lookupAlias(Namespace *self, Symbol *alias) { } /* - * Registers a namespace alias, linking the given symbol to the target Namespace - * `ns`. Throws an exception if the alias is already mapped to a different - * namespace. Uses lock-free atomic CAS to update the aliases map. Consumes - * `self`, `alias`, and `ns` arguments. + * Registers a namespace alias, linking the given symbol to the target + * Namespace `ns`. Throws an exception if the alias is already mapped to a + * different namespace. Uses lock-free atomic CAS to update the aliases map. + * Consumes `self`, `alias`, and `ns` arguments. */ void Namespace_addAlias(Namespace *self, Symbol *alias, Namespace *ns) { if (alias == NULL || ns == NULL) { @@ -594,9 +637,9 @@ void Namespace_addAlias(Namespace *self, Symbol *alias, Namespace *ns) { } /* - * Removes a previously registered namespace alias from the aliases dictionary. - * Uses lock-free atomic CAS to replace the aliases map with the alias - * dissociated. Consumes `self` and `alias` arguments. + * Removes a previously registered namespace alias from the aliases + * dictionary. Uses lock-free atomic CAS to replace the aliases map with the + * alias dissociated. Consumes `self` and `alias` arguments. */ void Namespace_removeAlias(Namespace *self, Symbol *alias) { if (alias->ns != NULL) { @@ -637,3 +680,19 @@ void Namespace_removeAlias(Namespace *self, Symbol *alias) { } } } + +RTValue RT_inNs(__attribute__((swift_context)) struct ExecutionContext *ctx, + Symbol *nsName) __attribute__((swiftcall)) { + Namespace *ns = Namespace_findOrCreate(nsName); + RTValue coreNsVal = + Namespace_find(Symbol_create(String_create("clojure.core"))); + assert(!RT_isNil(coreNsVal) && "clojure.core namespace not found"); + Namespace *coreNs = (Namespace *)RT_unboxPtr(coreNsVal); + + RTValue nsVarVal = + Namespace_getMapping(coreNs, Symbol_create(String_create("*ns*"))); + assert(!RT_isNil(nsVarVal) && "clojure.core/*ns* Var not found"); + Var *nsVar = (Var *)RT_unboxPtr(nsVarVal); + + return Var_set(ctx, nsVar, RT_boxPtr(ns)); +} diff --git a/backend/runtime/Namespace.h b/backend/runtime/Namespace.h index c7deecc..48040bd 100644 --- a/backend/runtime/Namespace.h +++ b/backend/runtime/Namespace.h @@ -31,8 +31,8 @@ void Namespace_promoteToShared(Namespace *self, uword_t count); RTValue Namespace_getMappings(Namespace *self); RTValue Namespace_getAliases(Namespace *self); +bool Namespace_checkReplacement(Namespace *self, Symbol *sym, RTValue old, RTValue neu); bool Namespace_isInternedMapping(Namespace *self, Symbol *sym, RTValue o); -bool Namespace_checkReplacement(Namespace *self, Symbol *sym, RTValue oldVal, RTValue neuVal); Var *Namespace_intern(Namespace *self, Symbol *sym); RTValue Namespace_reference(Namespace *self, Symbol *sym, RTValue val); RTValue Namespace_referenceClass(Namespace *self, Symbol *sym, Class *val); @@ -42,8 +42,10 @@ Var *Namespace_refer(Namespace *self, Symbol *sym, Var *var); RTValue Namespace_getMapping(Namespace *self, Symbol *name); RTValue Namespace_findInternedVar(Namespace *self, Symbol *symbol); RTValue Namespace_lookupAlias(Namespace *self, Symbol *alias); +struct ExecutionContext; void Namespace_addAlias(Namespace *self, Symbol *alias, Namespace *ns); void Namespace_removeAlias(Namespace *self, Symbol *alias); +RTValue RT_inNs(__attribute__((swift_context)) struct ExecutionContext *ctx, Symbol *nsName) __attribute__((swiftcall)); #ifdef __cplusplus } diff --git a/backend/runtime/PersistentArrayMap.c b/backend/runtime/PersistentArrayMap.c index 6aaff55..13225d4 100644 --- a/backend/runtime/PersistentArrayMap.c +++ b/backend/runtime/PersistentArrayMap.c @@ -108,6 +108,7 @@ bool PersistentArrayMap_equals(PersistentArrayMap *self, release(otherVal); return false; } + release(otherVal); } return true; } diff --git a/backend/runtime/Var.c b/backend/runtime/Var.c index 7759100..906dc86 100644 --- a/backend/runtime/Var.c +++ b/backend/runtime/Var.c @@ -115,6 +115,7 @@ RTValue Var_getMeta(Var *self) { return val; } +/* Outside fercount system */ Var *Var_setDynamic(Var *self, bool dynamic) { // modifies and returns self self->dynamic = dynamic; return self; @@ -219,7 +220,7 @@ RTValue Var_set(__attribute__((swift_context)) struct ExecutionContext *ctx, // Check if the var has a thread-local binding (Clojure behavior) PersistentArrayMap *m = RT_unboxPtr(ctx->bindingsMap); - Ptr_retain(m); // indexOf consumes map + Ptr_retain(m); // indexOf consumes map Ptr_retain(self); // indexOf consumes key if (PersistentArrayMap_indexOf(m, RT_boxPtr(self)) == -1) { release(value); @@ -230,7 +231,7 @@ RTValue Var_set(__attribute__((swift_context)) struct ExecutionContext *ctx, retain(value); RTValue oldMap = ctx->bindingsMap; Ptr_retain(self); // assoc consumes key - Ptr_retain(m); // assoc consumes map + Ptr_retain(m); // assoc consumes map ctx->bindingsMap = RT_boxPtr(PersistentArrayMap_assoc(m, RT_boxPtr(self), value)); release(oldMap); diff --git a/backend/runtime/tests/Namespace_test.c b/backend/runtime/tests/Namespace_test.c index 431c86d..28e63b4 100644 --- a/backend/runtime/tests/Namespace_test.c +++ b/backend/runtime/tests/Namespace_test.c @@ -9,6 +9,7 @@ #include "../PersistentArrayMap.h" #include "../RuntimeInterface.h" #include "../ConcurrentHashMap.h" +#include "../ExecutionContext.h" extern ConcurrentHashMap *namespaces; @@ -234,12 +235,14 @@ static void test_namespace_redefine_referred_var_should_throw(void **state) { Var *ref = Namespace_refer(ns, sym, otherVar); Ptr_release(ref); - // Now try to intern "foo" in ns. This should throw IllegalStateException! + // Now try to intern "foo" in ns. Replacement is allowed because the old value was a referred Var (non-interned), + // so it should succeed and return the newly created interned Var. Ptr_retain(ns); Ptr_retain(sym); - ASSERT_THROWS("IllegalStateException", { - Var *v = Namespace_intern(ns, sym); - Ptr_release(v); - }); + Var *v = Namespace_intern(ns, sym); + assert_non_null(v); + assert_ptr_equal(v->ns, ns); + assert_ptr_equal(v->sym, sym); + Ptr_release(v); Ptr_release(otherVar); Ptr_release(otherNs); @@ -264,12 +267,14 @@ static void test_namespace_redefine_class_should_throw(void **state) { RTValue ref = Namespace_reference(ns, sym, RT_boxPtr((Object *)strVal)); release(ref); - // Now try to intern "foo" in ns. This should throw IllegalStateException! + // Now try to intern "foo" in ns. Replacement is allowed because the old value was a String (non-Var), + // so it should succeed and return the newly created interned Var. Ptr_retain(ns); Ptr_retain(sym); - ASSERT_THROWS("IllegalStateException", { - Var *v = Namespace_intern(ns, sym); - Ptr_release(v); - }); + Var *v = Namespace_intern(ns, sym); + assert_non_null(v); + assert_ptr_equal(v->ns, ns); + assert_ptr_equal(v->sym, sym); + Ptr_release(v); Ptr_release(ns); Ptr_release(sym); @@ -303,12 +308,12 @@ static void test_namespace_redefine_referred_var_with_other_reference_should_thr Var *ref1 = Namespace_refer(ns, sym, otherVar); Ptr_release(ref1); - // Now try to refer thirdVar in ns under "foo". This should throw IllegalStateException! + // Now try to refer thirdVar in ns under "foo". Replacement is allowed (with a warning), + // so it should succeed and return thirdVar. Ptr_retain(ns); Ptr_retain(sym); Ptr_retain(thirdVar); - ASSERT_THROWS("IllegalStateException", { - Var *ref2 = Namespace_refer(ns, sym, thirdVar); - Ptr_release(ref2); - }); + Var *ref2 = Namespace_refer(ns, sym, thirdVar); + assert_ptr_equal(ref2, thirdVar); + Ptr_release(ref2); Ptr_release(otherVar); Ptr_release(thirdVar); @@ -320,6 +325,71 @@ static void test_namespace_redefine_referred_var_with_other_reference_should_thr }); } +static void test_rt_in_ns(void **state) { + (void)state; + MemoryState before, after; + captureMemoryState(&before); + { + // Setup execution context with empty bindings map + ExecutionContext *ctx = ExecutionContext_create(RT_boxPtr(PersistentArrayMap_empty())); + + // Setup clojure.core namespace and *ns* Var (like initializeDefaultNamespaces does) + Symbol *coreSym = Symbol_create(String_create("clojure.core")); + Namespace *coreNs = Namespace_findOrCreate(coreSym); + + Symbol *nsSym = Symbol_create(String_create("*ns*")); + Var *nsVar = Namespace_intern(coreNs, nsSym); + Var_setDynamic(nsVar, true); + + // Set initial bindings map: map *ns* to user namespace + Symbol *userSym = Symbol_create(String_create("user")); + Namespace *userNs = Namespace_findOrCreate(userSym); + Ptr_retain(nsVar); + Var_bindRoot(nsVar, RT_boxPtr((Object *)userNs)); + + RTValue oldBindings = ctx->bindingsMap; + Ptr_retain(RT_unboxPtr(oldBindings)); + Ptr_retain(nsVar); + Ptr_retain(userNs); + ctx->bindingsMap = RT_boxPtr(PersistentArrayMap_assoc( + (PersistentArrayMap *)RT_unboxPtr(oldBindings), + RT_boxPtr(nsVar), + RT_boxPtr((Object *)userNs))); + release(oldBindings); + + // Now call RT_inNs to switch to "my-new-ns" + Symbol *newNsSym = Symbol_create(String_create("my-new-ns")); + Ptr_retain(newNsSym); + RTValue res = RT_inNs(ctx, newNsSym); + assert_false(RT_isNil(res)); + Namespace *newNs = (Namespace *)RT_unboxPtr(res); + assert_string_equal(String_c_str(newNs->name->name), "my-new-ns"); + + // Check that the bindings map has indeed been updated to "my-new-ns" + Ptr_retain(RT_unboxPtr(ctx->bindingsMap)); + Ptr_retain(nsVar); + RTValue currentNsVal = PersistentArrayMap_get( + (PersistentArrayMap *)RT_unboxPtr(ctx->bindingsMap), + RT_boxPtr(nsVar)); + assert_ptr_equal(RT_unboxPtr(currentNsVal), newNs); + release(currentNsVal); + + // Clean up + release(res); + Ptr_release(ctx); + Ptr_release(nsVar); + + // We created "clojure.core", "user" and "my-new-ns". Let's dissoc them from the global map to prevent leaks: + ConcurrentHashMap_dissoc_preservesSelf(namespaces, RT_boxSymbol((Object *)Symbol_create(String_create("clojure.core")))); + ConcurrentHashMap_dissoc_preservesSelf(namespaces, RT_boxSymbol((Object *)Symbol_create(String_create("user")))); + ConcurrentHashMap_dissoc_preservesSelf(namespaces, RT_boxSymbol((Object *)Symbol_create(String_create("my-new-ns")))); + Ptr_release(newNsSym); + Ebr_force_reclaim(); + } + captureMemoryState(&after); + assertMemoryBalanceExcept(&before, &after, (int[]){stringType, persistentVectorType, persistentVectorNodeType, symbolType}, 4); +} + int main(void) { RuntimeInterface_initialise(); const struct CMUnitTest tests[] = { @@ -332,6 +402,7 @@ int main(void) { cmocka_unit_test(test_namespace_redefine_referred_var_should_throw), cmocka_unit_test(test_namespace_redefine_class_should_throw), cmocka_unit_test(test_namespace_redefine_referred_var_with_other_reference_should_throw), + cmocka_unit_test(test_rt_in_ns), }; int res = cmocka_run_group_tests(tests, NULL, NULL); RuntimeInterface_cleanup(); diff --git a/backend/state/ThreadsafeCompilerState.cpp b/backend/state/ThreadsafeCompilerState.cpp index 033d7ab..5693440 100644 --- a/backend/state/ThreadsafeCompilerState.cpp +++ b/backend/state/ThreadsafeCompilerState.cpp @@ -17,23 +17,20 @@ ThreadsafeCompilerState::ThreadsafeCompilerState() void ThreadsafeCompilerState::initializeDefaultNamespaces() { // Setup default Clojure namespaces and *ns* Var - // 1. Create "clojure.core" namespace - Symbol *coreSym = Symbol_create(String_create("clojure.core")); - Namespace *coreNs = Namespace_findOrCreate(coreSym); // coreNs has +1 refcount + Namespace *coreNs = Namespace_findOrCreate( + Symbol_create(String_create("clojure.core"))); // coreNs has +1 refcount // 2. Intern "*ns*" Var in "clojure.core" - Symbol *nsSym = Symbol_create(String_create("*ns*")); - Var *nsVar = Namespace_intern(coreNs, nsSym); + Var *nsVar = Namespace_intern(coreNs, Symbol_create(String_create("*ns*"))); Var_setDynamic(nsVar, true); // 3. Create "user" namespace - Symbol *userSym = Symbol_create(String_create("user")); - Namespace *userNs = Namespace_findOrCreate(userSym); // userNs has +1 refcount - RTValue userNsVal = RT_boxPtr(userNs); + Namespace *userNs = Namespace_findOrCreate( + Symbol_create(String_create("user"))); // userNs has +1 refcount // 4. Bind clojure.core/*ns* Var root to the user namespace object Ptr_retain(nsVar); // Retain nsVar because Var_bindRoot consumes self - Var_bindRoot(nsVar, userNsVal); + Var_bindRoot(nsVar, RT_boxPtr(userNs)); // 6. Register in compiler state so JIT finds this exact Var object registerVar("clojure.core/*ns*", nsVar); @@ -485,14 +482,15 @@ Var *ThreadsafeCompilerState::getOrCreateVar(const char *name) { // 1. Create Symbol for Namespace Symbol *nsSym = Symbol_create(String_createDynamicStr(nsName.c_str())); - + // 2. Find or create the Namespace (consumes nsSym, returns +1 ref) Namespace *ns = Namespace_findOrCreate(nsSym); // 3. Create Symbol for Var Symbol *varSym = Symbol_create(String_createDynamicStr(symName.c_str())); - // 4. Intern symbol inside Namespace (consumes ns and varSym, returns retained Var +1) + // 4. Intern symbol inside Namespace (consumes ns and varSym, returns retained + // Var +1) Var *var = Namespace_intern(ns, varSym); return var; } @@ -513,19 +511,21 @@ Var *ThreadsafeCompilerState::getCurrentVar(const char *name) { if (RT_isNil(nsVal)) { release(nsVal); - // Jeśli brak namespace, a nazwa była niekwalifikowana, szukamy w clojure.core + // Jeśli brak namespace, a nazwa była niekwalifikowana, szukamy w + // clojure.core if (slashPos == std::string::npos && nsName != "clojure.core") { Symbol *coreSym = Symbol_create(String_create("clojure.core")); RTValue coreNsVal = Namespace_find(coreSym); - + if (!RT_isNil(coreNsVal)) { Namespace *coreNs = (Namespace *)RT_unboxPtr(coreNsVal); - Symbol *coreVarSym = Symbol_create(String_createDynamicStr(symName.c_str())); + Symbol *coreVarSym = + Symbol_create(String_createDynamicStr(symName.c_str())); Ptr_retain(coreNs); RTValue coreMapping = Namespace_getMapping(coreNs, coreVarSym); Ptr_release(coreNs); release(coreNsVal); - + if (!RT_isNil(coreMapping) && getType(coreMapping) == varType) { Var *var = (Var *)RT_unboxPtr(coreMapping); return var; @@ -553,15 +553,16 @@ Var *ThreadsafeCompilerState::getCurrentVar(const char *name) { if (slashPos == std::string::npos && nsName != "clojure.core") { Symbol *coreSym = Symbol_create(String_create("clojure.core")); RTValue coreNsVal = Namespace_find(coreSym); - + if (!RT_isNil(coreNsVal)) { Namespace *coreNs = (Namespace *)RT_unboxPtr(coreNsVal); - Symbol *coreVarSym = Symbol_create(String_createDynamicStr(symName.c_str())); + Symbol *coreVarSym = + Symbol_create(String_createDynamicStr(symName.c_str())); Ptr_retain(coreNs); RTValue coreMapping = Namespace_getMapping(coreNs, coreVarSym); Ptr_release(coreNs); release(coreNsVal); - + if (!RT_isNil(coreMapping) && getType(coreMapping) == varType) { Var *var = (Var *)RT_unboxPtr(coreMapping); return var; @@ -596,14 +597,11 @@ void ThreadsafeCompilerState::registerVar(const char *name, Var *var) { symName = varName.substr(slashPos + 1); } - // 1. Znajdź lub utwórz Namespace Symbol *nsSym = Symbol_create(String_createDynamicStr(nsName.c_str())); Namespace *ns = Namespace_findOrCreate(nsSym); - // 2. Utwórz Symbol dla Var Symbol *varSym = Symbol_create(String_createDynamicStr(symName.c_str())); - // 3. Uzupełnij pola ns i sym w strukturze Var, jeśli są puste if (var->ns == nullptr) { var->ns = ns; } @@ -612,7 +610,6 @@ void ThreadsafeCompilerState::registerVar(const char *name, Var *var) { Ptr_retain(varSym); } - // 4. Przypisz Var w Namespace za pomocą referencji (consumes var) Var *referred = Namespace_refer(ns, varSym, var); Ptr_release(referred); } diff --git a/frontend/resources/rt-classes.edn b/frontend/resources/rt-classes.edn index c504d82..55bf134 100644 --- a/frontend/resources/rt-classes.edn +++ b/frontend/resources/rt-classes.edn @@ -80,7 +80,7 @@ clojure.lang.IObj {withMeta [{:args [:this :any] :type :call :symbol "Var_resetMeta" :returns clojure.lang.Var}]} clojure.lang.IDeref {deref [{:args [:context :this] :type :call :symbol "Var_deref" :returns :any :pass-binding-context true}]}} :instance-fns - {} + {set [{:args [:context :this :any] :type :call :symbol "Var_set" :returns :any :pass-binding-context true}]} :static-fns {set [{:args [:context clojure.lang.Var :any] :type :call :symbol "Var_set" :returns :any :pass-binding-context true}] pushThreadBindings [{:args [:context :any] :type :call :symbol "RT_bind_map" :returns :context :pass-binding-context true}] @@ -440,4 +440,4 @@ clojure.lang.PersistentList clojure.lang.RT {:static-fns - {}}} + {inNs [{:args [:context clojure.lang.Symbol] :type :call :symbol "RT_inNs" :returns :any :pass-binding-context true}]}}} From a09f9c5287a02f9c346c00f6ff7f1e569b6f7280 Mon Sep 17 00:00:00 2001 From: Marek Lipert Date: Sat, 30 May 2026 12:32:15 +0200 Subject: [PATCH 2/2] Complete the in-ns implementation. --- backend/state/ThreadsafeCompilerState.cpp | 37 ++++++++++++++++ .../state/ThreadsafeCompilerState_test.cpp | 33 +++++++++++++++ backend/tools/EdnParser.cpp | 10 ++++- frontend/resources/rt-classes-extension.clj | 8 +++- frontend/src/clojure/rt/core.clj | 42 +++++++++++++++---- 5 files changed, 120 insertions(+), 10 deletions(-) diff --git a/backend/state/ThreadsafeCompilerState.cpp b/backend/state/ThreadsafeCompilerState.cpp index 5693440..7907ef9 100644 --- a/backend/state/ThreadsafeCompilerState.cpp +++ b/backend/state/ThreadsafeCompilerState.cpp @@ -3,6 +3,7 @@ #include "../runtime/Namespace.h" #include "../runtime/Symbol.h" #include "../runtime/Var.h" +#include "../runtime/Function.h" #include "../tools/EdnParser.h" #include "../tools/RTValueWrapper.h" namespace rt { @@ -438,6 +439,7 @@ void ThreadsafeCompilerState::validateProtocolImplementations( } void ThreadsafeCompilerState::extendInternalClasses(RTValue from) { + retain(from); auto descriptions = buildClasses(from); for (auto &desc : descriptions) { @@ -467,6 +469,41 @@ void ThreadsafeCompilerState::extendInternalClasses(RTValue from) { } } } + + // Phase 5: Intern Clojure Var functions defined in class extensions + PersistentArrayMap *map = (PersistentArrayMap *)RT_unboxPtr(from); + for (uword_t i = 0; i < map->count; i++) { + RTValue key = map->keys[i]; + RTValue value = map->values[i]; + + if (getType(key) == symbolType && getType(value) == functionType) { + Symbol *sym = (Symbol *)RT_unboxPtr(key); + if (sym->ns != nullptr) { + std::string nsName = String_c_str(sym->ns); + std::string symName = String_c_str(sym->name); + + // Find or create the namespace + Ptr_retain(sym->ns); + Symbol *nsSym = Symbol_create(sym->ns); + Namespace *ns = Namespace_findOrCreate(nsSym); + + // Intern the symbol in that namespace to get the Var + Ptr_retain(sym->name); + Symbol *varSym = Symbol_create(sym->name); + Var *var = Namespace_intern(ns, varSym); + + // Bind root of the Var to the JIT-compiled ClojureFunction object + Ptr_retain(var); + retain(value); + Var_bindRoot(var, value); + + // Register in compiler state so it can be resolved by JIT + std::string fullVarName = nsName + "/" + symName; + registerVar(fullVarName.c_str(), var); + } + } + } + release(from); } Var *ThreadsafeCompilerState::getOrCreateVar(const char *name) { diff --git a/backend/tests/state/ThreadsafeCompilerState_test.cpp b/backend/tests/state/ThreadsafeCompilerState_test.cpp index a90b42a..29ce571 100644 --- a/backend/tests/state/ThreadsafeCompilerState_test.cpp +++ b/backend/tests/state/ThreadsafeCompilerState_test.cpp @@ -5,6 +5,7 @@ #include #include #include +#include extern "C" { #include @@ -120,12 +121,44 @@ static void test_extend_internal_classes_function_merge(void **state) { }); } +static void test_extend_internal_classes_var_interning(void **state) { + (void)state; + + ASSERT_MEMORY_ALL_BALANCED({ + rt::JITEngine engine; + rt::ThreadsafeCompilerState &compState = engine.threadsafeState; + + // 1. Create a dummy ClojureFunction + ClojureFunction *fnObj = Function_create(1, 1, false); + Function_fillMethod(fnObj, 0, 0, 1, false, (void *)dlsym(RTLD_DEFAULT, "RT_inNs_bridge"), nullptr, 0); + + // 2. Create the extension map containing a namespace-qualified Var definition: my.ns/my-var -> fnObj + PersistentArrayMap *extMap = PersistentArrayMap_empty(); + Symbol *qualifiedSym = Symbol_create(String_create("my-var")); + qualifiedSym->ns = String_create("my.ns"); + + extMap = PersistentArrayMap_assoc(extMap, RT_boxPtr(qualifiedSym), RT_boxPtr(fnObj)); + + // 3. Extend + compState.extendInternalClasses(RT_boxPtr(extMap)); + + // 4. Verify that Var my.ns/my-var is interned and bound to fnObj + Var *var = compState.getCurrentVar("my.ns/my-var"); + assert_non_null(var); + + RTValue boundVal = Var_deref(nullptr, var); // consumes var + assert_ptr_equal(RT_unboxPtr(boundVal), fnObj); + release(boundVal); + }); +} + int main(void) { initialise_memory(); const struct CMUnitTest tests[] = { cmocka_unit_test(test_extend_internal_classes_merge), cmocka_unit_test(test_extend_internal_classes_function_merge), + cmocka_unit_test(test_extend_internal_classes_var_interning), }; return cmocka_run_group_tests(tests, NULL, NULL); diff --git a/backend/tools/EdnParser.cpp b/backend/tools/EdnParser.cpp index 0c594a7..38d8a83 100644 --- a/backend/tools/EdnParser.cpp +++ b/backend/tools/EdnParser.cpp @@ -177,9 +177,15 @@ void TemporaryClassData::scanMetadata(RTValue from) { if (getType(key) != symbolType) throwInternalInconsistencyException( "Class definition key is not a symbol."); - if (getType(value) != persistentArrayMapType) + + if (getType(value) == functionType) { + continue; // Skip Var definitions (which map to functions, not class maps) + } + + if (getType(value) != persistentArrayMapType) { throwInternalInconsistencyException( - "Class definition value is not a map."); + "Class definition value must be a map."); + } // PersistentArrayMap_assoc consumes its arguments. // Protect borrowed value and key for assoc. diff --git a/frontend/resources/rt-classes-extension.clj b/frontend/resources/rt-classes-extension.clj index ea2f300..11931d8 100644 --- a/frontend/resources/rt-classes-extension.clj +++ b/frontend/resources/rt-classes-extension.clj @@ -15,5 +15,11 @@ (let [lst (fn [& rest] rest)] (if (= coll nil) (lst) - (.cons coll x))))}]}}} + (.cons coll x))))}]}} + 'clojure.core/sequence + (fn [x] x) + + 'clojure.core/in-ns + (fn [x] + (clojure.lang.Var/set #'clojure.core/*ns* (clojure.lang.Namespace/findOrCreate x)))} diff --git a/frontend/src/clojure/rt/core.clj b/frontend/src/clojure/rt/core.clj index d786bca..2983a69 100644 --- a/frontend/src/clojure/rt/core.clj +++ b/frontend/src/clojure/rt/core.clj @@ -111,6 +111,20 @@ :validate/wrong-tag-handler (fn [_ _] nil) :validate/unresolvable-symbol-handler (fn [_ _ _] nil)}) + (defn- extract-ns-sym + "Extracts the target namespace symbol from namespace-modifying forms + like (ns target.ns ...) or (in-ns 'target.ns). Returns nil if the + form is not a namespace-switching operation." + [form] + (when (seq? form) + (let [[op arg] form] + (cond + (= op 'ns) arg + (= op 'in-ns) (if (and (seq? arg) (= (first arg) 'quote)) + (second arg) + arg) + :else nil)))) + (defn analyze ([s filename] (analyze s filename false false)) ([s filename trivial-tree? simple-tree?] @@ -131,19 +145,33 @@ (when simple-tree? (clojure.pprint/pprint ret-val)) ret-val) (let [;; 1. Analyze the form with the accumulated environment - ast (a/analyze form current-env {:passes-opts passes-opts})] + ast (a/analyze form current-env {:passes-opts passes-opts}) + + ;; 2. Detect if the analyzed form is a namespace switch + ns-sym (extract-ns-sym form) + + ;; 3. Update the analysis environment for subsequent forms. + ;; If a namespace switch was detected, we explicitly bind :ns + ;; to the target namespace symbol to keep the analyzer context synchronized. + next-env (cond-> (:env ast) + ns-sym (assoc :ns ns-sym))] - ;; 2. Selective Eval for Macros - ;; If the AST represents a macro definition, we MUST eval it. - ;; Without this, the next form cannot be macro-expanded. + ;; 4. Selective Eval for Macros and Namespaces + ;; We must evaluate macro definitions in the JVM compiler process so that + ;; they are available for macroexpansion of subsequent forms in the same file. + ;; Similarly, namespace-switching forms (ns / in-ns) must be evaluated so the + ;; Clojure analyzer's internal JVM state matches our updated environment. (when (or (and (= (:op ast) :def) (:macro (:var ast))) (and (= (:op ast) :do) ; Handle (do (defmacro ...)) - (some #(and (= (:op %) :def) (:macro (:var %))) (:statements ast)))) + (some #(and (= (:op %) :def) (:macro (:var %))) (:statements ast))) + (and (seq? form) + (or (= (first form) 'in-ns) + (= (first form) 'ns)))) (eval form)) (recur (r/read {:eof :eof} reader) - ;; 3. Thread the updated environment to the next iteration - (:env ast) + ;; 5. Thread the updated environment to the next iteration + next-env (conj ret-val ast)))))))))) (defn generate-protobuf-defs [] (sch/generate-protobuf-defs "bytecode.proto"))