From 4b75ec20d9c45193514bf750877a8edb5cfe1530 Mon Sep 17 00:00:00 2001 From: mikera Date: Sat, 28 Sep 2024 10:09:31 +0100 Subject: [PATCH] Better handling for parent environment lookups --- .../main/java/convex/core/lang/Context.java | 50 +++++-------------- .../test/java/convex/core/lang/ACVMTest.java | 2 +- .../java/convex/core/lang/CompilerTest.java | 4 +- .../java/convex/core/lang/ContextTest.java | 3 ++ .../test/java/convex/core/lang/CoreTest.java | 9 ++-- 5 files changed, 22 insertions(+), 46 deletions(-) diff --git a/convex-core/src/main/java/convex/core/lang/Context.java b/convex-core/src/main/java/convex/core/lang/Context.java index 55d42ba3f..7510331d5 100644 --- a/convex-core/src/main/java/convex/core/lang/Context.java +++ b/convex-core/src/main/java/convex/core/lang/Context.java @@ -565,18 +565,7 @@ public Context lookupDynamic(Address address, Symbol symbol) { * @return Metadata for given symbol (may be empty) or null if undeclared */ public AHashMap lookupMeta(Symbol sym) { - AHashMap env=getEnvironment(); - if ((env!=null)&&env.containsKey(sym)) { - return getMetadata().get(sym,Maps.empty()); - } - AccountStatus as = getAliasedAccount(env); - if (as==null) return null; - - env=as.getEnvironment(); - if (env.containsKey(sym)) { - return as.getMetadata().get(sym,Maps.empty()); - } - return null; + return lookupMeta(getAddress(),sym); } /** @@ -586,12 +575,16 @@ public AHashMap lookupMeta(Symbol sym) { * @return Metadata for given symbol (may be empty) or null if undeclared */ public AHashMap lookupMeta(Address address,Symbol sym) { - if (address==null) return lookupMeta(sym); - AccountStatus as=getAccountStatus(address); - if (as==null) return null; - AHashMap env=as.getEnvironment(); - if (env.containsKey(sym)) { - return as.getMetadata().get(sym,Maps.empty()); + AccountStatus as=(address==null)?getAccountStatus():getAccountStatus(address); + for (int i=0; i<16; i++) { + if (as==null) return null; + AHashMap env=as.getEnvironment(); + if (env.containsKey(sym)) { + return as.getMetadata().get(sym,Maps.empty()); + } + address=getParentAddress(as); + if (address==null) return null; + as=getAccountStatus(address); } return null; } @@ -685,23 +678,11 @@ private MapEntry lookupDynamicEntry(AccountStatus as,Symbol sym) { MapEntry result=as.getEnvironment().getEntry(sym); if (result!=null) return result; - Address parent=as.getParent(); - as=(parent==null)?null:getAccountStatus(parent); + Address parent=getParentAddress(as); + as=getAccountStatus(parent); // if not found, will be null } return null; } - - /** - * Looks up the account for an Symbol alias in the given environment. - * @param env - * @param path An alias path - * @return AccountStatus for the alias, or null if not present - */ - private AccountStatus getAliasedAccount(AHashMap env) { - // TODO: alternative core accounts - return getCoreAccount(); - } - /** * Gets the account status for the current Address @@ -717,11 +698,6 @@ public AccountStatus getAccountStatus() { return chainState.state.getAccount(a); } - - private AccountStatus getCoreAccount() { - return getState().getAccount(Core.CORE_ADDRESS); - } - /** * Gets the holdings map for the current account. * @return Map of holdings, or null if the current account does not exist. diff --git a/convex-core/src/test/java/convex/core/lang/ACVMTest.java b/convex-core/src/test/java/convex/core/lang/ACVMTest.java index 81952c4ad..9bac7c189 100644 --- a/convex-core/src/test/java/convex/core/lang/ACVMTest.java +++ b/convex-core/src/test/java/convex/core/lang/ACVMTest.java @@ -76,7 +76,7 @@ protected Context context() { } /** - * Builds the Context for this test class instance. Subclasses may override + * Builds the base Context for this test class instance. Subclasses may override * to generate a separate context * @param ctx Context to modify * @return diff --git a/convex-core/src/test/java/convex/core/lang/CompilerTest.java b/convex-core/src/test/java/convex/core/lang/CompilerTest.java index 07adc542b..e433ac635 100644 --- a/convex-core/src/test/java/convex/core/lang/CompilerTest.java +++ b/convex-core/src/test/java/convex/core/lang/CompilerTest.java @@ -854,12 +854,12 @@ public void testStaticCompilation() { // Constant addresses should also get static compilation assertEquals(Constant.of(Core.TRANSFER), eval("(compile '#8/transfer)")); + assertEquals(Constant.of(Core.COUNT), eval("(compile '#1/count)")); } else { assertEquals(Lookup.create(Address.create(8), Symbols.COUNT), eval("(compile 'count)")); } - // Aliases that don't hit static definitions compile to dynamic lookup - assertEquals(Lookup.create(Address.create(1), Symbols.COUNT), eval("(compile '#1/count)")); + // Aliases that don't hit real accounts compile to dynamic lookup assertEquals(Lookup.create(Address.create(8888), Symbols.TRANSFER), eval("(compile '#8888/transfer)")); } diff --git a/convex-core/src/test/java/convex/core/lang/ContextTest.java b/convex-core/src/test/java/convex/core/lang/ContextTest.java index 26192ba1a..c55ca91e9 100644 --- a/convex-core/src/test/java/convex/core/lang/ContextTest.java +++ b/convex-core/src/test/java/convex/core/lang/ContextTest.java @@ -30,9 +30,12 @@ /** * Tests for basic execution Context mechanics and internals + * + */ public class ContextTest extends ACVMTest { + // TODO: should probably make independent of base state init protected ContextTest() { super(BaseTest.STATE); } diff --git a/convex-core/src/test/java/convex/core/lang/CoreTest.java b/convex-core/src/test/java/convex/core/lang/CoreTest.java index 23e50b17c..61258b5f9 100644 --- a/convex-core/src/test/java/convex/core/lang/CoreTest.java +++ b/convex-core/src/test/java/convex/core/lang/CoreTest.java @@ -2670,9 +2670,7 @@ public void testLookupSyntax() { AHashMap countMeta=Core.METADATA.get(Symbols.COUNT); assertSame(countMeta, (eval("(lookup-meta 'count)"))); assertSame(countMeta, (eval("(lookup-meta "+Init.CORE_ADDRESS+ " 'count)"))); - - // Not actually defined in current address - assertNull(eval("(lookup-meta *address* 'count)")); + assertSame(countMeta, eval("(lookup-meta *address* 'count)")); assertNull(eval("(lookup-meta 'non-existent-symbol)")); assertNull(eval("(lookup-meta #666666 'count)")); // invalid address @@ -4265,11 +4263,10 @@ public void testDefinedQ() { assertTrue(evalB("(do (def foobar [2 3]) (defined? 'foobar))")); assertTrue(evalB("(do (def foobar [2 3]) (defined? *address* 'foobar))")); - // Note: defined by reference to core. TODO: is this OK? assertTrue(evalB("(defined? 'count)")); - // Not defined in explicit environment. TODO: is this OK? - assertFalse(evalB("(defined? *address* 'count)")); + // Not defined in explicit environment, but indirectly by core. TODO: is this OK? + assertTrue(evalB("(defined? *address* 'count)")); // invalid names assertCastError(step("(defined? :count)")); // not a Symbol