Fix #746: nested namespace shadowing same-named top-level namespace throws in interpreter#803
Open
nickna wants to merge 1 commit into
Open
Fix #746: nested namespace shadowing same-named top-level namespace throws in interpreter#803nickna wants to merge 1 commit into
nickna wants to merge 1 commit into
Conversation
…hrows in interpreter ExecuteNamespace called _environment.GetNamespace(name), which walks up the full scope chain. When a nested `O.A` was declared and the global scope already held a top-level `namespace A`, GetNamespace found the global one and entered the merge path — skipping the DefineNamespace call that would have planted "A" in namespaceEnvO._values. The VariableResolver had correctly pre-computed distance=2 for the bare `A` reference inside O.B.f (pointing to O's scope), but GetAt(2, "A") returned Undefined because that slot was never written. The subsequent property access on Undefined threw "Only instances and objects have properties". Fix: add GetLocalNamespace (checks _namespaces on the current scope only, no chain walk) and use it in ExecuteNamespace. Same-level declaration merging is unaffected — GetLocalNamespace still finds a previously-defined namespace in the same scope. Compiled mode was already correct via ResolveNamespaceField.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
GetLocalNamespaceadded toRuntimeEnvironment— checks only the current scope's_namespacesdict (no chain traversal)ExecuteNamespacechanged to useGetLocalNamespaceinstead ofGetNamespacewhen deciding whether to merge a namespace declarationNestedNamespace_ShadowsSameNamedTopLevelNamespace(both modes)Root cause
ExecuteNamespacecalled_environment.GetNamespace(name), which walks up the full scope chain. When a nestedO.Awas declared and the global scope already held a top-levelnamespace A, the chain-walk found the globalAand entered the merge path — skipping theelsebranch that calls_environment.DefineNamespace(name, nsObj).As a result,
namespaceEnvO._valuesnever got an"A"entry. TheVariableResolverhad correctly pre-computed distance = 2 for the bareAreference insideO.B.f(pointing to O's scope), butGetAt(2, "A")returnedRuntimeValue.Undefinedbecause that slot was never written. The subsequent property access onUndefinedthrew:The compiled path was already correct —
ResolveNamespaceFieldkeys namespace fields by their full dotted path ("O.A"vs"A"), so the collision never occurs there.Fix
GetLocalNamespacechecks_namespaceson the current scope only. Using it inExecuteNamespaceensures a nestedO.Ais always treated as a new binding in O's scope rather than a merge into the outerA. Declaration-merging at the same level is unaffected —GetLocalNamespacestill finds a previously-defined namespace in the same scope.Test plan
NestedNamespace_ShadowsSameNamedTopLevelNamespacepasses in both interpreted and compiled mode (was failing with the runtime error above)NamespaceTestspass