From 5df74a8e3866129732ee83eed5bfadbe70ffebae Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Mon, 24 Aug 2020 16:18:21 -0700 Subject: [PATCH] Maintain ordering between variants and aliases AddFarVariationDependencies takes the first matching variant. To maintain sensible behavior on a module with aliases, the ordering of aliases and module variants needs to be maintained so that AddFarVariationDependencies can find an earlier matching alias instead of a more specific variant. Test: go test . Change-Id: I78f4e96edd98159f3a62d94e240e5d652667bec4 --- context.go | 265 +++++++++++++++++++++++------------------ context_test.go | 127 ++++++++++++-------- module_ctx.go | 54 +++++---- module_ctx_test.go | 40 ++----- name_interface.go | 4 +- splice_modules_test.go | 58 ++++----- visit_test.go | 4 +- 7 files changed, 299 insertions(+), 253 deletions(-) diff --git a/context.go b/context.go index 482a2b1..4ca4ef8 100644 --- a/context.go +++ b/context.go @@ -163,16 +163,65 @@ type moduleAlias struct { target *moduleInfo } +func (m *moduleAlias) alias() *moduleAlias { return m } +func (m *moduleAlias) module() *moduleInfo { return nil } +func (m *moduleAlias) moduleOrAliasTarget() *moduleInfo { return m.target } +func (m *moduleAlias) moduleOrAliasVariant() variant { return m.variant } + +func (m *moduleInfo) alias() *moduleAlias { return nil } +func (m *moduleInfo) module() *moduleInfo { return m } +func (m *moduleInfo) moduleOrAliasTarget() *moduleInfo { return m } +func (m *moduleInfo) moduleOrAliasVariant() variant { return m.variant } + +type moduleOrAlias interface { + alias() *moduleAlias + module() *moduleInfo + moduleOrAliasTarget() *moduleInfo + moduleOrAliasVariant() variant +} + +type modulesOrAliases []moduleOrAlias + +func (l modulesOrAliases) firstModule() *moduleInfo { + for _, moduleOrAlias := range l { + if m := moduleOrAlias.module(); m != nil { + return m + } + } + panic(fmt.Errorf("no first module!")) +} + +func (l modulesOrAliases) lastModule() *moduleInfo { + for i := range l { + if m := l[len(l)-1-i].module(); m != nil { + return m + } + } + panic(fmt.Errorf("no last module!")) +} + type moduleGroup struct { name string ninjaName string - modules []*moduleInfo - aliases []*moduleAlias + modules modulesOrAliases namespace Namespace } +func (group *moduleGroup) moduleOrAliasByVariantName(name string) moduleOrAlias { + for _, module := range group.modules { + if module.moduleOrAliasVariant().name == name { + return module + } + } + return nil +} + +func (group *moduleGroup) moduleByVariantName(name string) *moduleInfo { + return group.moduleOrAliasByVariantName(name).module() +} + type moduleInfo struct { // set during Parse typeName string @@ -201,8 +250,7 @@ type moduleInfo struct { waitingCount int // set during each runMutator - splitModules []*moduleInfo - pendingAliases []pendingAlias + splitModules modulesOrAliases // set during PrepareBuildActions actionDefs localBuildActions @@ -1256,14 +1304,14 @@ func newVariant(module *moduleInfo, mutatorName string, variationName string, } func (c *Context) createVariations(origModule *moduleInfo, mutatorName string, - defaultVariationName *string, variationNames []string, local bool) ([]*moduleInfo, []error) { + defaultVariationName *string, variationNames []string, local bool) (modulesOrAliases, []error) { if len(variationNames) == 0 { panic(fmt.Errorf("mutator %q passed zero-length variation list for module %q", mutatorName, origModule.Name())) } - newModules := []*moduleInfo{} + var newModules modulesOrAliases var errs []error @@ -1312,37 +1360,19 @@ func (c *Context) convertDepsToVariation(module *moduleInfo, if dep.module.logicModule == nil { var newDep *moduleInfo for _, m := range dep.module.splitModules { - if m.variant.variations[mutatorName] == variationName { - newDep = m + if m.moduleOrAliasVariant().variations[mutatorName] == variationName { + newDep = m.moduleOrAliasTarget() break } } - if newDep == nil { - // check aliases against variationNam - for _, a := range dep.module.pendingAliases { - if a.fromVariant.variations[mutatorName] == variationName { - newDep = a.target - break - } - } - } if newDep == nil && defaultVariationName != nil { // give it a second chance; match with defaultVariationName for _, m := range dep.module.splitModules { - if m.variant.variations[mutatorName] == *defaultVariationName { - newDep = m + if m.moduleOrAliasVariant().variations[mutatorName] == *defaultVariationName { + newDep = m.moduleOrAliasTarget() break } } - if newDep == nil { - // check aliases against defaultVariationName - for _, a := range dep.module.pendingAliases { - if a.fromVariant.variations[mutatorName] == *defaultVariationName { - newDep = a.target - break - } - } - } } if newDep == nil { errs = append(errs, &BlueprintError{ @@ -1372,14 +1402,14 @@ func (c *Context) prettyPrintVariant(variations variationMap) string { func (c *Context) prettyPrintGroupVariants(group *moduleGroup) string { var variants []string - for _, mod := range group.modules { - variants = append(variants, c.prettyPrintVariant(mod.variant.variations)) + for _, moduleOrAlias := range group.modules { + if mod := moduleOrAlias.module(); mod != nil { + variants = append(variants, c.prettyPrintVariant(mod.variant.variations)) + } else if alias := moduleOrAlias.alias(); alias != nil { + variants = append(variants, c.prettyPrintVariant(alias.variant.variations)+ + "(alias to "+c.prettyPrintVariant(alias.target.variant.variations)+")") + } } - for _, mod := range group.aliases { - variants = append(variants, c.prettyPrintVariant(mod.variant.variations)+ - "(alias to "+c.prettyPrintVariant(mod.target.variant.variations)+")") - } - sort.Strings(variants) return strings.Join(variants, "\n ") } @@ -1458,7 +1488,7 @@ func (c *Context) addModule(module *moduleInfo) []error { group := &moduleGroup{ name: name, - modules: []*moduleInfo{module}, + modules: modulesOrAliases{module}, } module.group = group namespace, errs := c.nameInterface.NewModule( @@ -1546,14 +1576,19 @@ func blueprintDepsMutator(ctx BottomUpMutatorContext) { // and returns the matching module, or nil if one is not found. A group with exactly one module // is always considered matching. func findExactVariantOrSingle(module *moduleInfo, possible *moduleGroup, reverse bool) *moduleInfo { - if len(possible.modules) == 1 { - return possible.modules[0] - } else { - found, _ := findVariant(module, possible, nil, false, reverse) - return found + found, _ := findVariant(module, possible, nil, false, reverse) + if found == nil { + for _, moduleOrAlias := range possible.modules { + if m := moduleOrAlias.module(); m != nil { + if found != nil { + // more than one possible match, give up + return nil + } + found = m + } + } } - - return nil + return found } func (c *Context) addDependency(module *moduleInfo, tag DependencyTag, depName string) []error { @@ -1660,21 +1695,12 @@ func findVariant(module *moduleInfo, possibleDeps *moduleGroup, variations []Var var foundDep *moduleInfo for _, m := range possibleDeps.modules { - if check(m.variant.variations) { - foundDep = m + if check(m.moduleOrAliasVariant().variations) { + foundDep = m.moduleOrAliasTarget() break } } - if foundDep == nil { - for _, m := range possibleDeps.aliases { - if check(m.variant.variations) { - foundDep = m.target - break - } - } - } - return foundDep, newVariant } @@ -1689,7 +1715,7 @@ func (c *Context) addVariationDependency(module *moduleInfo, variations []Variat return c.discoveredMissingDependencies(module, depName) } - foundDep, newVariant := c.findVariant(module, possibleDeps, variations, far, false) + foundDep, newVariant := findVariant(module, possibleDeps, variations, far, false) if foundDep == nil { if c.allowMissingDependencies { @@ -1732,14 +1758,16 @@ func (c *Context) addInterVariantDependency(origModule *moduleInfo, tag Dependen } var fromInfo, toInfo *moduleInfo - for _, m := range origModule.splitModules { - if m.logicModule == from { - fromInfo = m - } - if m.logicModule == to { - toInfo = m - if fromInfo != nil { - panic(fmt.Errorf("%q depends on later version of itself", origModule.Name())) + for _, moduleOrAlias := range origModule.splitModules { + if m := moduleOrAlias.module(); m != nil { + if m.logicModule == from { + fromInfo = m + } + if m.logicModule == to { + toInfo = m + if fromInfo != nil { + panic(fmt.Errorf("%q depends on later version of itself", origModule.Name())) + } } } } @@ -1971,7 +1999,9 @@ func (c *Context) updateDependencies() (errs []error) { if dep == module { break } - deps[dep] = true + if depModule := dep.module(); depModule != nil { + deps[depModule] = true + } } for _, dep := range module.directDeps { @@ -2213,7 +2243,7 @@ func (c *Context) runMutator(config interface{}, mutator *mutatorInfo, errsCh := make(chan []error) globalStateCh := make(chan globalStateChange) - newVariationsCh := make(chan []*moduleInfo) + newVariationsCh := make(chan modulesOrAliases) done := make(chan bool) c.depsModified = 0 @@ -2223,8 +2253,6 @@ func (c *Context) runMutator(config interface{}, mutator *mutatorInfo, panic("split module found in sorted module list") } - module.pendingAliases = nil - mctx := &mutatorContext{ baseModuleContext: baseModuleContext{ context: c, @@ -2286,8 +2314,10 @@ func (c *Context) runMutator(config interface{}, mutator *mutatorInfo, newModules = append(newModules, globalStateChange.newModules...) deps = append(deps, globalStateChange.deps...) case newVariations := <-newVariationsCh: - for _, m := range newVariations { - newModuleInfo[m.logicModule] = m + for _, moduleOrAlias := range newVariations { + if m := moduleOrAlias.module(); m != nil { + newModuleInfo[m.logicModule] = m + } } case <-done: return @@ -2311,31 +2341,27 @@ func (c *Context) runMutator(config interface{}, mutator *mutatorInfo, for _, group := range c.moduleGroups { for i := 0; i < len(group.modules); i++ { - module := group.modules[i] + module := group.modules[i].module() + if module == nil { + // Existing alias, skip it + continue + } // Update module group to contain newly split variants if module.splitModules != nil { group.modules, i = spliceModules(group.modules, i, module.splitModules) } - // Create any new aliases. - for _, alias := range module.pendingAliases { - group.aliases = append(group.aliases, &moduleAlias{ - variant: alias.fromVariant, - target: alias.target, - }) - } - // Fix up any remaining dependencies on modules that were split into variants // by replacing them with the first variant for j, dep := range module.directDeps { if dep.module.logicModule == nil { - module.directDeps[j].module = dep.module.splitModules[0] + module.directDeps[j].module = dep.module.splitModules.firstModule() } } if module.createdBy != nil && module.createdBy.logicModule == nil { - module.createdBy = module.createdBy.splitModules[0] + module.createdBy = module.createdBy.splitModules.firstModule() } // Add in any new direct dependencies that were added by the mutator @@ -2344,26 +2370,30 @@ func (c *Context) runMutator(config interface{}, mutator *mutatorInfo, } findAliasTarget := func(variant variant) *moduleInfo { - for _, alias := range group.aliases { - if alias.variant.variations.equal(variant.variations) { - return alias.target + for _, moduleOrAlias := range group.modules { + if alias := moduleOrAlias.alias(); alias != nil { + if alias.variant.variations.equal(variant.variations) { + return alias.target + } } } return nil } // Forward or delete any dangling aliases. - for i := 0; i < len(group.aliases); i++ { - alias := group.aliases[i] - - if alias.target.logicModule == nil { - newTarget := findAliasTarget(alias.target.variant) - if newTarget != nil { - alias.target = newTarget - } else { - // The alias was left dangling, remove it. - group.aliases = append(group.aliases[:i], group.aliases[i+1:]...) - i-- + // Use a manual loop instead of range because len(group.modules) can + // change inside the loop + for i := 0; i < len(group.modules); i++ { + if alias := group.modules[i].alias(); alias != nil { + if alias.target.logicModule == nil { + newTarget := findAliasTarget(alias.target.variant) + if newTarget != nil { + alias.target = newTarget + } else { + // The alias was left dangling, remove it. + group.modules = append(group.modules[:i], group.modules[i+1:]...) + i-- + } } } } @@ -2438,15 +2468,15 @@ func (c *Context) cloneModules() { // Removes modules[i] from the list and inserts newModules... where it was located, returning // the new slice and the index of the last inserted element -func spliceModules(modules []*moduleInfo, i int, newModules []*moduleInfo) ([]*moduleInfo, int) { +func spliceModules(modules modulesOrAliases, i int, newModules modulesOrAliases) (modulesOrAliases, int) { spliceSize := len(newModules) newLen := len(modules) + spliceSize - 1 - var dest []*moduleInfo + var dest modulesOrAliases if cap(modules) >= len(modules)-1+len(newModules) { // We can fit the splice in the existing capacity, do everything in place dest = modules[:newLen] } else { - dest = make([]*moduleInfo, newLen) + dest = make(modulesOrAliases, newLen) copy(dest, modules[:i]) } @@ -2703,14 +2733,8 @@ func (c *Context) moduleMatchingVariant(module *moduleInfo, name string) *module } for _, m := range group.modules { - if module.variant.name == m.variant.name { - return m - } - } - - for _, m := range group.aliases { - if module.variant.name == m.variant.name { - return m.target + if module.variant.variations.equal(m.moduleOrAliasVariant().variations) { + return m.moduleOrAliasTarget() } } @@ -2807,8 +2831,10 @@ func (c *Context) visitAllModules(visit func(Module)) { }() for _, moduleGroup := range c.sortedModuleGroups() { - for _, module = range moduleGroup.modules { - visit(module.logicModule) + for _, moduleOrAlias := range moduleGroup.modules { + if module = moduleOrAlias.module(); module != nil { + visit(module.logicModule) + } } } } @@ -2826,9 +2852,11 @@ func (c *Context) visitAllModulesIf(pred func(Module) bool, }() for _, moduleGroup := range c.sortedModuleGroups() { - for _, module := range moduleGroup.modules { - if pred(module.logicModule) { - visit(module.logicModule) + for _, moduleOrAlias := range moduleGroup.modules { + if module = moduleOrAlias.module(); module != nil { + if pred(module.logicModule) { + visit(module.logicModule) + } } } } @@ -2846,8 +2874,10 @@ func (c *Context) visitAllModuleVariants(module *moduleInfo, } }() - for _, variant = range module.group.modules { - visit(variant.logicModule) + for _, moduleOrAlias := range module.group.modules { + if variant = moduleOrAlias.module(); variant != nil { + visit(variant.logicModule) + } } } @@ -3191,12 +3221,11 @@ func (c *Context) VisitDepsDepthFirstIf(module Module, pred func(Module) bool, v } func (c *Context) PrimaryModule(module Module) Module { - return c.moduleInfo[module].group.modules[0].logicModule + return c.moduleInfo[module].group.modules.firstModule().logicModule } func (c *Context) FinalModule(module Module) Module { - modules := c.moduleInfo[module].group.modules - return modules[len(modules)-1].logicModule + return c.moduleInfo[module].group.modules.lastModule().logicModule } func (c *Context) VisitAllModuleVariants(module Module, @@ -3756,15 +3785,15 @@ func (c *Context) writeLocalBuildActions(nw *ninjaWriter, return nil } -func beforeInModuleList(a, b *moduleInfo, list []*moduleInfo) bool { +func beforeInModuleList(a, b *moduleInfo, list modulesOrAliases) bool { found := false if a == b { return false } for _, l := range list { - if l == a { + if l.module() == a { found = true - } else if l == b { + } else if l.module() == b { return found } } diff --git a/context_test.go b/context_test.go index 073693c..b33c7bf 100644 --- a/context_test.go +++ b/context_test.go @@ -238,7 +238,7 @@ func TestWalkDeps(t *testing.T) { t.FailNow() } - topModule := ctx.moduleGroupFromName("A", nil).modules[0] + topModule := ctx.moduleGroupFromName("A", nil).modules.firstModule() outputDown, outputUp := walkDependencyGraph(ctx, topModule, false) if outputDown != "BCEFG" { t.Errorf("unexpected walkDeps behaviour: %s\ndown should be: BCEFG", outputDown) @@ -319,7 +319,7 @@ func TestWalkDepsDuplicates(t *testing.T) { t.FailNow() } - topModule := ctx.moduleGroupFromName("A", nil).modules[0] + topModule := ctx.moduleGroupFromName("A", nil).modules.firstModule() outputDown, outputUp := walkDependencyGraph(ctx, topModule, true) if outputDown != "BCEGHFGG" { t.Errorf("unexpected walkDeps behaviour: %s\ndown should be: BCEGHFGG", outputDown) @@ -386,7 +386,7 @@ func TestWalkDepsDuplicates_IgnoreFirstPath(t *testing.T) { t.FailNow() } - topModule := ctx.moduleGroupFromName("A", nil).modules[0] + topModule := ctx.moduleGroupFromName("A", nil).modules.firstModule() outputDown, outputUp := walkDependencyGraph(ctx, topModule, true) expectedDown := "BDCDE" if outputDown != expectedDown { @@ -432,10 +432,10 @@ func TestCreateModule(t *testing.T) { t.FailNow() } - a := ctx.moduleGroupFromName("A", nil).modules[0].logicModule.(*fooModule) - b := ctx.moduleGroupFromName("B", nil).modules[0].logicModule.(*barModule) - c := ctx.moduleGroupFromName("C", nil).modules[0].logicModule.(*barModule) - d := ctx.moduleGroupFromName("D", nil).modules[0].logicModule.(*fooModule) + a := ctx.moduleGroupFromName("A", nil).modules.firstModule().logicModule.(*fooModule) + b := ctx.moduleGroupFromName("B", nil).modules.firstModule().logicModule.(*barModule) + c := ctx.moduleGroupFromName("C", nil).modules.firstModule().logicModule.(*barModule) + d := ctx.moduleGroupFromName("D", nil).modules.firstModule().logicModule.(*fooModule) checkDeps := func(m Module, expected string) { var deps []string @@ -622,28 +622,42 @@ func Test_findVariant(t *testing.T) { } type alias struct { - variations variationMap - target int + variant variant + target int } - makeDependencyGroup := func(modules []*moduleInfo, aliases []alias) *moduleGroup { + makeDependencyGroup := func(in ...interface{}) *moduleGroup { group := &moduleGroup{ - name: "dep", - modules: modules, + name: "dep", + } + for _, x := range in { + switch m := x.(type) { + case *moduleInfo: + m.group = group + group.modules = append(group.modules, m) + case alias: + // aliases may need to target modules that haven't been processed + // yet, put an empty alias in for now. + group.modules = append(group.modules, nil) + default: + t.Fatalf("unexpected type %T", x) + } } - for _, alias := range aliases { - group.aliases = append(group.aliases, &moduleAlias{ - variant: variant{ - variations: alias.variations, - }, - target: group.modules[alias.target], - }) + for i, x := range in { + switch m := x.(type) { + case *moduleInfo: + // already added in the first pass + case alias: + group.modules[i] = &moduleAlias{ + variant: m.variant, + target: group.modules[m.target].moduleOrAliasTarget(), + } + default: + t.Fatalf("unexpected type %T", x) + } } - for _, m := range group.modules { - m.group = group - } return group } @@ -658,8 +672,8 @@ func Test_findVariant(t *testing.T) { { name: "AddVariationDependencies(nil)", // A dependency that matches the non-local variations of the module - possibleDeps: makeDependencyGroup([]*moduleInfo{ - { + possibleDeps: makeDependencyGroup( + &moduleInfo{ variant: variant{ name: "normal", variations: variationMap{ @@ -667,7 +681,7 @@ func Test_findVariant(t *testing.T) { }, }, }, - }, nil), + ), variations: nil, far: false, reverse: false, @@ -676,8 +690,17 @@ func Test_findVariant(t *testing.T) { { name: "AddVariationDependencies(nil) to alias", // A dependency with an alias that matches the non-local variations of the module - possibleDeps: makeDependencyGroup([]*moduleInfo{ - { + possibleDeps: makeDependencyGroup( + alias{ + variant: variant{ + name: "normal", + variations: variationMap{ + "normal": "normal", + }, + }, + target: 1, + }, + &moduleInfo{ variant: variant{ name: "normal_a", variations: variationMap{ @@ -686,14 +709,7 @@ func Test_findVariant(t *testing.T) { }, }, }, - }, []alias{ - { - variations: variationMap{ - "normal": "normal", - }, - target: 0, - }, - }), + ), variations: nil, far: false, reverse: false, @@ -702,8 +718,8 @@ func Test_findVariant(t *testing.T) { { name: "AddVariationDependencies(a)", // A dependency with local variations - possibleDeps: makeDependencyGroup([]*moduleInfo{ - { + possibleDeps: makeDependencyGroup( + &moduleInfo{ variant: variant{ name: "normal_a", variations: variationMap{ @@ -712,7 +728,7 @@ func Test_findVariant(t *testing.T) { }, }, }, - }, nil), + ), variations: []Variation{{"a", "a"}}, far: false, reverse: false, @@ -721,8 +737,8 @@ func Test_findVariant(t *testing.T) { { name: "AddFarVariationDependencies(far)", // A dependency with far variations - possibleDeps: makeDependencyGroup([]*moduleInfo{ - { + possibleDeps: makeDependencyGroup( + &moduleInfo{ variant: variant{ name: "far", variations: variationMap{ @@ -730,7 +746,7 @@ func Test_findVariant(t *testing.T) { }, }, }, - }, nil), + ), variations: []Variation{{"far", "far"}}, far: true, reverse: false, @@ -739,8 +755,17 @@ func Test_findVariant(t *testing.T) { { name: "AddFarVariationDependencies(far) to alias", // A dependency with far variations and aliases - possibleDeps: makeDependencyGroup([]*moduleInfo{ - { + possibleDeps: makeDependencyGroup( + alias{ + variant: variant{ + name: "far", + variations: variationMap{ + "far": "far", + }, + }, + target: 2, + }, + &moduleInfo{ variant: variant{ name: "far_a", variations: variationMap{ @@ -749,18 +774,20 @@ func Test_findVariant(t *testing.T) { }, }, }, - }, []alias{ - { - variations: variationMap{ - "far": "far", + &moduleInfo{ + variant: variant{ + name: "far_b", + variations: variationMap{ + "far": "far", + "b": "b", + }, }, - target: 0, }, - }), + ), variations: []Variation{{"far", "far"}}, far: true, reverse: false, - want: "far_a", + want: "far_b", }, } for _, tt := range tests { diff --git a/module_ctx.go b/module_ctx.go index 34f70f4..50fbcf0 100644 --- a/module_ctx.go +++ b/module_ctx.go @@ -643,11 +643,11 @@ func (m *baseModuleContext) WalkDeps(visit func(child, parent Module) bool) { } func (m *baseModuleContext) PrimaryModule() Module { - return m.module.group.modules[0].logicModule + return m.module.group.modules.firstModule().logicModule } func (m *baseModuleContext) FinalModule() Module { - return m.module.group.modules[len(m.module.group.modules)-1].logicModule + return m.module.group.modules.lastModule().logicModule } func (m *baseModuleContext) VisitAllModuleVariants(visit func(Module)) { @@ -722,8 +722,8 @@ type mutatorContext struct { reverseDeps []reverseDep rename []rename replace []replace - newVariations []*moduleInfo // new variants of existing modules - newModules []*moduleInfo // brand new modules + newVariations modulesOrAliases // new variants of existing modules + newModules []*moduleInfo // brand new modules defaultVariation *string } @@ -921,7 +921,7 @@ func (mctx *mutatorContext) createVariations(variationNames []string, local bool } for _, module := range modules { - ret = append(ret, module.logicModule) + ret = append(ret, module.module().logicModule) } if mctx.newVariations != nil { @@ -937,25 +937,30 @@ func (mctx *mutatorContext) createVariations(variationNames []string, local bool } func (mctx *mutatorContext) AliasVariation(variationName string) { - for _, alias := range mctx.module.pendingAliases { - if alias.fromVariant.variations.equal(mctx.module.variant.variations) { - panic(fmt.Errorf("AliasVariation already called")) + for _, moduleOrAlias := range mctx.module.splitModules { + if alias := moduleOrAlias.alias(); alias != nil { + if alias.variant.variations.equal(mctx.module.variant.variations) { + panic(fmt.Errorf("AliasVariation already called")) + } } } for _, variant := range mctx.newVariations { - if variant.variant.variations[mctx.name] == variationName { - mctx.module.pendingAliases = append(mctx.module.pendingAliases, pendingAlias{ - fromVariant: mctx.module.variant, - target: variant, - }) + if variant.moduleOrAliasVariant().variations[mctx.name] == variationName { + alias := &moduleAlias{ + variant: mctx.module.variant, + target: variant.moduleOrAliasTarget(), + } + // Prepend the alias so that AddFarVariationDependencies subset match matches + // the alias before matching the first variation. + mctx.module.splitModules = append(modulesOrAliases{alias}, mctx.module.splitModules...) return } } var foundVariations []string for _, variant := range mctx.newVariations { - foundVariations = append(foundVariations, variant.variant.variations[mctx.name]) + foundVariations = append(foundVariations, variant.moduleOrAliasVariant().variations[mctx.name]) } panic(fmt.Errorf("no %q variation in module variations %q", variationName, foundVariations)) } @@ -963,17 +968,22 @@ func (mctx *mutatorContext) AliasVariation(variationName string) { func (mctx *mutatorContext) CreateAliasVariation(aliasVariationName, targetVariationName string) { newVariant := newVariant(mctx.module, mctx.name, aliasVariationName, false) - for _, alias := range mctx.module.pendingAliases { - if alias.fromVariant.variations.equal(newVariant.variations) { - panic(fmt.Errorf("can't alias %q to %q, already aliased to %q", aliasVariationName, targetVariationName, alias.target.variant.name)) + for _, moduleOrAlias := range mctx.module.splitModules { + if moduleOrAlias.moduleOrAliasVariant().variations.equal(newVariant.variations) { + if alias := moduleOrAlias.alias(); alias != nil { + panic(fmt.Errorf("can't alias %q to %q, already aliased to %q", aliasVariationName, targetVariationName, alias.target.variant.name)) + } else { + panic(fmt.Errorf("can't alias %q to %q, there is already a variant with that name", aliasVariationName, targetVariationName)) + } } } for _, variant := range mctx.newVariations { - if variant.variant.variations[mctx.name] == targetVariationName { - mctx.module.pendingAliases = append(mctx.module.pendingAliases, pendingAlias{ - fromVariant: newVariant, - target: variant, + if variant.moduleOrAliasVariant().variations[mctx.name] == targetVariationName { + // Append the alias here so that it comes after any aliases created by AliasVariation. + mctx.module.splitModules = append(mctx.module.splitModules, &moduleAlias{ + variant: newVariant, + target: variant.moduleOrAliasTarget(), }) return } @@ -981,7 +991,7 @@ func (mctx *mutatorContext) CreateAliasVariation(aliasVariationName, targetVaria var foundVariations []string for _, variant := range mctx.newVariations { - foundVariations = append(foundVariations, variant.variant.variations[mctx.name]) + foundVariations = append(foundVariations, variant.moduleOrAliasVariant().variations[mctx.name]) } panic(fmt.Errorf("no %q variation in module variations %q", targetVariationName, foundVariations)) } diff --git a/module_ctx_test.go b/module_ctx_test.go index 22666dc..62a89c1 100644 --- a/module_ctx_test.go +++ b/module_ctx_test.go @@ -131,12 +131,8 @@ func TestAliasVariation(t *testing.T) { run(ctx) - foo := ctx.moduleGroupFromName("foo", nil).modules[0] - barB := ctx.moduleGroupFromName("bar", nil).modules[1] - - if g, w := barB.variant.name, "b"; g != w { - t.Fatalf("expected bar.modules[1] variant to be %q, got %q", w, g) - } + foo := ctx.moduleGroupFromName("foo", nil).moduleByVariantName("") + barB := ctx.moduleGroupFromName("bar", nil).moduleByVariantName("b") if g, w := foo.forwardDeps, []*moduleInfo{barB}; !reflect.DeepEqual(g, w) { t.Fatalf("expected foo deps to be %q, got %q", w, g) @@ -155,12 +151,8 @@ func TestAliasVariation(t *testing.T) { run(ctx) - foo := ctx.moduleGroupFromName("foo", nil).modules[0] - barBB := ctx.moduleGroupFromName("bar", nil).modules[3] - - if g, w := barBB.variant.name, "b_b"; g != w { - t.Fatalf("expected bar.modules[3] variant to be %q, got %q", w, g) - } + foo := ctx.moduleGroupFromName("foo", nil).moduleByVariantName("") + barBB := ctx.moduleGroupFromName("bar", nil).moduleByVariantName("b_b") if g, w := foo.forwardDeps, []*moduleInfo{barBB}; !reflect.DeepEqual(g, w) { t.Fatalf("expected foo deps to be %q, got %q", w, g) @@ -179,12 +171,8 @@ func TestAliasVariation(t *testing.T) { run(ctx) - foo := ctx.moduleGroupFromName("foo", nil).modules[0] - barAB := ctx.moduleGroupFromName("bar", nil).modules[1] - - if g, w := barAB.variant.name, "a_b"; g != w { - t.Fatalf("expected bar.modules[1] variant to be %q, got %q", w, g) - } + foo := ctx.moduleGroupFromName("foo", nil).moduleByVariantName("") + barAB := ctx.moduleGroupFromName("bar", nil).moduleByVariantName("a_b") if g, w := foo.forwardDeps, []*moduleInfo{barAB}; !reflect.DeepEqual(g, w) { t.Fatalf("expected foo deps to be %q, got %q", w, g) @@ -270,12 +258,8 @@ func TestCreateAliasVariations(t *testing.T) { run(ctx) - foo := ctx.moduleGroupFromName("foo", nil).modules[0] - barB := ctx.moduleGroupFromName("bar", nil).modules[1] - - if g, w := barB.variant.name, "b"; g != w { - t.Fatalf("expected bar.modules[1] variant to be %q, got %q", w, g) - } + foo := ctx.moduleGroupFromName("foo", nil).moduleByVariantName("") + barB := ctx.moduleGroupFromName("bar", nil).moduleByVariantName("b") if g, w := foo.forwardDeps, []*moduleInfo{barB}; !reflect.DeepEqual(g, w) { t.Fatalf("expected foo deps to be %q, got %q", w, g) @@ -294,12 +278,8 @@ func TestCreateAliasVariations(t *testing.T) { run(ctx) - foo := ctx.moduleGroupFromName("foo", nil).modules[0] - barBB := ctx.moduleGroupFromName("bar", nil).modules[3] - - if g, w := barBB.variant.name, "b_b"; g != w { - t.Fatalf("expected bar.modules[3] variant to be %q, got %q", w, g) - } + foo := ctx.moduleGroupFromName("foo", nil).moduleByVariantName("") + barBB := ctx.moduleGroupFromName("bar", nil).moduleByVariantName("b_b") if g, w := foo.forwardDeps, []*moduleInfo{barBB}; !reflect.DeepEqual(g, w) { t.Fatalf("expected foo deps to be %q, got %q", w, g) diff --git a/name_interface.go b/name_interface.go index 1849e9d..5e7e16e 100644 --- a/name_interface.go +++ b/name_interface.go @@ -109,7 +109,7 @@ func (s *SimpleNameInterface) NewModule(ctx NamespaceContext, group ModuleGroup, return nil, []error{ // seven characters at the start of the second line to align with the string "error: " fmt.Errorf("module %q already defined\n"+ - " %s <-- previous definition here", name, group.modules[0].pos), + " %s <-- previous definition here", name, group.modules.firstModule().pos), } } @@ -130,7 +130,7 @@ func (s *SimpleNameInterface) Rename(oldName string, newName string, namespace N // seven characters at the start of the second line to align with the string "error: " fmt.Errorf("renaming module %q to %q conflicts with existing module\n"+ " %s <-- existing module defined here", - oldName, newName, existingGroup.modules[0].pos), + oldName, newName, existingGroup.modules.firstModule().pos), } } diff --git a/splice_modules_test.go b/splice_modules_test.go index 059ff41..473999a 100644 --- a/splice_modules_test.go +++ b/splice_modules_test.go @@ -29,82 +29,82 @@ var ( ) var spliceModulesTestCases = []struct { - in []*moduleInfo + in modulesOrAliases at int - with []*moduleInfo - out []*moduleInfo + with modulesOrAliases + out modulesOrAliases outAt int reallocate bool }{ { // Insert at the beginning - in: []*moduleInfo{testModuleA, testModuleB, testModuleC}, + in: modulesOrAliases{testModuleA, testModuleB, testModuleC}, at: 0, - with: []*moduleInfo{testModuleD, testModuleE}, - out: []*moduleInfo{testModuleD, testModuleE, testModuleB, testModuleC}, + with: modulesOrAliases{testModuleD, testModuleE}, + out: modulesOrAliases{testModuleD, testModuleE, testModuleB, testModuleC}, outAt: 1, reallocate: true, }, { // Insert in the middle - in: []*moduleInfo{testModuleA, testModuleB, testModuleC}, + in: modulesOrAliases{testModuleA, testModuleB, testModuleC}, at: 1, - with: []*moduleInfo{testModuleD, testModuleE}, - out: []*moduleInfo{testModuleA, testModuleD, testModuleE, testModuleC}, + with: modulesOrAliases{testModuleD, testModuleE}, + out: modulesOrAliases{testModuleA, testModuleD, testModuleE, testModuleC}, outAt: 2, reallocate: true, }, { // Insert at the end - in: []*moduleInfo{testModuleA, testModuleB, testModuleC}, + in: modulesOrAliases{testModuleA, testModuleB, testModuleC}, at: 2, - with: []*moduleInfo{testModuleD, testModuleE}, - out: []*moduleInfo{testModuleA, testModuleB, testModuleD, testModuleE}, + with: modulesOrAliases{testModuleD, testModuleE}, + out: modulesOrAliases{testModuleA, testModuleB, testModuleD, testModuleE}, outAt: 3, reallocate: true, }, { // Insert over a single element - in: []*moduleInfo{testModuleA}, + in: modulesOrAliases{testModuleA}, at: 0, - with: []*moduleInfo{testModuleD, testModuleE}, - out: []*moduleInfo{testModuleD, testModuleE}, + with: modulesOrAliases{testModuleD, testModuleE}, + out: modulesOrAliases{testModuleD, testModuleE}, outAt: 1, reallocate: true, }, { // Insert at the beginning without reallocating - in: []*moduleInfo{testModuleA, testModuleB, testModuleC, nil}[0:3], + in: modulesOrAliases{testModuleA, testModuleB, testModuleC, nil}[0:3], at: 0, - with: []*moduleInfo{testModuleD, testModuleE}, - out: []*moduleInfo{testModuleD, testModuleE, testModuleB, testModuleC}, + with: modulesOrAliases{testModuleD, testModuleE}, + out: modulesOrAliases{testModuleD, testModuleE, testModuleB, testModuleC}, outAt: 1, reallocate: false, }, { // Insert in the middle without reallocating - in: []*moduleInfo{testModuleA, testModuleB, testModuleC, nil}[0:3], + in: modulesOrAliases{testModuleA, testModuleB, testModuleC, nil}[0:3], at: 1, - with: []*moduleInfo{testModuleD, testModuleE}, - out: []*moduleInfo{testModuleA, testModuleD, testModuleE, testModuleC}, + with: modulesOrAliases{testModuleD, testModuleE}, + out: modulesOrAliases{testModuleA, testModuleD, testModuleE, testModuleC}, outAt: 2, reallocate: false, }, { // Insert at the end without reallocating - in: []*moduleInfo{testModuleA, testModuleB, testModuleC, nil}[0:3], + in: modulesOrAliases{testModuleA, testModuleB, testModuleC, nil}[0:3], at: 2, - with: []*moduleInfo{testModuleD, testModuleE}, - out: []*moduleInfo{testModuleA, testModuleB, testModuleD, testModuleE}, + with: modulesOrAliases{testModuleD, testModuleE}, + out: modulesOrAliases{testModuleA, testModuleB, testModuleD, testModuleE}, outAt: 3, reallocate: false, }, { // Insert over a single element without reallocating - in: []*moduleInfo{testModuleA, nil}[0:1], + in: modulesOrAliases{testModuleA, nil}[0:1], at: 0, - with: []*moduleInfo{testModuleD, testModuleE}, - out: []*moduleInfo{testModuleD, testModuleE}, + with: modulesOrAliases{testModuleD, testModuleE}, + out: modulesOrAliases{testModuleD, testModuleE}, outAt: 1, reallocate: false, }, @@ -112,7 +112,7 @@ var spliceModulesTestCases = []struct { func TestSpliceModules(t *testing.T) { for _, testCase := range spliceModulesTestCases { - in := make([]*moduleInfo, len(testCase.in), cap(testCase.in)) + in := make(modulesOrAliases, len(testCase.in), cap(testCase.in)) copy(in, testCase.in) origIn := in got, gotAt := spliceModules(in, testCase.at, testCase.with) @@ -139,6 +139,6 @@ func TestSpliceModules(t *testing.T) { } } -func sameArray(a, b []*moduleInfo) bool { +func sameArray(a, b modulesOrAliases) bool { return &a[0:cap(a)][cap(a)-1] == &b[0:cap(b)][cap(b)-1] } diff --git a/visit_test.go b/visit_test.go index efaadba..1c74b93 100644 --- a/visit_test.go +++ b/visit_test.go @@ -149,13 +149,13 @@ func setupVisitTest(t *testing.T) *Context { func TestVisit(t *testing.T) { ctx := setupVisitTest(t) - topModule := ctx.moduleGroupFromName("A", nil).modules[0].logicModule.(*visitModule) + topModule := ctx.moduleGroupFromName("A", nil).modules.firstModule().logicModule.(*visitModule) assertString(t, topModule.properties.VisitDepsDepthFirst, "FEDCB") assertString(t, topModule.properties.VisitDepsDepthFirstIf, "FEDC") assertString(t, topModule.properties.VisitDirectDeps, "B") assertString(t, topModule.properties.VisitDirectDepsIf, "") - eModule := ctx.moduleGroupFromName("E", nil).modules[0].logicModule.(*visitModule) + eModule := ctx.moduleGroupFromName("E", nil).modules.firstModule().logicModule.(*visitModule) assertString(t, eModule.properties.VisitDepsDepthFirst, "F") assertString(t, eModule.properties.VisitDepsDepthFirstIf, "F") assertString(t, eModule.properties.VisitDirectDeps, "FF")