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

@@ -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
+	modules modulesOrAliases
-	aliases []*moduleAlias
 
 	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
+	splitModules modulesOrAliases
-	pendingAliases []pendingAlias
 
 	// 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 {
+				if m.moduleOrAliasVariant().variations[mutatorName] == variationName {
-					newDep = m
+					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 {
+					if m.moduleOrAliasVariant().variations[mutatorName] == *defaultVariationName {
-						newDep = m
+						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 {
+	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,15 +1576,20 @@ 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
+	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 found
+}
 
 func (c *Context) addDependency(module *moduleInfo, tag DependencyTag, depName string) []error {
 	if _, ok := tag.(BaseDependencyTag); ok {
@@ -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) {
+		if check(m.moduleOrAliasVariant().variations) {
-			foundDep = m
+			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,7 +1758,8 @@ func (c *Context) addInterVariantDependency(origModule *moduleInfo, tag Dependen
 	}
 
 	var fromInfo, toInfo *moduleInfo
-	for _, m := range origModule.splitModules {
+	for _, moduleOrAlias := range origModule.splitModules {
+		if m := moduleOrAlias.module(); m != nil {
 			if m.logicModule == from {
 				fromInfo = m
 			}
@@ -1743,6 +1770,7 @@ func (c *Context) addInterVariantDependency(origModule *moduleInfo, tag Dependen
 				}
 			}
 		}
+	}
 
 	if fromInfo == nil || toInfo == nil {
 		panic(fmt.Errorf("AddInterVariantDependency called for module %q on invalid variant",
@@ -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,9 +2314,11 @@ 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 {
+				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,30 +2370,34 @@ func (c *Context) runMutator(config interface{}, mutator *mutatorInfo,
 		}
 
 		findAliasTarget := func(variant variant) *moduleInfo {
-			for _, alias := range group.aliases {
+			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++ {
+		// Use a manual loop instead of range because len(group.modules) can
-			alias := group.aliases[i]
+		// 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.aliases = append(group.aliases[:i], group.aliases[i+1:]...)
+						group.modules = append(group.modules[:i], group.modules[i+1:]...)
 						i--
 					}
 				}
 			}
 		}
+	}
 
 	// Add in any new reverse dependencies that were added by the mutator
 	for module, deps := range reverseDeps {
@@ -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 {
+		if module.variant.variations.equal(m.moduleOrAliasVariant().variations) {
-			return m
+			return m.moduleOrAliasTarget()
-		}
-	}
-
-	for _, m := range group.aliases {
-		if module.variant.name == m.variant.name {
-			return m.target
 		}
 	}
 
@@ -2807,11 +2831,13 @@ func (c *Context) visitAllModules(visit func(Module)) {
 	}()
 
 	for _, moduleGroup := range c.sortedModuleGroups() {
-		for _, module = range moduleGroup.modules {
+		for _, moduleOrAlias := range moduleGroup.modules {
+			if module = moduleOrAlias.module(); module != nil {
 				visit(module.logicModule)
 			}
 		}
 	}
+}
 
 func (c *Context) visitAllModulesIf(pred func(Module) bool,
 	visit func(Module)) {
@@ -2826,13 +2852,15 @@ func (c *Context) visitAllModulesIf(pred func(Module) bool,
 	}()
 
 	for _, moduleGroup := range c.sortedModuleGroups() {
-		for _, module := range moduleGroup.modules {
+		for _, moduleOrAlias := range moduleGroup.modules {
+			if module = moduleOrAlias.module(); module != nil {
 				if pred(module.logicModule) {
 					visit(module.logicModule)
 				}
 			}
 		}
 	}
+}
 
 func (c *Context) visitAllModuleVariants(module *moduleInfo,
 	visit func(Module)) {
@@ -2846,10 +2874,12 @@ func (c *Context) visitAllModuleVariants(module *moduleInfo,
 		}
 	}()
 
-	for _, variant = range module.group.modules {
+	for _, moduleOrAlias := range module.group.modules {
+		if variant = moduleOrAlias.module(); variant != nil {
 			visit(variant.logicModule)
 		}
 	}
+}
 
 func (c *Context) requireNinjaVersion(major, minor, micro int) {
 	if major != 1 {
@@ -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 c.moduleInfo[module].group.modules.lastModule().logicModule
-	return modules[len(modules)-1].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
 		}
 	}

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)
+	a := ctx.moduleGroupFromName("A", nil).modules.firstModule().logicModule.(*fooModule)
-	b := ctx.moduleGroupFromName("B", nil).modules[0].logicModule.(*barModule)
+	b := ctx.moduleGroupFromName("B", nil).modules.firstModule().logicModule.(*barModule)
-	c := ctx.moduleGroupFromName("C", nil).modules[0].logicModule.(*barModule)
+	c := ctx.moduleGroupFromName("C", nil).modules.firstModule().logicModule.(*barModule)
-	d := ctx.moduleGroupFromName("D", nil).modules[0].logicModule.(*fooModule)
+	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
+		variant variant
 		target  int
 	}
 
-	makeDependencyGroup := func(modules []*moduleInfo, aliases []alias) *moduleGroup {
+	makeDependencyGroup := func(in ...interface{}) *moduleGroup {
 		group := &moduleGroup{
 			name: "dep",
-			modules: modules,
 		}
-
+		for _, x := range in {
-		for _, alias := range aliases {
+			switch m := x.(type) {
-			group.aliases = append(group.aliases, &moduleAlias{
+			case *moduleInfo:
-				variant: variant{
-					variations: alias.variations,
-				},
-				target: group.modules[alias.target],
-			})
-		}
-
-		for _, m := range group.modules {
 				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 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)
+			}
+		}
+
 		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{
+				&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 {

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,7 +722,7 @@ type mutatorContext struct {
 	reverseDeps      []reverseDep
 	rename           []rename
 	replace          []replace
-	newVariations    []*moduleInfo // new variants of existing 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 {
+	for _, moduleOrAlias := range mctx.module.splitModules {
-		if alias.fromVariant.variations.equal(mctx.module.variant.variations) {
+		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 {
+		if variant.moduleOrAliasVariant().variations[mctx.name] == variationName {
-			mctx.module.pendingAliases = append(mctx.module.pendingAliases, pendingAlias{
+			alias := &moduleAlias{
-				fromVariant: mctx.module.variant,
+				variant: mctx.module.variant,
-				target:      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 {
+	for _, moduleOrAlias := range mctx.module.splitModules {
-		if alias.fromVariant.variations.equal(newVariant.variations) {
+		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 {
+		if variant.moduleOrAliasVariant().variations[mctx.name] == targetVariationName {
-			mctx.module.pendingAliases = append(mctx.module.pendingAliases, pendingAlias{
+			// Append the alias here so that it comes after any aliases created by AliasVariation.
-				fromVariant: newVariant,
+			mctx.module.splitModules = append(mctx.module.splitModules, &moduleAlias{
-				target:      variant,
+				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))
 }

module_ctx_test.go

@@ -131,12 +131,8 @@ func TestAliasVariation(t *testing.T) {
 
 		run(ctx)
 
-		foo := ctx.moduleGroupFromName("foo", nil).modules[0]
+		foo := ctx.moduleGroupFromName("foo", nil).moduleByVariantName("")
-		barB := ctx.moduleGroupFromName("bar", nil).modules[1]
+		barB := ctx.moduleGroupFromName("bar", nil).moduleByVariantName("b")
-
-		if g, w := barB.variant.name, "b"; g != w {
-			t.Fatalf("expected bar.modules[1] variant to be %q, got %q", w, g)
-		}
 
 		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]
+		foo := ctx.moduleGroupFromName("foo", nil).moduleByVariantName("")
-		barBB := ctx.moduleGroupFromName("bar", nil).modules[3]
+		barBB := ctx.moduleGroupFromName("bar", nil).moduleByVariantName("b_b")
-
-		if g, w := barBB.variant.name, "b_b"; g != w {
-			t.Fatalf("expected bar.modules[3] variant to be %q, got %q", w, g)
-		}
 
 		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]
+		foo := ctx.moduleGroupFromName("foo", nil).moduleByVariantName("")
-		barAB := ctx.moduleGroupFromName("bar", nil).modules[1]
+		barAB := ctx.moduleGroupFromName("bar", nil).moduleByVariantName("a_b")
-
-		if g, w := barAB.variant.name, "a_b"; g != w {
-			t.Fatalf("expected bar.modules[1] variant to be %q, got %q", w, g)
-		}
 
 		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]
+		foo := ctx.moduleGroupFromName("foo", nil).moduleByVariantName("")
-		barB := ctx.moduleGroupFromName("bar", nil).modules[1]
+		barB := ctx.moduleGroupFromName("bar", nil).moduleByVariantName("b")
-
-		if g, w := barB.variant.name, "b"; g != w {
-			t.Fatalf("expected bar.modules[1] variant to be %q, got %q", w, g)
-		}
 
 		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]
+		foo := ctx.moduleGroupFromName("foo", nil).moduleByVariantName("")
-		barBB := ctx.moduleGroupFromName("bar", nil).modules[3]
+		barBB := ctx.moduleGroupFromName("bar", nil).moduleByVariantName("b_b")
-
-		if g, w := barBB.variant.name, "b_b"; g != w {
-			t.Fatalf("expected bar.modules[3] variant to be %q, got %q", w, g)
-		}
 
 		if g, w := foo.forwardDeps, []*moduleInfo{barBB}; !reflect.DeepEqual(g, w) {
 			t.Fatalf("expected foo deps to be %q, got %q", w, g)

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),
 		}
 	}
 

splice_modules_test.go

@@ -29,82 +29,82 @@ var (
 )
 
 var spliceModulesTestCases = []struct {
-	in         []*moduleInfo
+	in         modulesOrAliases
 	at         int
-	with       []*moduleInfo
+	with       modulesOrAliases
-	out        []*moduleInfo
+	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},
+		with:       modulesOrAliases{testModuleD, testModuleE},
-		out:        []*moduleInfo{testModuleD, testModuleE, testModuleB, testModuleC},
+		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},
+		with:       modulesOrAliases{testModuleD, testModuleE},
-		out:        []*moduleInfo{testModuleA, testModuleD, testModuleE, testModuleC},
+		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},
+		with:       modulesOrAliases{testModuleD, testModuleE},
-		out:        []*moduleInfo{testModuleA, testModuleB, 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},
+		with:       modulesOrAliases{testModuleD, testModuleE},
-		out:        []*moduleInfo{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},
+		with:       modulesOrAliases{testModuleD, testModuleE},
-		out:        []*moduleInfo{testModuleD, testModuleE, testModuleB, testModuleC},
+		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},
+		with:       modulesOrAliases{testModuleD, testModuleE},
-		out:        []*moduleInfo{testModuleA, testModuleD, testModuleE, testModuleC},
+		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},
+		with:       modulesOrAliases{testModuleD, testModuleE},
-		out:        []*moduleInfo{testModuleA, testModuleB, 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},
+		with:       modulesOrAliases{testModuleD, testModuleE},
-		out:        []*moduleInfo{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]
 }

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")