diff --git a/compiler/air/lower.go b/compiler/air/lower.go index 86313853..164c1d44 100644 --- a/compiler/air/lower.go +++ b/compiler/air/lower.go @@ -30,7 +30,11 @@ func LowerModulesWithTests(modules []checker.Module) (*Program, error) { } func LowerModulesWithOptions(modules []checker.Module, options LowerOptions) (*Program, error) { - l := newLowerer(options) + // A single root has one complete, immutable import graph, so owner lookups + // remain valid as its transitive modules are registered in moduleByName. + // Multiple roots may contain independent graphs with competing method tables; + // preserve their existing uncached lookup behavior rather than sharing entries. + l := newLowerer(options, len(modules)) for _, module := range modules { if err := l.lowerModule(module); err != nil { return nil, err @@ -56,6 +60,12 @@ type lowerer struct { functions map[string]FunctionID globals map[string]GlobalID + cacheMethodLookups bool + structMethodsByOwner map[checker.MethodOwner]map[string]*checker.FunctionDef + traitMethodsByOwner map[checker.TraitMethodOwner]map[string]*checker.FunctionDef + requiredGoMethodsByOwner map[checker.MethodOwner]map[string]*checker.FunctionDef + inherentMethodsByOwner map[checker.MethodOwner]map[string]*checker.FunctionDef + loweringModules map[string]bool loweredModules map[string]bool loweringFuncs map[FunctionID]bool @@ -87,7 +97,7 @@ type functionLowerer struct { directLetValue checker.Expression } -func newLowerer(options LowerOptions) *lowerer { +func newLowerer(options LowerOptions, rootCount int) *lowerer { l := &lowerer{ program: Program{ Entry: NoFunction, @@ -101,6 +111,8 @@ func newLowerer(options LowerOptions) *lowerer { functions: map[string]FunctionID{}, globals: map[string]GlobalID{}, + cacheMethodLookups: rootCount == 1, + loweringModules: map[string]bool{}, loweredModules: map[string]bool{}, loweringFuncs: map[FunctionID]bool{}, @@ -114,6 +126,12 @@ func newLowerer(options LowerOptions) *lowerer { genericMethodDefs: map[string]FunctionID{}, includeTests: options.IncludeTests, } + if l.cacheMethodLookups { + l.structMethodsByOwner = map[checker.MethodOwner]map[string]*checker.FunctionDef{} + l.traitMethodsByOwner = map[checker.TraitMethodOwner]map[string]*checker.FunctionDef{} + l.requiredGoMethodsByOwner = map[checker.MethodOwner]map[string]*checker.FunctionDef{} + l.inherentMethodsByOwner = map[checker.MethodOwner]map[string]*checker.FunctionDef{} + } l.mustIntern(checker.Void) l.mustIntern(checker.Int) l.mustIntern(checker.Float64) @@ -137,36 +155,76 @@ func (l *lowerer) structMethods(def *checker.StructDef) map[string]*checker.Func if def == nil { return nil } - return checker.StructMethodsInModules(l.moduleByName, checker.StructMethodOwner(def)) + owner := checker.StructMethodOwner(def) + if !l.cacheMethodLookups { + return checker.StructMethodsInModules(l.moduleByName, owner) + } + if methods, ok := l.structMethodsByOwner[owner]; ok { + return methods + } + methods := checker.StructMethodsInModules(l.moduleByName, owner) + l.structMethodsByOwner[owner] = methods + return methods } func (l *lowerer) traitMethods(typ checker.Type, trait *checker.Trait) map[string]*checker.FunctionDef { owner, ok := checker.MethodOwnerForType(typ) - if !ok { + if !ok || trait == nil { return nil } - return checker.TraitMethodsInModules(l.moduleByName, owner, trait) + if !l.cacheMethodLookups { + return checker.TraitMethodsInModules(l.moduleByName, owner, trait) + } + key := checker.TraitMethodOwner{ + MethodOwner: owner, + TraitModulePath: trait.ModulePath, + TraitName: trait.Name, + } + if methods, ok := l.traitMethodsByOwner[key]; ok { + return methods + } + methods := checker.TraitMethodsInModules(l.moduleByName, owner, trait) + l.traitMethodsByOwner[key] = methods + return methods } func (l *lowerer) requiredGoMethods(def *checker.StructDef) map[string]*checker.FunctionDef { if def == nil { return nil } - return checker.RequiredGoMethodsInModules(l.moduleByName, checker.StructMethodOwner(def)) + owner := checker.StructMethodOwner(def) + if !l.cacheMethodLookups { + return checker.RequiredGoMethodsInModules(l.moduleByName, owner) + } + if methods, ok := l.requiredGoMethodsByOwner[owner]; ok { + return methods + } + methods := checker.RequiredGoMethodsInModules(l.moduleByName, owner) + l.requiredGoMethodsByOwner[owner] = methods + return methods } func (l *lowerer) inherentMethods(def *checker.StructDef) map[string]*checker.FunctionDef { if def == nil { return nil } - return checker.InherentMethodsInModules(l.moduleByName, checker.StructMethodOwner(def)) + owner := checker.StructMethodOwner(def) + if !l.cacheMethodLookups { + return checker.InherentMethodsInModules(l.moduleByName, owner) + } + if methods, ok := l.inherentMethodsByOwner[owner]; ok { + return methods + } + methods := checker.InherentMethodsInModules(l.moduleByName, owner) + l.inherentMethodsByOwner[owner] = methods + return methods } func (l *lowerer) findReachableModule(path string) checker.Module { if mod, ok := l.moduleByName[path]; ok { return mod } - for _, mod := range l.moduleByName { + for _, mod := range sortedModules(l.moduleByName) { if found := findReachableModuleSeen(mod, path, map[string]bool{}); found != nil { l.moduleByName[path] = found return found @@ -191,7 +249,7 @@ func findReachableModuleSeen(mod checker.Module, path string, seen map[string]bo if program == nil { return nil } - for _, imported := range program.Imports { + for _, imported := range sortedModules(program.Imports) { if found := findReachableModuleSeen(imported, path, seen); found != nil { return found } @@ -227,7 +285,7 @@ func (l *lowerer) lowerModule(module checker.Module) error { return nil } - for _, imported := range prog.Imports { + for _, imported := range sortedModules(prog.Imports) { l.moduleByName[imported.Path()] = imported importID := l.internModule(imported.Path()) mod.Imports = append(mod.Imports, importID) @@ -599,7 +657,8 @@ func (l *lowerer) declareGenericFunctionDef(module ModuleID, callDef *checker.Fu }) l.program.Modules[module].Functions = appendUniqueFunction(l.program.Modules[module].Functions, id) typeVars := make(map[string]TypeID, len(params)) - for p, idx := range params { + for _, p := range paramNames { + idx := params[p] tp, err := l.internTypeParam(paramOwner, p, idx) if err != nil { return NoFunction, err @@ -811,11 +870,16 @@ func (l *lowerer) newFunctionLowerer(fn *Function, def *checker.FunctionDef, par for name, typeID := range l.functionTypeVars[fn.ID] { fl.typeVars[name] = typeID } - for name, typ := range def.GenericBindings { + bindingNames := make([]string, 0, len(def.GenericBindings)) + for name := range def.GenericBindings { + bindingNames = append(bindingNames, name) + } + sort.Strings(bindingNames) + for _, name := range bindingNames { if _, ok := fl.typeVars[name]; ok { continue } - typeID, err := fl.internResolvedType(typ) + typeID, err := fl.internResolvedType(def.GenericBindings[name]) if err == nil { fl.typeVars[name] = typeID } @@ -908,9 +972,9 @@ func (fl *functionLowerer) bindTypeVarsSeen(pattern checker.Type, actual TypeID, for _, field := range actualInfo.Fields { fieldsByName[field.Name] = field } - for name, fieldType := range typ.Fields { + for _, name := range sortedFieldNames(typ.Fields) { if field, ok := fieldsByName[name]; ok { - fl.bindTypeVarsSeen(fieldType, field.Type, seen) + fl.bindTypeVarsSeen(typ.Fields[name], field.Type, seen) } } } @@ -1576,7 +1640,7 @@ func (l *lowerer) declareGenericStructMethodsAndTraitImpls(module ModuleID, def // Required Go methods use their exact native name. Builtin Error shares // that method function with its Ard trait implementation. requiredMethods := map[string]*checker.FunctionDef{} - for _, method := range l.requiredGoMethods(def) { + for _, method := range sortedMethodDefinitions(l.requiredGoMethods(def)) { if method != nil { requiredMethods[method.Name] = method } @@ -1585,8 +1649,14 @@ func (l *lowerer) declareGenericStructMethodsAndTraitImpls(module ModuleID, def if !checker.IsBuiltinError(trait) { continue } - for name, method := range l.traitMethods(def, trait) { - if method != nil { + methods := l.traitMethods(def, trait) + methodNames := make([]string, 0, len(methods)) + for name := range methods { + methodNames = append(methodNames, name) + } + sort.Strings(methodNames) + for _, name := range methodNames { + if method := methods[name]; method != nil { requiredMethods[name] = method } } @@ -1694,11 +1764,11 @@ func (l *lowerer) declareInherentImplMethodsForStruct(module ModuleID, def *chec if trait == nil { continue } - for _, method := range l.traitMethods(def, trait) { + for _, method := range sortedMethodDefinitions(l.traitMethods(def, trait)) { traitMethodDefs[method] = true } } - for _, method := range l.requiredGoMethods(def) { + for _, method := range sortedMethodDefinitions(l.requiredGoMethods(def)) { if method == nil || traitMethodDefs[method] || functionHasUnresolvedTypeVar(method) { continue } @@ -1710,7 +1780,7 @@ func (l *lowerer) declareInherentImplMethodsForStruct(module ModuleID, def *chec return err } } - for _, method := range l.inherentMethods(def) { + for _, method := range sortedMethodDefinitions(l.inherentMethods(def)) { if method == nil || functionHasUnresolvedTypeVar(method) { continue } @@ -2223,7 +2293,8 @@ func (fl *functionLowerer) declareGenericMethodFunction(module ModuleID, instanc }) fl.l.program.Modules[module].Functions = appendUniqueFunction(fl.l.program.Modules[module].Functions, id) typeVars := make(map[string]TypeID, len(paramNames)) - for p, idx := range params { + for _, p := range paramNames { + idx := params[p] tp, err := fl.l.internTypeParam(paramOwner, p, idx) if err != nil { return NoFunction, nil, err @@ -2409,7 +2480,7 @@ func collectReachableModules(mod checker.Module, seen map[string]checker.Module) if mod.Program() == nil { return } - for _, imported := range mod.Program().Imports { + for _, imported := range sortedModules(mod.Program().Imports) { collectReachableModules(imported, seen) } } @@ -2428,11 +2499,11 @@ func (l *lowerer) lookupStructDef(modulePath, name string, generic bool) *checke // recover only a unique declaration of the same generic shape. Ambiguity must // not merge unrelated nominal types. modules := map[string]checker.Module{} - for _, mod := range l.moduleByName { + for _, mod := range sortedModules(l.moduleByName) { collectReachableModules(mod, modules) } var found *checker.StructDef - for _, mod := range modules { + for _, mod := range sortedModules(modules) { sd := structDefInModule(mod, name, generic) if sd == nil { continue @@ -2824,10 +2895,10 @@ func (l *lowerer) typeOwnerPath(t checker.Type) string { default: return "" } - for path, module := range l.moduleByName { + for _, module := range sortedModules(l.moduleByName) { sym := module.Get(name) if sym.Type == t { - return path + return module.Path() } } return "" @@ -4675,8 +4746,13 @@ func (fl *functionLowerer) lowerExpr(expr checker.Expression) (*Expr, error) { return &Expr{Kind: ExprDiscardingFunctionCoercion, Type: typeID, Target: value}, nil case *checker.ForeignStructInstance: fields := make([]StructFieldValue, 0, len(e.Fields)) - for name, valueExpr := range e.Fields { - value, err := fl.lowerExpr(valueExpr) + fieldNames := make([]string, 0, len(e.Fields)) + for name := range e.Fields { + fieldNames = append(fieldNames, name) + } + sort.Strings(fieldNames) + for _, name := range fieldNames { + value, err := fl.lowerExpr(e.Fields[name]) if err != nil { return nil, err } @@ -6574,9 +6650,14 @@ func (fl *functionLowerer) localKind(local LocalID) TypeKind { } func (l *lowerer) lookupFunction(name string) (FunctionID, bool) { - for key, id := range l.functions { + keys := make([]string, 0, len(l.functions)) + for key := range l.functions { + keys = append(keys, key) + } + sort.Strings(keys) + for _, key := range keys { if keyHasFunctionName(key, name) { - return id, true + return l.functions[key], true } } return NoFunction, false @@ -6604,7 +6685,7 @@ func (l *lowerer) ensureModuleImportTraitImplsDeclared(moduleID ModuleID) error if !ok || mod.Program() == nil { return nil } - for _, imported := range mod.Program().Imports { + for _, imported := range sortedModules(mod.Program().Imports) { importedID := l.internModule(imported.Path()) l.program.Modules[moduleID].Imports = appendUniqueModule(l.program.Modules[moduleID].Imports, importedID) if err := l.ensureModuleTraitImplsDeclaredRecursive(imported.Path(), map[string]bool{}); err != nil { @@ -6626,7 +6707,7 @@ func (l *lowerer) ensureModuleTraitImplsDeclaredRecursive(modulePath string, see if !ok || mod.Program() == nil { return nil } - for _, imported := range mod.Program().Imports { + for _, imported := range sortedModules(mod.Program().Imports) { if err := l.ensureModuleTraitImplsDeclaredRecursive(imported.Path(), seen); err != nil { return err } @@ -6678,7 +6759,7 @@ func (l *lowerer) ensureModuleTypesDeclared(modulePath string) error { } modID := l.internModule(modulePath) prog := mod.Program() - for _, imported := range prog.Imports { + for _, imported := range sortedModules(prog.Imports) { l.moduleByName[imported.Path()] = imported importedID := l.internModule(imported.Path()) l.program.Modules[modID].Imports = appendUniqueModule(l.program.Modules[modID].Imports, importedID) @@ -6727,7 +6808,7 @@ func (l *lowerer) ensureModuleGlobalsDeclared(modulePath string) error { } modID := l.internModule(modulePath) prog := mod.Program() - for _, imported := range prog.Imports { + for _, imported := range sortedModules(prog.Imports) { l.moduleByName[imported.Path()] = imported importedID := l.internModule(imported.Path()) l.program.Modules[modID].Imports = appendUniqueModule(l.program.Modules[modID].Imports, importedID) @@ -6911,7 +6992,7 @@ func (l *lowerer) moduleForInstanceMethod(method *checker.InstanceMethod, fallba l.findReachableModule(ownerModulePath) return l.internModule(ownerModulePath) } - for modulePath, mod := range l.moduleByName { + for _, mod := range sortedModules(l.moduleByName) { if mod.Program() == nil { continue } @@ -6919,11 +7000,11 @@ func (l *lowerer) moduleForInstanceMethod(method *checker.InstanceMethod, fallba switch def := stmt.Stmt.(type) { case *checker.StructDef: if def.Name == ownerName && l.hasStructMethod(def, method.Method.Name) { - return l.internModule(modulePath) + return l.internModule(mod.Path()) } case *checker.Enum: if def.Name == ownerName && def.Methods[method.Method.Name] != nil { - return l.internModule(modulePath) + return l.internModule(mod.Path()) } } } @@ -7015,6 +7096,51 @@ func sortedFieldNames(fields map[string]checker.Type) []string { return names } +// sortedModules makes checker map traversal safe for AIR ID allocation. Module, +// type, and function IDs become part of generated names, so discovery order must +// not depend on Go's randomized map iteration. +func sortedModules(modules map[string]checker.Module) []checker.Module { + type moduleEntry struct { + key string + path string + module checker.Module + } + entries := make([]moduleEntry, 0, len(modules)) + for key, module := range modules { + path := "" + if module != nil { + path = module.Path() + } + entries = append(entries, moduleEntry{key: key, path: path, module: module}) + } + sort.Slice(entries, func(i, j int) bool { + if entries[i].path == entries[j].path { + return entries[i].key < entries[j].key + } + return entries[i].path < entries[j].path + }) + ordered := make([]checker.Module, len(entries)) + for i, entry := range entries { + ordered[i] = entry.module + } + return ordered +} + +// sortedMethodDefinitions preserves the method table's canonical key order +// before declarations allocate AIR function IDs. +func sortedMethodDefinitions(methods map[string]*checker.FunctionDef) []*checker.FunctionDef { + names := make([]string, 0, len(methods)) + for name := range methods { + names = append(names, name) + } + sort.Strings(names) + ordered := make([]*checker.FunctionDef, len(names)) + for i, name := range names { + ordered[i] = methods[name] + } + return ordered +} + func appendUniqueType(items []TypeID, id TypeID) []TypeID { for _, item := range items { if item == id { diff --git a/compiler/air/lower_test.go b/compiler/air/lower_test.go index 9dbba4c5..3cc8fc11 100644 --- a/compiler/air/lower_test.go +++ b/compiler/air/lower_test.go @@ -12,6 +12,54 @@ import ( "github.com/akonwi/ard/parse" ) +type orderingTestModule struct { + path string +} + +func (m orderingTestModule) Path() string { return m.path } +func (m orderingTestModule) Get(string) checker.Symbol { return checker.Symbol{} } +func (m orderingTestModule) Program() *checker.Program { return nil } +func (m orderingTestModule) Symbols() map[string]checker.Symbol { return nil } + +func TestSortedModulesUsesCanonicalPathOrder(t *testing.T) { + modules := map[string]checker.Module{ + "middle": orderingTestModule{path: "demo/middle"}, + "last": orderingTestModule{path: "demo/z_last"}, + "first": orderingTestModule{path: "demo/a_first"}, + } + + ordered := sortedModules(modules) + got := make([]string, len(ordered)) + for i, module := range ordered { + got[i] = module.Path() + } + want := []string{"demo/a_first", "demo/middle", "demo/z_last"} + if strings.Join(got, ",") != strings.Join(want, ",") { + t.Fatalf("sorted modules = %v, want %v", got, want) + } +} + +func TestSortedMethodDefinitionsUsesCanonicalKeyOrder(t *testing.T) { + first := &checker.FunctionDef{Name: "first"} + middle := &checker.FunctionDef{Name: "middle"} + last := &checker.FunctionDef{Name: "last"} + methods := map[string]*checker.FunctionDef{ + "z_last": last, + "middle": middle, + "a_first": first, + } + + ordered := sortedMethodDefinitions(methods) + got := make([]string, len(ordered)) + for i, method := range ordered { + got[i] = method.Name + } + want := []string{"first", "middle", "last"} + if strings.Join(got, ",") != strings.Join(want, ",") { + t.Fatalf("sorted methods = %v, want %v", got, want) + } +} + func TestLowerFunctionParameterWithIndirectMutuallyRecursiveStructs(t *testing.T) { const helperEnv = "ARD_TEST_AIR_RECURSIVE_TYPE_BINDING" const input = ` @@ -1617,8 +1665,147 @@ func TestLowerGenericStructMethodBodyUsesReceiverBindings(t *testing.T) { t.Fatalf("get return kind = %v, want TypeParam", typeKind(t, program, get.Signature.Return)) } } + +func TestLowererCachesCrossModuleMethodLookupsByOwner(t *testing.T) { + owner := checker.MethodOwner{ModulePath: "dependency", TypeName: "Widget"} + trait := &checker.Trait{Name: "Printable", ModulePath: "dependency"} + traitOwner := checker.TraitMethodOwner{ + MethodOwner: owner, + TraitModulePath: trait.ModulePath, + TraitName: trait.Name, + } + structMethod := &checker.FunctionDef{Name: "render"} + traitMethod := &checker.FunctionDef{Name: "print"} + required := &checker.FunctionDef{Name: "string", RequiredGoMethodName: "String"} + inherent := &checker.FunctionDef{Name: "label"} + dependency := &countingMethodModule{ + path: "dependency", + program: &checker.Program{ + StructMethods: map[checker.MethodOwner]map[string]*checker.FunctionDef{ + owner: {"render": structMethod}, + }, + TraitMethods: map[checker.TraitMethodOwner]map[string]*checker.FunctionDef{ + traitOwner: {"print": traitMethod}, + }, + RequiredGoMethods: map[checker.MethodOwner]map[string]*checker.FunctionDef{ + owner: {"String": required}, + }, + InherentMethods: map[checker.MethodOwner]map[string]*checker.FunctionDef{ + owner: {"label": inherent}, + }, + }, + } + root := &countingMethodModule{ + path: "root", + program: &checker.Program{Imports: map[string]checker.Module{"dependency": dependency}}, + } + lowerer := newLowerer(LowerOptions{}, 1) + lowerer.moduleByName[root.path] = root + def := &checker.StructDef{Name: owner.TypeName, ModulePath: owner.ModulePath} + + for i := 0; i < 2; i++ { + if got := lowerer.requiredGoMethods(def); got["String"] != required { + t.Fatalf("required lookup %d = %#v, want String method", i, got) + } + } + if root.programCalls != 1 || dependency.programCalls != 1 { + t.Fatalf("required lookup traversed modules more than once: root=%d dependency=%d", root.programCalls, dependency.programCalls) + } + + for i := 0; i < 2; i++ { + if got := lowerer.inherentMethods(def); got["label"] != inherent { + t.Fatalf("inherent lookup %d = %#v, want label method", i, got) + } + } + if root.programCalls != 2 || dependency.programCalls != 2 { + t.Fatalf("inherent lookup traversed modules more than once: root=%d dependency=%d", root.programCalls, dependency.programCalls) + } + + for i := 0; i < 2; i++ { + if got := lowerer.structMethods(def); got["render"] != structMethod { + t.Fatalf("struct lookup %d = %#v, want render method", i, got) + } + } + if root.programCalls != 3 || dependency.programCalls != 3 { + t.Fatalf("struct lookup traversed modules more than once: root=%d dependency=%d", root.programCalls, dependency.programCalls) + } + + for i := 0; i < 2; i++ { + if got := lowerer.traitMethods(def, trait); got["print"] != traitMethod { + t.Fatalf("trait lookup %d = %#v, want print method", i, got) + } + } + if root.programCalls != 4 || dependency.programCalls != 4 { + t.Fatalf("trait lookup traversed modules more than once: root=%d dependency=%d", root.programCalls, dependency.programCalls) + } +} + +func TestLowererCachesMissingCrossModuleMethods(t *testing.T) { + dependency := &countingMethodModule{path: "dependency", program: &checker.Program{}} + root := &countingMethodModule{ + path: "root", + program: &checker.Program{Imports: map[string]checker.Module{"dependency": dependency}}, + } + lowerer := newLowerer(LowerOptions{}, 1) + lowerer.moduleByName[root.path] = root + def := &checker.StructDef{Name: "Missing", ModulePath: "dependency"} + + for i := 0; i < 2; i++ { + if got := lowerer.requiredGoMethods(def); got != nil { + t.Fatalf("required lookup %d = %#v, want nil", i, got) + } + } + if root.programCalls != 1 || dependency.programCalls != 1 { + t.Fatalf("missing lookup traversed modules more than once: root=%d dependency=%d", root.programCalls, dependency.programCalls) + } +} + +func TestLowererDisablesMethodLookupCachingForMultipleRoots(t *testing.T) { + owner := checker.MethodOwner{ModulePath: "dependency", TypeName: "Widget"} + required := &checker.FunctionDef{Name: "string", RequiredGoMethodName: "String"} + dependency := &countingMethodModule{ + path: "dependency", + program: &checker.Program{RequiredGoMethods: map[checker.MethodOwner]map[string]*checker.FunctionDef{ + owner: {"String": required}, + }}, + } + root := &countingMethodModule{ + path: "root", + program: &checker.Program{Imports: map[string]checker.Module{"dependency": dependency}}, + } + lowerer := newLowerer(LowerOptions{}, 2) + lowerer.moduleByName[root.path] = root + def := &checker.StructDef{Name: owner.TypeName, ModulePath: owner.ModulePath} + + for i := 0; i < 2; i++ { + if got := lowerer.requiredGoMethods(def); got["String"] != required { + t.Fatalf("required lookup %d = %#v, want String method", i, got) + } + } + if root.programCalls != 2 || dependency.programCalls != 2 { + t.Fatalf("uncached lookup did not retraverse modules: root=%d dependency=%d", root.programCalls, dependency.programCalls) + } +} + +type countingMethodModule struct { + path string + program *checker.Program + programCalls int +} + +func (m *countingMethodModule) Path() string { return m.path } + +func (m *countingMethodModule) Get(string) checker.Symbol { return checker.Symbol{} } + +func (m *countingMethodModule) Program() *checker.Program { + m.programCalls++ + return m.program +} + +func (m *countingMethodModule) Symbols() map[string]checker.Symbol { return nil } + func TestReferenceSyntheticIdentityUsesReferentTypeID(t *testing.T) { - lowerer := newLowerer(LowerOptions{}) + lowerer := newLowerer(LowerOptions{}, 0) left := TypeID(len(lowerer.program.Types) + 1) lowerer.program.Types = append(lowerer.program.Types, TypeInfo{ID: left, Kind: TypeStruct, Name: "Item", ModulePath: "left"}) right := TypeID(len(lowerer.program.Types) + 1) diff --git a/compiler/checker/go_import_internal_test.go b/compiler/checker/go_import_internal_test.go index 634781ed..a9a54b1f 100644 --- a/compiler/checker/go_import_internal_test.go +++ b/compiler/checker/go_import_internal_test.go @@ -195,6 +195,31 @@ func TestReadonlyModuleCompletionErrorClassification(t *testing.T) { } } +func TestDependencyReplaceOverlayUsesCanonicalModuleOrder(t *testing.T) { + root := t.TempDir() + if err := os.WriteFile(filepath.Join(root, "go.mod"), []byte("module example.com/app\n\ngo 1.27\n"), 0o644); err != nil { + t.Fatal(err) + } + resolver := NewGoPackagesResolver(root, nil) + resolver.DependencyModuleRoots = map[string]string{ + "example.com/z_last": t.TempDir(), + "example.com/a_first": t.TempDir(), + } + + overlay := resolver.dependencyReplaceOverlay() + got := string(overlay[filepath.Join(root, "go.mod")]) + firstRequire := strings.Index(got, "example.com/a_first v0.0.0") + lastRequire := strings.Index(got, "example.com/z_last v0.0.0") + firstReplace := strings.Index(got, "replace example.com/a_first") + lastReplace := strings.Index(got, "replace example.com/z_last") + if firstRequire < 0 || lastRequire < 0 || firstRequire > lastRequire { + t.Fatalf("dependency requirements are not canonical:\n%s", got) + } + if firstReplace < 0 || lastReplace < 0 || firstReplace > lastReplace { + t.Fatalf("dependency replacements are not canonical:\n%s", got) + } +} + func TestDependencyModfilesTrustOnlyProjectChecksums(t *testing.T) { cacheRoot := t.TempDir() t.Setenv("HOME", cacheRoot) diff --git a/compiler/checker/go_packages_resolver.go b/compiler/checker/go_packages_resolver.go index ffc9d582..65b852b0 100644 --- a/compiler/checker/go_packages_resolver.go +++ b/compiler/checker/go_packages_resolver.go @@ -686,8 +686,14 @@ func (r *GoPackagesResolver) dependencyReplaceOverlay() map[string][]byte { if err != nil { return nil } + modulePaths := make([]string, 0, len(r.DependencyModuleRoots)) + for modulePath := range r.DependencyModuleRoots { + modulePaths = append(modulePaths, modulePath) + } + sort.Strings(modulePaths) changed := false - for modulePath, dir := range r.DependencyModuleRoots { + for _, modulePath := range modulePaths { + dir := r.DependencyModuleRoots[modulePath] if modulePath == "" || dir == "" { continue } @@ -759,24 +765,39 @@ func DependencyGoModuleRoots(info *ProjectInfo) map[string]string { } roots[modulePath] = root } + dependencyAliases := make([]string, 0, len(info.Dependencies)) + for alias := range info.Dependencies { + dependencyAliases = append(dependencyAliases, alias) + } + sort.Strings(dependencyAliases) + packageIDs := make([]string, 0, len(info.Packages)) + for packageID := range info.Packages { + packageIDs = append(packageIDs, packageID) + } + sort.Strings(packageIDs) + // Add path dependencies first so a locked Git checkout retains precedence // if malformed dependency metadata names the same Go module from both. - for _, dep := range info.Dependencies { + for _, alias := range dependencyAliases { + dep := info.Dependencies[alias] if dep.Git == "" { add(dep.RootPath) } } - for packageID, pkg := range info.Packages { + for _, packageID := range packageIDs { + pkg := info.Packages[packageID] if packageID != info.RootPackageID && pkg.Git == "" && pkg.Path != "" { add(pkg.RootPath) } } - for _, dep := range info.Dependencies { + for _, alias := range dependencyAliases { + dep := info.Dependencies[alias] if dep.Git != "" { add(dep.RootPath) } } - for packageID, pkg := range info.Packages { + for _, packageID := range packageIDs { + pkg := info.Packages[packageID] if packageID != info.RootPackageID && pkg.Git != "" { add(pkg.RootPath) } diff --git a/compiler/go/backend.go b/compiler/go/backend.go index e9f5b11a..597a04ef 100644 --- a/compiler/go/backend.go +++ b/compiler/go/backend.go @@ -440,12 +440,14 @@ func generatedGoMod(dir string, program *air.Program, projectInfo *checker.Proje requireSeen := requireKeys(goMod) requires := make([]string, 0) addDependencyGoModRequirements(&requires, requireSeen, program, projectInfo) + sort.Strings(requires) goMod += formatRequireBlock(requires) replaceSeen := replaceKeys(goMod) replaces := make([]string, 0) addDependencyGoModRootReplaces(&replaces, replaceSeen, program, projectInfo) addDependencyGoModReplaces(&replaces, replaceSeen, program, projectInfo) + sort.Strings(replaces) goMod += formatReplaceBlock(replaces) return goMod, nil } @@ -455,12 +457,12 @@ func generatedGoMod(dir string, program *air.Program, projectInfo *checker.Proje // or the declared source root for path dependencies (#437). This mirrors the // checker's resolution, keeping type-checking and the build in agreement. func addDependencyGoModRootReplaces(out *[]string, seen map[string]bool, program *air.Program, projectInfo *checker.ProjectInfo) { - for modulePath, root := range dependencyGoModPackages(program, projectInfo) { - abs, err := filepath.Abs(root) + for _, dependency := range sortedDependencyGoModPackages(program, projectInfo) { + abs, err := filepath.Abs(dependency.root) if err != nil { continue } - addGoModReplace(out, seen, fmt.Sprintf("%s => %s", modulePath, abs)) + addGoModReplace(out, seen, fmt.Sprintf("%s => %s", dependency.modulePath, abs)) } } @@ -542,14 +544,14 @@ func isRelativeLocalReplacePath(path string) bool { } func addDependencyGoModRequirements(out *[]string, seen map[string]bool, program *air.Program, projectInfo *checker.ProjectInfo) { - packages := dependencyGoModPackages(program, projectInfo) - for _, root := range packages { - addGoModRequirementsFromFile(out, seen, filepath.Join(root, "go.mod")) + dependencies := sortedDependencyGoModPackages(program, projectInfo) + for _, dependency := range dependencies { + addGoModRequirementsFromFile(out, seen, filepath.Join(dependency.root, "go.mod")) } - for modulePath := range packages { - if !seen[modulePath] { - seen[modulePath] = true - *out = append(*out, modulePath+" v0.0.0") + for _, dependency := range dependencies { + if !seen[dependency.modulePath] { + seen[dependency.modulePath] = true + *out = append(*out, dependency.modulePath+" v0.0.0") } } } @@ -665,8 +667,8 @@ func projectGoModuleName(projectInfo *checker.ProjectInfo) string { } func addDependencyGoModReplaces(out *[]string, seen map[string]bool, program *air.Program, projectInfo *checker.ProjectInfo) { - for _, root := range dependencyGoModPackages(program, projectInfo) { - addGoModReplacesFromFile(out, seen, filepath.Join(root, "go.mod"), root) + for _, dependency := range sortedDependencyGoModPackages(program, projectInfo) { + addGoModReplacesFromFile(out, seen, filepath.Join(dependency.root, "go.mod"), dependency.root) } } @@ -802,12 +804,13 @@ func mergeGoSum(dir string, program *air.Program, projectInfo *checker.ProjectIn if projectInfo != nil && strings.TrimSpace(projectInfo.RootPath) != "" { addGoSumLines(&lines, seen, filepath.Join(projectInfo.RootPath, "go.sum")) } - for _, root := range dependencyGoModPackages(program, projectInfo) { - addGoSumLines(&lines, seen, filepath.Join(root, "go.sum")) + for _, dependency := range sortedDependencyGoModPackages(program, projectInfo) { + addGoSumLines(&lines, seen, filepath.Join(dependency.root, "go.sum")) } if len(lines) == 0 { return nil } + sort.Strings(lines) return os.WriteFile(goSumPath, []byte(strings.Join(lines, "\n")+"\n"), 0o644) } @@ -1049,6 +1052,26 @@ func dependencyGoModPackages(program *air.Program, projectInfo *checker.ProjectI return checker.DependencyGoModuleRoots(projectInfo) } +type dependencyGoModPackage struct { + modulePath string + root string +} + +func sortedDependencyGoModPackages(program *air.Program, projectInfo *checker.ProjectInfo) []dependencyGoModPackage { + packages := dependencyGoModPackages(program, projectInfo) + ordered := make([]dependencyGoModPackage, 0, len(packages)) + for modulePath, root := range packages { + ordered = append(ordered, dependencyGoModPackage{modulePath: modulePath, root: root}) + } + sort.Slice(ordered, func(i, j int) bool { + if ordered[i].modulePath == ordered[j].modulePath { + return ordered[i].root < ordered[j].root + } + return ordered[i].modulePath < ordered[j].modulePath + }) + return ordered +} + func dependencyAliasForModulePath(modulePath string, projectInfo *checker.ProjectInfo) (string, bool) { key, _, ok := dependencyPackageForModulePath(modulePath, projectInfo) return key, ok @@ -1059,7 +1082,13 @@ func dependencyPackageForModulePath(modulePath string, projectInfo *checker.Proj return "", "", false } first := strings.Split(modulePath, "/")[0] - for _, dep := range projectInfo.Dependencies { + dependencyAliases := make([]string, 0, len(projectInfo.Dependencies)) + for alias := range projectInfo.Dependencies { + dependencyAliases = append(dependencyAliases, alias) + } + sort.Strings(dependencyAliases) + for _, alias := range dependencyAliases { + dep := projectInfo.Dependencies[alias] packageID := dep.PackageID if packageID == "" { packageID = dep.Alias @@ -1069,13 +1098,18 @@ func dependencyPackageForModulePath(modulePath string, projectInfo *checker.Proj return key, dependencyRootPath(dep), true } } - for packageID, pkg := range projectInfo.Packages { + packageIDs := make([]string, 0, len(projectInfo.Packages)) + for packageID := range projectInfo.Packages { + packageIDs = append(packageIDs, packageID) + } + sort.Strings(packageIDs) + for _, packageID := range packageIDs { if packageID == projectInfo.RootPackageID || packageID == "" { continue } key := checker.PackageModulePrefix(packageID) if first == key { - return key, pkg.RootPath, true + return key, projectInfo.Packages[packageID].RootPath, true } } return "", "", false diff --git a/compiler/go/backend_test.go b/compiler/go/backend_test.go index 570b1b24..b6ab9b23 100644 --- a/compiler/go/backend_test.go +++ b/compiler/go/backend_test.go @@ -5150,6 +5150,26 @@ func TestArtifactWorkspaceUsesProjectLocalArdOut(t *testing.T) { } } +func TestSortedDependencyGoModPackagesUsesModulePathOrder(t *testing.T) { + firstRoot := t.TempDir() + lastRoot := t.TempDir() + if err := os.WriteFile(filepath.Join(firstRoot, "go.mod"), []byte("module example.com/a_first\n\ngo 1.27\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(lastRoot, "go.mod"), []byte("module example.com/z_last\n\ngo 1.27\n"), 0o644); err != nil { + t.Fatal(err) + } + project := &checker.ProjectInfo{Dependencies: map[string]checker.DependencyInfo{ + "last": {Alias: "last", RootPath: lastRoot}, + "first": {Alias: "first", RootPath: firstRoot}, + }} + + ordered := sortedDependencyGoModPackages(nil, project) + if len(ordered) != 2 || ordered[0].modulePath != "example.com/a_first" || ordered[1].modulePath != "example.com/z_last" { + t.Fatalf("sorted dependency Go modules = %#v", ordered) + } +} + func mapsKeys[V any](m map[string]V) []string { keys := make([]string, 0, len(m)) for key := range m { diff --git a/compiler/go/lower.go b/compiler/go/lower.go index 57676f66..e0a0d114 100644 --- a/compiler/go/lower.go +++ b/compiler/go/lower.go @@ -1193,6 +1193,12 @@ func traitDispatchMethodName(trait air.TraitID, methodIndex int) string { } func (l *lowerer) lowerGlobal(global air.Global) (ast.Decl, error) { + // Generated temporaries are lexical to one initializer. Restarting here + // prevents unrelated declaration order from renumbering this global's output. + previousTempCounter := l.tempCounter + l.tempCounter = 0 + defer func() { l.tempCounter = previousTempCounter }() + globalType, err := l.goType(global.Type) if err != nil { return nil, err @@ -1233,6 +1239,12 @@ func (l *lowerer) lowerGlobal(global air.Global) (ast.Decl, error) { } func (l *lowerer) lowerFunction(fn air.Function) (ast.Decl, error) { + // Generated temporaries are function-local. Restarting here keeps one + // function's output independent of functions lowered before it. + previousTempCounter := l.tempCounter + l.tempCounter = 0 + defer func() { l.tempCounter = previousTempCounter }() + l.declaredLocals = map[air.LocalID]bool{} methodName, directMethod := l.directGoMethodName(fn) if fn.RequiredGoMethodName != "" && !directMethod { diff --git a/compiler/go/temp_scope_test.go b/compiler/go/temp_scope_test.go new file mode 100644 index 00000000..75795c39 --- /dev/null +++ b/compiler/go/temp_scope_test.go @@ -0,0 +1,55 @@ +package gotarget + +import ( + "go/ast" + "testing" +) + +func TestGeneratedTemporaryNamesRestartForEachFunction(t *testing.T) { + program := lowerSource(t, ` + fn first(value: Int?) Int { + let actual = try value -> _ { 0 } + actual + } + + fn second(value: Int?) Int { + let actual = try value -> _ { 0 } + actual + } + `) + + files := lowerProgramAST(t, program, Options{PackageName: "main"}) + for _, functionName := range []string{"First", "Second"} { + function := findGeneratedFunction(t, files, functionName) + if !astNodeContainsIdent(function, "_tmp_0") { + t.Fatalf("generated function %s does not restart temporary names at _tmp_0", functionName) + } + } +} + +func findGeneratedFunction(t *testing.T, files map[string]*ast.File, name string) *ast.FuncDecl { + t.Helper() + for _, file := range files { + for _, declaration := range file.Decls { + function, ok := declaration.(*ast.FuncDecl) + if ok && function.Name != nil && function.Name.Name == name { + return function + } + } + } + t.Fatalf("generated function %s not found", name) + return nil +} + +func astNodeContainsIdent(node ast.Node, name string) bool { + found := false + ast.Inspect(node, func(candidate ast.Node) bool { + identifier, ok := candidate.(*ast.Ident) + if ok && identifier.Name == name { + found = true + return false + } + return !found + }) + return found +}