partial revert of store changes

gnolang · May 9, 2024 · dbde296 · dbde296
1 parent 39ae304
commit dbde296
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 38 deletions.
diff --git a/gno.land/pkg/sdk/vm/builtins.go b/gno.land/pkg/sdk/vm/builtins.go
@@ -16,7 +16,7 @@ func (vm *VMKeeper) initBuiltinPackagesAndTypes(store gno.Store) {
 	// NOTE: native functions/methods added here must be quick operations,
 	// or account for gas before operation.
 	// TODO: define criteria for inclusion, and solve gas calculations.
-	getPackage := func(pkgPath string, newStore gno.Store) (pn *gno.PackageNode, pv *gno.PackageValue) {
+	getPackage := func(pkgPath string) (pn *gno.PackageNode, pv *gno.PackageValue) {
 		// otherwise, built-in package value.
 		// first, load from filepath.
 		stdlibPath := filepath.Join(vm.stdlibsDir, pkgPath)
@@ -34,7 +34,7 @@ func (vm *VMKeeper) initBuiltinPackagesAndTypes(store gno.Store) {
 			PkgPath: "gno.land/r/stdlibs/" + pkgPath,
 			// PkgPath: pkgPath,
 			Output: os.Stdout,
-			Store:  newStore,
+			Store:  store,
 		})
 		defer m2.Release()
 		return m2.RunMemPackage(memPkg, true)

diff --git a/gnovm/pkg/gnolang/store.go b/gnovm/pkg/gnolang/store.go
@@ -14,9 +14,8 @@ import (
 
 // PackageGetter specifies how the store may retrieve packages which are not
 // already in its cache. PackageGetter should return nil when the requested
-// package does not exist. store should be used to run the machine, or otherwise
-// call any methods which may call store.GetPackage, to avoid import cycles.
-type PackageGetter func(pkgPath string, store Store) (*PackageNode, *PackageValue)
+// package does not exist.
+type PackageGetter func(pkgPath string) (*PackageNode, *PackageValue)
 
 // inject natives into a new or loaded package (value and node)
 type PackageInjector func(store Store, pn *PackageNode)
@@ -82,7 +81,8 @@ type defaultStore struct {
 	go2gnoStrict     bool                  // if true, native->gno type conversion must be registered.
 
 	// transient
-	opslog []StoreOp // for debugging and testing.
+	opslog  []StoreOp // for debugging and testing.
+	current []string  // for detecting import cycles.
 }
 
 func NewStore(alloc *Allocator, baseStore, iavlStore store.Store) *defaultStore {
@@ -109,29 +109,18 @@ func (ds *defaultStore) SetPackageGetter(pg PackageGetter) {
 	ds.pkgGetter = pg
 }
 
-type importerStore struct {
-	*defaultStore
-	importChain []string
-}
-
-func (is importerStore) GetPackage(pkgPath string, isImport bool) *PackageValue {
-	if !isImport {
-		// if not an import, match behaviour to the defaultStore
-		return is.defaultStore.GetPackage(pkgPath, isImport)
-	}
-	// it is an import -- detect cyclic imports
-	if slices.Contains(is.importChain, pkgPath) {
-		panic(fmt.Sprintf("import cycle detected: %q (through %v)", pkgPath, is.importChain))
-	}
-	return is.getPackage(pkgPath, is)
-}
-
 // Gets package from cache, or loads it from baseStore, or gets it from package getter.
 func (ds *defaultStore) GetPackage(pkgPath string, isImport bool) *PackageValue {
-	return ds.getPackage(pkgPath, importerStore{})
-}
-
-func (ds *defaultStore) getPackage(pkgPath string, impStore importerStore) *PackageValue {
+	// detect circular imports
+	if isImport {
+		if slices.Contains(ds.current, pkgPath) {
+			panic(fmt.Sprintf("import cycle detected: %q (through %v)", pkgPath, ds.current))
+		}
+		ds.current = append(ds.current, pkgPath)
+		defer func() {
+			ds.current = ds.current[:len(ds.current)-1]
+		}()
+	}
 	// first, check cache.
 	oid := ObjectIDFromPkgPath(pkgPath)
 	if oo, exists := ds.cacheObjects[oid]; exists {
@@ -175,12 +164,7 @@ func (ds *defaultStore) getPackage(pkgPath string, impStore importerStore) *Pack
 	}
 	// otherwise, fetch from pkgGetter.
 	if ds.pkgGetter != nil {
-		if impStore.defaultStore == nil {
-			// pre-allocate 16 entries to likely avoid further slice allocations.
-			impStore = importerStore{defaultStore: ds, importChain: make([]string, 0, 16)}
-		}
-		impStore.importChain = append(impStore.importChain, pkgPath)
-		if pn, pv := ds.pkgGetter(pkgPath, impStore); pv != nil {
+		if pn, pv := ds.pkgGetter(pkgPath); pv != nil {
 			// e.g. tests/imports_tests loads example/gno.land/r/... realms.
 			// if pv.IsRealm() {
 			// 	panic("realm packages cannot be gotten from pkgGetter")
@@ -626,6 +610,7 @@ func (ds *defaultStore) ClearObjectCache() {
 	ds.alloc.Reset()
 	ds.cacheObjects = make(map[ObjectID]Object) // new cache.
 	ds.opslog = nil                             // new ops log.
+	ds.current = nil
 	ds.SetCachePackage(Uverse())
 }
 
@@ -645,6 +630,7 @@ func (ds *defaultStore) Fork() Store {
 		nativeStore:      ds.nativeStore,
 		go2gnoStrict:     ds.go2gnoStrict,
 		opslog:           nil, // new ops log.
+		current:          nil,
 	}
 	ds2.SetCachePackage(Uverse())
 	return ds2

diff --git a/gnovm/tests/imports.go b/gnovm/tests/imports.go
@@ -62,7 +62,7 @@ const (
 
 // NOTE: this isn't safe, should only be used for testing.
 func TestStore(rootDir, filesPath string, stdin io.Reader, stdout, stderr io.Writer, mode importMode) (store gno.Store) {
-	getPackage := func(pkgPath string, newStore gno.Store) (pn *gno.PackageNode, pv *gno.PackageValue) {
+	getPackage := func(pkgPath string) (pn *gno.PackageNode, pv *gno.PackageValue) {
 		if pkgPath == "" {
 			panic(fmt.Sprintf("invalid zero package path in testStore().pkgGetter"))
 		}
@@ -81,7 +81,7 @@ func TestStore(rootDir, filesPath string, stdin io.Reader, stdout, stderr io.Wri
 				m2 := gno.NewMachineWithOptions(gno.MachineOptions{
 					PkgPath: "test",
 					Output:  stdout,
-					Store:   newStore,
+					Store:   store,
 				})
 				// pkg := gno.NewPackageNode(gno.Name(memPkg.Name), memPkg.Path, nil)
 				// pv := pkg.NewPackage()
@@ -93,7 +93,7 @@ func TestStore(rootDir, filesPath string, stdin io.Reader, stdout, stderr io.Wri
 		// if stdlibs package is preferred , try to load it first.
 		if mode == ImportModeStdlibsOnly ||
 			mode == ImportModeStdlibsPreferred {
-			pn, pv = loadStdlib(rootDir, pkgPath, newStore, stdout)
+			pn, pv = loadStdlib(rootDir, pkgPath, store, stdout)
 			if pn != nil {
 				return
 			}
@@ -377,7 +377,7 @@ func TestStore(rootDir, filesPath string, stdin io.Reader, stdout, stderr io.Wri
 
 		// if native package is preferred, try to load stdlibs/* as backup.
 		if mode == ImportModeNativePreferred {
-			pn, pv = loadStdlib(rootDir, pkgPath, newStore, stdout)
+			pn, pv = loadStdlib(rootDir, pkgPath, store, stdout)
 			if pn != nil {
 				return
 			}
@@ -394,7 +394,7 @@ func TestStore(rootDir, filesPath string, stdin io.Reader, stdout, stderr io.Wri
 			m2 := gno.NewMachineWithOptions(gno.MachineOptions{
 				PkgPath: "test",
 				Output:  stdout,
-				Store:   newStore,
+				Store:   store,
 			})
 			pn, pv = m2.RunMemPackage(memPkg, true)
 			return