Snap for 6439596 from ba1ea7583953186a1a5519c0cd1087e403ad516f to qt-aml-tzdata-releaseq_tzdata_aml_297100400q_tzdata_aml_297100300q_tzdata_aml_297100000q_tzdata_aml_296200000q_tzdata_aml_295600118q_tzdata_aml_295600110q_tzdata_aml_295500002q_tzdata_aml_295500001q_tzdata_aml_294400310android-mainline-12.0.0_r54android-mainline-12.0.0_r111android-mainline-10.0.0_r13android-mainline-10.0.0_r12android-mainline-10.0.0_r11q_tzdata_aml_297100000android12-mainline-tzdata-releaseandroid10-mainline-tzdata-releaseandroid10-android13-mainline-tzdata-release

Change-Id: Ie8d9fbfba3393b6aaf6fee881241593b47c79967

25 files changed, 229 insertions, 1123 deletions

diff --git a/.travis.yml b/.travis.yml
index 706e469..6abd686 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -1,10 +1,10 @@
 language: go
 
 go:
+    - 1.9
     - "1.10"
     - "1.11"
     - "1.12"
-    - "1.13"
 
 cache:
     directories:
diff --git a/Blueprints b/Blueprints
index c3c8975..35e4190 100644
--- a/Blueprints
+++ b/Blueprints
@@ -79,7 +79,6 @@ bootstrap_go_package {
         "proptools/clone.go",
         "proptools/escape.go",
         "proptools/extend.go",
-        "proptools/filter.go",
         "proptools/proptools.go",
         "proptools/tag.go",
         "proptools/typeequal.go",
@@ -88,7 +87,6 @@ bootstrap_go_package {
         "proptools/clone_test.go",
         "proptools/escape_test.go",
         "proptools/extend_test.go",
-        "proptools/filter_test.go",
         "proptools/tag_test.go",
         "proptools/typeequal_test.go",
     ],
@@ -127,8 +125,6 @@ bootstrap_go_package {
         "bootstrap/bpdoc/reader.go",
     ],
     testSrcs: [
-        "bootstrap/bpdoc/bpdoc_test.go",
-        "bootstrap/bpdoc/properties_test.go",
         "bootstrap/bpdoc/reader_test.go",
     ],
 }
diff --git a/bootstrap/bootstrap.go b/bootstrap/bootstrap.go
index f6146b5..3464c0b 100644
--- a/bootstrap/bootstrap.go
+++ b/bootstrap/bootstrap.go
@@ -27,7 +27,6 @@ import (
 	"github.com/google/blueprint/pathtools"
 )
 
-const mainSubDir = ".primary"
 const bootstrapSubDir = ".bootstrap"
 const miniBootstrapSubDir = ".minibootstrap"
 
@@ -144,16 +143,15 @@ var (
 		"depfile")
 
 	_ = pctx.VariableFunc("BinDir", func(config interface{}) (string, error) {
-		return bootstrapBinDir(), nil
+		return binDir(), nil
 	})
 
 	_ = pctx.VariableFunc("ToolDir", func(config interface{}) (string, error) {
 		return toolDir(config), nil
 	})
 
-	docsDir = filepath.Join(mainDir, "docs")
+	docsDir = filepath.Join(bootstrapDir, "docs")
 
-	mainDir          = filepath.Join("$buildDir", mainSubDir)
 	bootstrapDir     = filepath.Join("$buildDir", bootstrapSubDir)
 	miniBootstrapDir = filepath.Join("$buildDir", miniBootstrapSubDir)
 
@@ -167,7 +165,7 @@ type GoBinaryTool interface {
 	isGoBinary()
 }
 
-func bootstrapBinDir() string {
+func binDir() string {
 	return filepath.Join(BuildDir, bootstrapSubDir, "bin")
 }
 
@@ -309,14 +307,14 @@ func (g *goPackage) GenerateBuildActions(ctx blueprint.ModuleContext) {
 		return
 	}
 
-	g.pkgRoot = packageRoot(ctx, g.config)
+	g.pkgRoot = packageRoot(ctx)
 	g.archiveFile = filepath.Join(g.pkgRoot,
 		filepath.FromSlash(g.properties.PkgPath)+".a")
 
 	ctx.VisitDepsDepthFirstIf(isGoPluginFor(name),
 		func(module blueprint.Module) { hasPlugins = true })
 	if hasPlugins {
-		pluginSrc = filepath.Join(moduleGenSrcDir(ctx, g.config), "plugin.go")
+		pluginSrc = filepath.Join(moduleGenSrcDir(ctx), "plugin.go")
 		genSrcs = append(genSrcs, pluginSrc)
 	}
 
@@ -334,9 +332,9 @@ func (g *goPackage) GenerateBuildActions(ctx blueprint.ModuleContext) {
 	}
 
 	if g.config.runGoTests {
-		testArchiveFile := filepath.Join(testRoot(ctx, g.config),
+		testArchiveFile := filepath.Join(testRoot(ctx),
 			filepath.FromSlash(g.properties.PkgPath)+".a")
-		g.testResultFile = buildGoTest(ctx, testRoot(ctx, g.config), testArchiveFile,
+		g.testResultFile = buildGoTest(ctx, testRoot(ctx), testArchiveFile,
 			g.properties.PkgPath, srcs, genSrcs,
 			testSrcs)
 	}
@@ -397,9 +395,9 @@ func (g *goBinary) InstallPath() string {
 func (g *goBinary) GenerateBuildActions(ctx blueprint.ModuleContext) {
 	var (
 		name            = ctx.ModuleName()
-		objDir          = moduleObjDir(ctx, g.config)
+		objDir          = moduleObjDir(ctx)
 		archiveFile     = filepath.Join(objDir, name+".a")
-		testArchiveFile = filepath.Join(testRoot(ctx, g.config), name+".a")
+		testArchiveFile = filepath.Join(testRoot(ctx), name+".a")
 		aoutFile        = filepath.Join(objDir, "a.out")
 		hasPlugins      = false
 		pluginSrc       = ""
@@ -408,16 +406,14 @@ func (g *goBinary) GenerateBuildActions(ctx blueprint.ModuleContext) {
 
 	if g.properties.Tool_dir {
 		g.installPath = filepath.Join(toolDir(ctx.Config()), name)
-	} else if g.config.stage == StageMain {
-		g.installPath = filepath.Join(mainDir, "bin", name)
 	} else {
-		g.installPath = filepath.Join(bootstrapDir, "bin", name)
+		g.installPath = filepath.Join(binDir(), name)
 	}
 
 	ctx.VisitDepsDepthFirstIf(isGoPluginFor(name),
 		func(module blueprint.Module) { hasPlugins = true })
 	if hasPlugins {
-		pluginSrc = filepath.Join(moduleGenSrcDir(ctx, g.config), "plugin.go")
+		pluginSrc = filepath.Join(moduleGenSrcDir(ctx), "plugin.go")
 		genSrcs = append(genSrcs, pluginSrc)
 	}
 
@@ -437,11 +433,11 @@ func (g *goBinary) GenerateBuildActions(ctx blueprint.ModuleContext) {
 	}
 
 	if g.config.runGoTests {
-		deps = buildGoTest(ctx, testRoot(ctx, g.config), testArchiveFile,
+		deps = buildGoTest(ctx, testRoot(ctx), testArchiveFile,
 			name, srcs, genSrcs, testSrcs)
 	}
 
-	buildGoPackage(ctx, objDir, "main", archiveFile, srcs, genSrcs)
+	buildGoPackage(ctx, objDir, name, archiveFile, srcs, genSrcs)
 
 	var linkDeps []string
 	var libDirFlags []string
@@ -762,26 +758,18 @@ func (s *singleton) GenerateBuildActions(ctx blueprint.SingletonContext) {
 	}
 }
 
-func stageDir(config *Config) string {
-	if config.stage == StageMain {
-		return mainDir
-	} else {
-		return bootstrapDir
-	}
-}
-
 // packageRoot returns the module-specific package root directory path.  This
 // directory is where the final package .a files are output and where dependant
 // modules search for this package via -I arguments.
-func packageRoot(ctx blueprint.ModuleContext, config *Config) string {
-	return filepath.Join(stageDir(config), ctx.ModuleName(), "pkg")
+func packageRoot(ctx blueprint.ModuleContext) string {
+	return filepath.Join(bootstrapDir, ctx.ModuleName(), "pkg")
 }
 
 // testRoot returns the module-specific package root directory path used for
 // building tests. The .a files generated here will include everything from
 // packageRoot, plus the test-only code.
-func testRoot(ctx blueprint.ModuleContext, config *Config) string {
-	return filepath.Join(stageDir(config), ctx.ModuleName(), "test")
+func testRoot(ctx blueprint.ModuleContext) string {
+	return filepath.Join(bootstrapDir, ctx.ModuleName(), "test")
 }
 
 // moduleSrcDir returns the path of the directory that all source file paths are
@@ -791,11 +779,11 @@ func moduleSrcDir(ctx blueprint.ModuleContext) string {
 }
 
 // moduleObjDir returns the module-specific object directory path.
-func moduleObjDir(ctx blueprint.ModuleContext, config *Config) string {
-	return filepath.Join(stageDir(config), ctx.ModuleName(), "obj")
+func moduleObjDir(ctx blueprint.ModuleContext) string {
+	return filepath.Join(bootstrapDir, ctx.ModuleName(), "obj")
 }
 
 // moduleGenSrcDir returns the module-specific generated sources path.
-func moduleGenSrcDir(ctx blueprint.ModuleContext, config *Config) string {
-	return filepath.Join(stageDir(config), ctx.ModuleName(), "gen")
+func moduleGenSrcDir(ctx blueprint.ModuleContext) string {
+	return filepath.Join(bootstrapDir, ctx.ModuleName(), "gen")
 }
diff --git a/bootstrap/bpdoc/bpdoc.go b/bootstrap/bpdoc/bpdoc.go
index 4acfc5d..8ce41cf 100644
--- a/bootstrap/bpdoc/bpdoc.go
+++ b/bootstrap/bpdoc/bpdoc.go
@@ -173,9 +173,6 @@ func nestedPropertyStructs(s reflect.Value) map[string]reflect.Value {
 				// The field is not exported so just skip it.
 				continue
 			}
-			if proptools.HasTag(field, "blueprint", "mutated") {
-				continue
-			}
 
 			fieldValue := structValue.Field(i)
 
diff --git a/bootstrap/bpdoc/bpdoc_test.go b/bootstrap/bpdoc/bpdoc_test.go
deleted file mode 100644
index 687d97b..0000000
--- a/bootstrap/bpdoc/bpdoc_test.go
+++ /dev/null
@@ -1,46 +0,0 @@
-package bpdoc
-
-import (
-	"reflect"
-	"testing"
-)
-
-type parentProps struct {
-	A string
-
-	Child *childProps
-
-	Mutated *mutatedProps `blueprint:"mutated"`
-}
-
-type childProps struct {
-	B int
-
-	Child *grandchildProps
-}
-
-type grandchildProps struct {
-	C bool
-}
-
-type mutatedProps struct {
-	D int
-}
-
-func TestNestedPropertyStructs(t *testing.T) {
-	parent := parentProps{Child: &childProps{Child: &grandchildProps{}}, Mutated: &mutatedProps{}}
-
-	allStructs := nestedPropertyStructs(reflect.ValueOf(parent))
-
-	// mutated shouldn't be found because it's a mutated property.
-	expected := []string{"child", "child.child"}
-	if len(allStructs) != len(expected) {
-		t.Errorf("expected %d structs, got %d, all entries: %q",
-			len(expected), len(allStructs), allStructs)
-	}
-	for _, e := range expected {
-		if _, ok := allStructs[e]; !ok {
-			t.Errorf("missing entry %q, all entries: %q", e, allStructs)
-		}
-	}
-}
diff --git a/bootstrap/bpdoc/properties.go b/bootstrap/bpdoc/properties.go
index 9256d8e..23b1ffd 100644
--- a/bootstrap/bpdoc/properties.go
+++ b/bootstrap/bpdoc/properties.go
@@ -250,23 +250,17 @@ func filterPropsByTag(props *[]Property, key, value string, exclude bool) {
 	// len(props) times to this slice will overwrite the original slice contents
 	filtered := (*props)[:0]
 	for _, x := range *props {
-		if hasTag(x.Tag, key, value) == !exclude {
-			filtered = append(filtered, x)
+		tag := x.Tag.Get(key)
+		for _, entry := range strings.Split(tag, ",") {
+			if (entry == value) == !exclude {
+				filtered = append(filtered, x)
+			}
 		}
 	}
 
 	*props = filtered
 }
 
-func hasTag(tag reflect.StructTag, key, value string) bool {
-	for _, entry := range strings.Split(tag.Get(key), ",") {
-		if entry == value {
-			return true
-		}
-	}
-	return false
-}
-
 func formatText(text string) template.HTML {
 	var html template.HTML
 	lines := strings.Split(text, "\n")
diff --git a/bootstrap/bpdoc/properties_test.go b/bootstrap/bpdoc/properties_test.go
deleted file mode 100644
index 4045cb1..0000000
--- a/bootstrap/bpdoc/properties_test.go
+++ /dev/null
@@ -1,58 +0,0 @@
-// Copyright 2019 Google Inc. All rights reserved.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//     http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package bpdoc
-
-import (
-	"reflect"
-	"testing"
-)
-
-func TestExcludeByTag(t *testing.T) {
-	r := NewReader(pkgFiles)
-	ps, err := r.PropertyStruct(pkgPath, "tagTestProps", reflect.ValueOf(tagTestProps{}))
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	ps.ExcludeByTag("tag1", "a")
-
-	expected := []string{"c"}
-	actual := []string{}
-	for _, p := range ps.Properties {
-		actual = append(actual, p.Name)
-	}
-	if !reflect.DeepEqual(expected, actual) {
-		t.Errorf("unexpected ExcludeByTag result, expected: %q, actual: %q", expected, actual)
-	}
-}
-
-func TestIncludeByTag(t *testing.T) {
-	r := NewReader(pkgFiles)
-	ps, err := r.PropertyStruct(pkgPath, "tagTestProps", reflect.ValueOf(tagTestProps{A: "B"}))
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	ps.IncludeByTag("tag1", "c")
-
-	expected := []string{"b", "c"}
-	actual := []string{}
-	for _, p := range ps.Properties {
-		actual = append(actual, p.Name)
-	}
-	if !reflect.DeepEqual(expected, actual) {
-		t.Errorf("unexpected IncludeByTag result, expected: %q, actual: %q", expected, actual)
-	}
-}
diff --git a/bootstrap/bpdoc/reader_test.go b/bootstrap/bpdoc/reader_test.go
index 0d608b3..b8ff109 100644
--- a/bootstrap/bpdoc/reader_test.go
+++ b/bootstrap/bpdoc/reader_test.go
@@ -34,13 +34,6 @@ type props struct {
 	A string
 }
 
-// for properties_test.go
-type tagTestProps struct {
-	A string `tag1:"a,b" tag2:"c"`
-	B string `tag1:"a,c"`
-	C string `tag1:"b,c"`
-}
-
 var pkgPath string
 var pkgFiles map[string][]string
 
diff --git a/bootstrap/bpglob/bpglob.go b/bootstrap/bpglob/bpglob.go
index fe47b6f..1097760 100644
--- a/bootstrap/bpglob/bpglob.go
+++ b/bootstrap/bpglob/bpglob.go
@@ -21,9 +21,7 @@ package main
 import (
 	"flag"
 	"fmt"
-	"io/ioutil"
 	"os"
-	"time"
 
 	"github.com/google/blueprint/pathtools"
 )
@@ -73,15 +71,7 @@ func main() {
 
 	_, err := pathtools.GlobWithDepFile(flag.Arg(0), *out, *out+".d", excludes)
 	if err != nil {
-		// Globs here were already run in the primary builder without error.  The only errors here should be if the glob
-		// pattern was made invalid by a change in the pathtools glob implementation, in which case the primary builder
-		// needs to be rerun anyways.  Update the output file with something that will always cause the primary builder
-		// to rerun.
-		s := fmt.Sprintf("%s: error: %s\n", time.Now().Format(time.StampNano), err.Error())
-		err := ioutil.WriteFile(*out, []byte(s), 0666)
-		if err != nil {
-			fmt.Fprintf(os.Stderr, "error: %s\n", err.Error())
-			os.Exit(1)
-		}
+		fmt.Fprintf(os.Stderr, "error: %s\n", err.Error())
+		os.Exit(1)
 	}
 }
diff --git a/bpfmt/bpfmt.go b/bpfmt/bpfmt.go
index c287ea2..17fe513 100644
--- a/bpfmt/bpfmt.go
+++ b/bpfmt/bpfmt.go
@@ -125,7 +125,6 @@ func walkDir(path string) {
 }
 
 func main() {
-	flag.Usage = usage
 	flag.Parse()
 
 	if !*writeToStout && !*overwriteSourceFile && !*doDiff && !*list {
@@ -136,7 +135,8 @@ func main() {
 		// file to parse is stdin
 		if *overwriteSourceFile {
 			fmt.Fprintln(os.Stderr, "error: cannot use -w with standard input")
-			os.Exit(2)
+			exitCode = 2
+			return
 		}
 		if err := processReader("<standard input>", os.Stdin, os.Stdout); err != nil {
 			report(err)
@@ -157,8 +157,6 @@ func main() {
 			}
 		}
 	}
-
-	os.Exit(exitCode)
 }
 
 func diff(b1, b2 []byte) (data []byte, err error) {
diff --git a/context.go b/context.go
index cedf3d8..23d9262 100644
--- a/context.go
+++ b/context.go
@@ -173,7 +173,6 @@ type moduleInfo struct {
 	relBlueprintsFile string
 	pos               scanner.Position
 	propertyPos       map[string]scanner.Position
-	createdBy         *moduleInfo
 
 	variantName       string
 	variant           variationMap
@@ -184,13 +183,12 @@ type moduleInfo struct {
 	properties  []interface{}
 
 	// set during ResolveDependencies
-	missingDeps   []string
-	newDirectDeps []depInfo
+	directDeps  []depInfo
+	missingDeps []string
 
 	// set during updateDependencies
 	reverseDeps []*moduleInfo
 	forwardDeps []*moduleInfo
-	directDeps  []depInfo
 
 	// used by parallelVisitAllBottomUp
 	waitingCount int
@@ -216,10 +214,6 @@ func (module *moduleInfo) String() string {
 	if module.variantName != "" {
 		s += fmt.Sprintf(" variant %q", module.variantName)
 	}
-	if module.createdBy != nil {
-		s += fmt.Sprintf(" (created by %s)", module.createdBy)
-	}
-
 	return s
 }
 
@@ -1410,21 +1404,12 @@ func blueprintDepsMutator(ctx BottomUpMutatorContext) {
 
 // findMatchingVariant searches the moduleGroup for a module with the same variant as module,
 // and returns the matching module, or nil if one is not found.
-func (c *Context) findMatchingVariant(module *moduleInfo, possible []*moduleInfo, reverse bool) *moduleInfo {
+func (c *Context) findMatchingVariant(module *moduleInfo, possible []*moduleInfo) *moduleInfo {
 	if len(possible) == 1 {
 		return possible[0]
 	} else {
-		var variantToMatch variationMap
-		if !reverse {
-			// For forward dependency, ignore local variants by matching against
-			// dependencyVariant which doesn't have the local variants
-			variantToMatch = module.dependencyVariant
-		} else {
-			// For reverse dependency, use all the variants
-			variantToMatch = module.variant
-		}
 		for _, m := range possible {
-			if m.variant.equal(variantToMatch) {
+			if m.variant.equal(module.dependencyVariant) {
 				return m
 			}
 		}
@@ -1450,8 +1435,8 @@ func (c *Context) addDependency(module *moduleInfo, tag DependencyTag, depName s
 		return c.discoveredMissingDependencies(module, depName)
 	}
 
-	if m := c.findMatchingVariant(module, possibleDeps, false); m != nil {
-		module.newDirectDeps = append(module.newDirectDeps, depInfo{m, tag})
+	if m := c.findMatchingVariant(module, possibleDeps); m != nil {
+		module.directDeps = append(module.directDeps, depInfo{m, tag})
 		atomic.AddUint32(&c.depsModified, 1)
 		return nil
 	}
@@ -1488,7 +1473,7 @@ func (c *Context) findReverseDependency(module *moduleInfo, destName string) (*m
 		}}
 	}
 
-	if m := c.findMatchingVariant(module, possibleDeps, true); m != nil {
+	if m := c.findMatchingVariant(module, possibleDeps); m != nil {
 		return m, nil
 	}
 
@@ -1554,7 +1539,7 @@ func (c *Context) addVariationDependency(module *moduleInfo, variations []Variat
 					Pos: module.pos,
 				}}
 			}
-			module.newDirectDeps = append(module.newDirectDeps, depInfo{m, tag})
+			module.directDeps = append(module.directDeps, depInfo{m, tag})
 			atomic.AddUint32(&c.depsModified, 1)
 			return nil
 		}
@@ -1599,7 +1584,7 @@ func (c *Context) addInterVariantDependency(origModule *moduleInfo, tag Dependen
 			origModule.Name()))
 	}
 
-	fromInfo.newDirectDeps = append(fromInfo.newDirectDeps, depInfo{toInfo, tag})
+	fromInfo.directDeps = append(fromInfo.directDeps, depInfo{toInfo, tag})
 	atomic.AddUint32(&c.depsModified, 1)
 }
 
@@ -2173,18 +2158,9 @@ func (c *Context) runMutator(config interface{}, mutator *mutatorInfo,
 					module.directDeps[j].module = dep.module.splitModules[0]
 				}
 			}
-
-			if module.createdBy != nil && module.createdBy.logicModule == nil {
-				module.createdBy = module.createdBy.splitModules[0]
-			}
-
-			// Add in any new direct dependencies that were added by the mutator
-			module.directDeps = append(module.directDeps, module.newDirectDeps...)
-			module.newDirectDeps = nil
 		}
 	}
 
-	// Add in any new reverse dependencies that were added by the mutator
 	for module, deps := range reverseDeps {
 		sort.Sort(depSorter(deps))
 		module.directDeps = append(module.directDeps, deps...)
diff --git a/doc.go b/doc.go
index 4a9dfd7..f489bfd 100644
--- a/doc.go
+++ b/doc.go
@@ -14,7 +14,7 @@
 
 // Blueprint is a meta-build system that reads in Blueprints files that describe
 // modules that need to be built, and produces a Ninja
-// (https://ninja-build.org/) manifest describing the commands that need
+// (http://martine.github.io/ninja/) manifest describing the commands that need
 // to be run and their dependencies.  Where most build systems use built-in
 // rules or a domain-specific language to describe the logic how modules are
 // converted to build rules, Blueprint delegates this to per-project build logic
diff --git a/go.mod b/go.mod
index fe96d45..933cd12 100644
--- a/go.mod
+++ b/go.mod
@@ -1,3 +1 @@
 module github.com/google/blueprint
-
-go 1.13
diff --git a/module_ctx.go b/module_ctx.go
index be5d974..bdb14ad 100644
--- a/module_ctx.go
+++ b/module_ctx.go
@@ -121,39 +121,15 @@ type DynamicDependerModule interface {
 }
 
 type BaseModuleContext interface {
-	// Module returns the current module as a Module.  It should rarely be necessary, as the module already has a
-	// reference to itself.
-	Module() Module
-
-	// ModuleName returns the name of the module.  This is generally the value that was returned by Module.Name() when
-	// the module was created, but may have been modified by calls to BaseMutatorContext.Rename.
 	ModuleName() string
-
-	// ModuleDir returns the path to the directory that contains the defintion of the module.
 	ModuleDir() string
-
-	// ModuleType returns the name of the module type that was used to create the module, as specified in
-	// RegisterModuleType.
 	ModuleType() string
-
-	// Config returns the config object that was passed to Context.PrepareBuildActions.
 	Config() interface{}
 
-	// ContainsProperty returns true if the specified property name was set in the module definition.
 	ContainsProperty(name string) bool
-
-	// Errorf reports an error at the specified position of the module definition file.
 	Errorf(pos scanner.Position, fmt string, args ...interface{})
-
-	// ModuleErrorf reports an error at the line number of the module type in the module definition.
 	ModuleErrorf(fmt string, args ...interface{})
-
-	// PropertyErrorf reports an error at the line number of a property in the module definition.
 	PropertyErrorf(property, fmt string, args ...interface{})
-
-	// Failed returns true if any errors have been reported.  In most cases the module can continue with generating
-	// build rules after an error, allowing it to report additional errors in a single run, but in cases where the error
-	// has prevented the module from creating necessary data it can return early when Failed returns true.
 	Failed() bool
 
 	// GlobWithDeps returns a list of files and directories that match the
@@ -164,145 +140,46 @@ type BaseModuleContext interface {
 	// does not match the pattern is added to a searched directory.
 	GlobWithDeps(pattern string, excludes []string) ([]string, error)
 
-	// Fs returns a pathtools.Filesystem that can be used to interact with files.  Using the Filesystem interface allows
-	// the module to be used in build system tests that run against a mock filesystem.
 	Fs() pathtools.FileSystem
-
-	// AddNinjaFileDeps adds dependencies on the specified files to the rule that creates the ninja manifest.  The
-	// primary builder will be rerun whenever the specified files are modified.
 	AddNinjaFileDeps(deps ...string)
 
 	moduleInfo() *moduleInfo
 	error(err error)
 
-	// Namespace returns the Namespace object provided by the NameInterface set by Context.SetNameInterface, or the
-	// default SimpleNameInterface if Context.SetNameInterface was not called.
 	Namespace() Namespace
+}
 
-	// GetDirectDepWithTag returns the Module the direct dependency with the specified name, or nil if
-	// none exists.  It panics if the dependency does not have the specified tag.
-	GetDirectDepWithTag(name string, tag DependencyTag) Module
-
-	// GetDirectDep returns the Module and DependencyTag for the  direct dependency with the specified
-	// name, or nil if none exists.  If there are multiple dependencies on the same module it returns
-	// the first DependencyTag.
-	GetDirectDep(name string) (Module, DependencyTag)
-
-	// VisitDirectDeps calls visit for each direct dependency.  If there are multiple direct dependencies on the same
-	// module visit will be called multiple times on that module and OtherModuleDependencyTag will return a different
-	// tag for each.
-	//
-	// The Module passed to the visit function should not be retained outside of the visit function, it may be
-	// invalidated by future mutators.
-	VisitDirectDeps(visit func(Module))
-
-	// VisitDirectDepsIf calls pred for each direct dependency, and if pred returns true calls visit.  If there are
-	// multiple direct dependencies on the same module pred and visit will be called multiple times on that module and
-	// OtherModuleDependencyTag will return a different tag for each.
-	//
-	// The Module passed to the visit function should not be retained outside of the visit function, it may be
-	// invalidated by future mutators.
-	VisitDirectDepsIf(pred func(Module) bool, visit func(Module))
-
-	// VisitDepsDepthFirst calls visit for each transitive dependency, traversing the dependency tree in depth first
-	// order. visit will only be called once for any given module, even if there are multiple paths through the
-	// dependency tree to the module or multiple direct dependencies with different tags.  OtherModuleDependencyTag will
-	// return the tag for the first path found to the module.
-	//
-	// The Module passed to the visit function should not be retained outside of the visit function, it may be
-	// invalidated by future mutators.
-	VisitDepsDepthFirst(visit func(Module))
-
-	// VisitDepsDepthFirst calls pred for each transitive dependency, and if pred returns true calls visit, traversing
-	// the dependency tree in depth first order.  visit will only be called once for any given module, even if there are
-	// multiple paths through the dependency tree to the module or multiple direct dependencies with different tags.
-	// OtherModuleDependencyTag will return the tag for the first path found to the module.  The return value of pred
-	// does not affect which branches of the tree are traversed.
-	//
-	// The Module passed to the visit function should not be retained outside of the visit function, it may be
-	// invalidated by future mutators.
-	VisitDepsDepthFirstIf(pred func(Module) bool, visit func(Module))
+type DynamicDependerModuleContext BottomUpMutatorContext
 
-	// WalkDeps calls visit for each transitive dependency, traversing the dependency tree in top down order.  visit may
-	// be called multiple times for the same (child, parent) pair if there are multiple direct dependencies between the
-	// child and parent with different tags.  OtherModuleDependencyTag will return the tag for the currently visited
-	// (child, parent) pair.  If visit returns false WalkDeps will not continue recursing down to child.
-	//
-	// The Modules passed to the visit function should not be retained outside of the visit function, they may be
-	// invalidated by future mutators.
-	WalkDeps(visit func(Module, Module) bool)
+type ModuleContext interface {
+	BaseModuleContext
 
-	// OtherModuleName returns the name of another Module.  See BaseModuleContext.ModuleName for more information.
-	// It is intended for use inside the visit functions of Visit* and WalkDeps.
 	OtherModuleName(m Module) string
-
-	// OtherModuleDir returns the directory of another Module.  See BaseModuleContext.ModuleDir for more information.
-	// It is intended for use inside the visit functions of Visit* and WalkDeps.
 	OtherModuleDir(m Module) string
-
-	// OtherModuleSubDir returns the unique subdirectory name of another Module.  See ModuleContext.ModuleSubDir for
-	// more information.
-	// It is intended for use inside the visit functions of Visit* and WalkDeps.
 	OtherModuleSubDir(m Module) string
-
-	// OtherModuleType returns the type of another Module.  See BaseModuleContext.ModuleType for more information.
-	// It is intended for use inside the visit functions of Visit* and WalkDeps.
 	OtherModuleType(m Module) string
-
-	// OtherModuleErrorf reports an error on another Module.  See BaseModuleContext.ModuleErrorf for more information.
-	// It is intended for use inside the visit functions of Visit* and WalkDeps.
 	OtherModuleErrorf(m Module, fmt string, args ...interface{})
-
-	// OtherModuleDependencyTag returns the dependency tag used to depend on a module, or nil if there is no dependency
-	// on the module.  When called inside a Visit* method with current module being visited, and there are multiple
-	// dependencies on the module being visited, it returns the dependency tag used for the current dependency.
 	OtherModuleDependencyTag(m Module) DependencyTag
 
-	// OtherModuleExists returns true if a module with the specified name exists, as determined by the NameInterface
-	// passed to Context.SetNameInterface, or SimpleNameInterface if it was not called.
-	OtherModuleExists(name string) bool
-}
-
-type DynamicDependerModuleContext BottomUpMutatorContext
+	GetDirectDepWithTag(name string, tag DependencyTag) Module
+	GetDirectDep(name string) (Module, DependencyTag)
 
-type ModuleContext interface {
-	BaseModuleContext
+	VisitDirectDeps(visit func(Module))
+	VisitDirectDepsIf(pred func(Module) bool, visit func(Module))
+	VisitDepsDepthFirst(visit func(Module))
+	VisitDepsDepthFirstIf(pred func(Module) bool, visit func(Module))
+	WalkDeps(visit func(child, parent Module) bool)
 
-	// ModuleSubDir returns a unique name for the current variant of a module that can be used as part of the path
-	// to ensure that each variant of a module gets its own intermediates directory to write to.
 	ModuleSubDir() string
 
-	// Variable creates a new ninja variable scoped to the module.  It can be referenced by calls to Rule and Build
-	// in the same module.
 	Variable(pctx PackageContext, name, value string)
-
-	// Rule creates a new ninja rule scoped to the module.  It can be referenced by calls to Build in the same module.
 	Rule(pctx PackageContext, name string, params RuleParams, argNames ...string) Rule
-
-	// Build creates a new ninja build statement.
 	Build(pctx PackageContext, params BuildParams)
 
-	// PrimaryModule returns the first variant of the current module.  Variants of a module are always visited in
-	// order by mutators and GenerateBuildActions, so the data created by the current mutator can be read from the
-	// Module returned by PrimaryModule without data races.  This can be used to perform singleton actions that are
-	// only done once for all variants of a module.
 	PrimaryModule() Module
-
-	// FinalModule returns the last variant of the current module.  Variants of a module are always visited in
-	// order by mutators and GenerateBuildActions, so the data created by the current mutator can be read from all
-	// variants using VisitAllModuleVariants if the current module == FinalModule().  This can be used to perform
-	// singleton actions that are only done once for all variants of a module.
 	FinalModule() Module
-
-	// VisitAllModuleVariants calls visit for each variant of the current module.  Variants of a module are always
-	// visited in order by mutators and GenerateBuildActions, so the data created by the current mutator can be read
-	// from all variants if the current module == FinalModule().  Otherwise, care must be taken to not access any
-	// data modified by the current mutator.
 	VisitAllModuleVariants(visit func(Module))
 
-	// GetMissingDependencies returns the list of dependencies that were passed to AddDependencies or related methods,
-	// but do not exist.  It can be used with Context.SetAllowMissingDependencies to allow the primary builder to
-	// handle missing dependencies on its own instead of having Blueprint treat them as an error.
 	GetMissingDependencies() []string
 }
 
@@ -322,10 +199,6 @@ func (d *baseModuleContext) moduleInfo() *moduleInfo {
 	return d.module
 }
 
-func (d *baseModuleContext) Module() Module {
-	return d.module.logicModule
-}
-
 func (d *baseModuleContext) ModuleName() string {
 	return d.module.Name()
 }
@@ -454,6 +327,9 @@ func (m *baseModuleContext) OtherModuleErrorf(logicModule Module, format string,
 	})
 }
 
+// OtherModuleDependencyTag returns the dependency tag used to depend on a module, or nil if there is no dependency
+// on the module.  When called inside a Visit* method with current module being visited, and there are multiple
+// dependencies on the module being visited, it returns the dependency tag used for the current dependency.
 func (m *baseModuleContext) OtherModuleDependencyTag(logicModule Module) DependencyTag {
 	// fast path for calling OtherModuleDependencyTag from inside VisitDirectDeps
 	if logicModule == m.visitingDep.module.logicModule {
@@ -469,11 +345,9 @@ func (m *baseModuleContext) OtherModuleDependencyTag(logicModule Module) Depende
 	return nil
 }
 
-func (m *baseModuleContext) OtherModuleExists(name string) bool {
-	_, exists := m.context.nameInterface.ModuleFromName(name, m.module.namespace())
-	return exists
-}
-
+// GetDirectDep returns the Module and DependencyTag for the  direct dependency with the specified
+// name, or nil if none exists.  If there are multiple dependencies on the same module it returns
+// the first DependencyTag.
 func (m *baseModuleContext) GetDirectDep(name string) (Module, DependencyTag) {
 	for _, dep := range m.module.directDeps {
 		if dep.module.Name() == name {
@@ -484,6 +358,8 @@ func (m *baseModuleContext) GetDirectDep(name string) (Module, DependencyTag) {
 	return nil, nil
 }
 
+// GetDirectDepWithTag returns the Module the direct dependency with the specified name, or nil if
+// none exists.  It panics if the dependency does not have the specified tag.
 func (m *baseModuleContext) GetDirectDepWithTag(name string, tag DependencyTag) Module {
 	var deps []depInfo
 	for _, dep := range m.module.directDeps {
@@ -502,6 +378,8 @@ func (m *baseModuleContext) GetDirectDepWithTag(name string, tag DependencyTag)
 	return nil
 }
 
+// VisitDirectDeps calls visit for each direct dependency.  If there are multiple direct dependencies on the same module
+// visit will be called multiple times on that module and OtherModuleDependencyTag will return a different tag for each.
 func (m *baseModuleContext) VisitDirectDeps(visit func(Module)) {
 	defer func() {
 		if r := recover(); r != nil {
@@ -521,6 +399,9 @@ func (m *baseModuleContext) VisitDirectDeps(visit func(Module)) {
 	m.visitingDep = depInfo{}
 }
 
+// VisitDirectDepsIf calls pred for each direct dependency, and if pred returns true calls visit.  If there are multiple
+// direct dependencies on the same module pred and visit will be called multiple times on that module and
+// OtherModuleDependencyTag will return a different tag for each.
 func (m *baseModuleContext) VisitDirectDepsIf(pred func(Module) bool, visit func(Module)) {
 	defer func() {
 		if r := recover(); r != nil {
@@ -542,6 +423,10 @@ func (m *baseModuleContext) VisitDirectDepsIf(pred func(Module) bool, visit func
 	m.visitingDep = depInfo{}
 }
 
+// VisitDepsDepthFirst calls visit for each transitive dependency, traversing the dependency tree in depth first order.
+// visit will only be called once for any given module, even if there are multiple paths through the dependency tree
+// to the module or multiple direct dependencies with different tags.  OtherModuleDependencyTag will return the tag for
+// the first path found to the module.
 func (m *baseModuleContext) VisitDepsDepthFirst(visit func(Module)) {
 	defer func() {
 		if r := recover(); r != nil {
@@ -560,6 +445,11 @@ func (m *baseModuleContext) VisitDepsDepthFirst(visit func(Module)) {
 	m.visitingDep = depInfo{}
 }
 
+// VisitDepsDepthFirst calls pred for each transitive dependency, and if pred returns true calls visit, traversing the
+// dependency tree in depth first order.  visit will only be called once for any given module, even if there are
+// multiple paths through the dependency tree to the module or multiple direct dependencies with different tags.
+// OtherModuleDependencyTag will return the tag for the first path found to the module.  The return value of pred does
+// not affect which branches of the tree are traversed.
 func (m *baseModuleContext) VisitDepsDepthFirstIf(pred func(Module) bool,
 	visit func(Module)) {
 
@@ -582,6 +472,10 @@ func (m *baseModuleContext) VisitDepsDepthFirstIf(pred func(Module) bool,
 	m.visitingDep = depInfo{}
 }
 
+// WalkDeps calls visit for each transitive dependency, traversing the dependency tree in top down order.  visit may be
+// called multiple times for the same (child, parent) pair if there are multiple direct dependencies between the
+// child and parent with different tags.  OtherModuleDependencyTag will return the tag for the currently visited
+// (child, parent) pair.  If visit returns false WalkDeps will not continue recursing down to child.
 func (m *baseModuleContext) WalkDeps(visit func(child, parent Module) bool) {
 	m.context.walkDeps(m.module, true, func(dep depInfo, parent *moduleInfo) bool {
 		m.visitingParent = parent
@@ -670,86 +564,49 @@ type mutatorContext struct {
 	defaultVariation *string
 }
 
-type BaseMutatorContext interface {
+type baseMutatorContext interface {
 	BaseModuleContext
 
-	// Rename all variants of a module.  The new name is not visible to calls to ModuleName,
-	// AddDependency or OtherModuleName until after this mutator pass is complete.
+	OtherModuleExists(name string) bool
 	Rename(name string)
-
-	// MutatorName returns the name that this mutator was registered with.
-	MutatorName() string
+	Module() Module
 }
 
 type EarlyMutatorContext interface {
-	BaseMutatorContext
+	baseMutatorContext
 
-	// CreateVariations splits  a module into mulitple variants, one for each name in the variationNames
-	// parameter.  It returns a list of new modules in the same order as the variationNames
-	// list.
-	//
-	// If any of the dependencies of the module being operated on were already split
-	// by calling CreateVariations with the same name, the dependency will automatically
-	// be updated to point the matching variant.
-	//
-	// If a module is split, and then a module depending on the first module is not split
-	// when the Mutator is later called on it, the dependency of the depending module will
-	// automatically be updated to point to the first variant.
 	CreateVariations(...string) []Module
-
-	// CreateLocationVariations splits a module into mulitple variants, one for each name in the variantNames
-	// parameter.  It returns a list of new modules in the same order as the variantNames
-	// list.
-	//
-	// Local variations do not affect automatic dependency resolution - dependencies added
-	// to the split module via deps or DynamicDependerModule must exactly match a variant
-	// that contains all the non-local variations.
 	CreateLocalVariations(...string) []Module
 }
 
 type TopDownMutatorContext interface {
-	BaseMutatorContext
+	baseMutatorContext
+
+	OtherModuleName(m Module) string
+	OtherModuleDir(m Module) string
+	OtherModuleSubDir(m Module) string
+	OtherModuleType(m Module) string
+	OtherModuleErrorf(m Module, fmt string, args ...interface{})
+	OtherModuleDependencyTag(m Module) DependencyTag
+
+	CreateModule(ModuleFactory, ...interface{})
+
+	GetDirectDepWithTag(name string, tag DependencyTag) Module
+	GetDirectDep(name string) (Module, DependencyTag)
 
-	// CreateModule creates a new module by calling the factory method for the specified moduleType, and applies
-	// the specified property structs to it as if the properties were set in a blueprint file.
-	CreateModule(ModuleFactory, ...interface{}) Module
+	VisitDirectDeps(visit func(Module))
+	VisitDirectDepsIf(pred func(Module) bool, visit func(Module))
+	VisitDepsDepthFirst(visit func(Module))
+	VisitDepsDepthFirstIf(pred func(Module) bool, visit func(Module))
+	WalkDeps(visit func(Module, Module) bool)
 }
 
 type BottomUpMutatorContext interface {
-	BaseMutatorContext
+	baseMutatorContext
 
-	// AddDependency adds a dependency to the given module.
-	// Does not affect the ordering of the current mutator pass, but will be ordered
-	// correctly for all future mutator passes.
 	AddDependency(module Module, tag DependencyTag, name ...string)
-
-	// AddReverseDependency adds a dependency from the destination to the given module.
-	// Does not affect the ordering of the current mutator pass, but will be ordered
-	// correctly for all future mutator passes.  All reverse dependencies for a destination module are
-	// collected until the end of the mutator pass, sorted by name, and then appended to the destination
-	// module's dependency list.
 	AddReverseDependency(module Module, tag DependencyTag, name string)
-
-	// CreateVariations splits  a module into mulitple variants, one for each name in the variationNames
-	// parameter.  It returns a list of new modules in the same order as the variationNames
-	// list.
-	//
-	// If any of the dependencies of the module being operated on were already split
-	// by calling CreateVariations with the same name, the dependency will automatically
-	// be updated to point the matching variant.
-	//
-	// If a module is split, and then a module depending on the first module is not split
-	// when the Mutator is later called on it, the dependency of the depending module will
-	// automatically be updated to point to the first variant.
 	CreateVariations(...string) []Module
-
-	// CreateLocationVariations splits a module into mulitple variants, one for each name in the variantNames
-	// parameter.  It returns a list of new modules in the same order as the variantNames
-	// list.
-	//
-	// Local variations do not affect automatic dependency resolution - dependencies added
-	// to the split module via deps or DynamicDependerModule must exactly match a variant
-	// that contains all the non-local variations.
 	CreateLocalVariations(...string) []Module
 
 	// SetDependencyVariation sets all dangling dependencies on the current module to point to the variation
@@ -760,30 +617,9 @@ type BottomUpMutatorContext interface {
 	// during the subsequent calls on Create*Variations* functions. To reset, set it to nil.
 	SetDefaultDependencyVariation(*string)
 
-	// AddVariationDependencies adds deps as dependencies of the current module, but uses the variations
-	// argument to select which variant of the dependency to use.  A variant of the dependency must
-	// exist that matches the all of the non-local variations of the current module, plus the variations
-	// argument.
 	AddVariationDependencies([]Variation, DependencyTag, ...string)
-
-	// AddFarVariationDependencies adds deps as dependencies of the current module, but uses the
-	// variations argument to select which variant of the dependency to use.  A variant of the
-	// dependency must exist that matches the variations argument, but may also have other variations.
-	// For any unspecified variation the first variant will be used.
-	//
-	// Unlike AddVariationDependencies, the variations of the current module are ignored - the
-	// dependency only needs to match the supplied variations.
 	AddFarVariationDependencies([]Variation, DependencyTag, ...string)
-
-	// AddInterVariantDependency adds a dependency between two variants of the same module.  Variants are always
-	// ordered in the same orderas they were listed in CreateVariations, and AddInterVariantDependency does not change
-	// that ordering, but it associates a DependencyTag with the dependency and makes it visible to VisitDirectDeps,
-	// WalkDeps, etc.
 	AddInterVariantDependency(tag DependencyTag, from, to Module)
-
-	// ReplaceDependencies replaces all dependencies on the identical variant of the module with the
-	// specified name with the current variant of this module.  Replacements don't take effect until
-	// after the mutator pass is finished.
 	ReplaceDependencies(string)
 }
 
@@ -816,14 +652,28 @@ func (BaseDependencyTag) dependencyTag(DependencyTag) {
 
 var _ DependencyTag = BaseDependencyTag{}
 
-func (mctx *mutatorContext) MutatorName() string {
-	return mctx.name
-}
-
+// Split a module into mulitple variants, one for each name in the variationNames
+// parameter.  It returns a list of new modules in the same order as the variationNames
+// list.
+//
+// If any of the dependencies of the module being operated on were already split
+// by calling CreateVariations with the same name, the dependency will automatically
+// be updated to point the matching variant.
+//
+// If a module is split, and then a module depending on the first module is not split
+// when the Mutator is later called on it, the dependency of the depending module will
+// automatically be updated to point to the first variant.
 func (mctx *mutatorContext) CreateVariations(variationNames ...string) []Module {
 	return mctx.createVariations(variationNames, false)
 }
 
+// Split a module into mulitple variants, one for each name in the variantNames
+// parameter.  It returns a list of new modules in the same order as the variantNames
+// list.
+//
+// Local variations do not affect automatic dependency resolution - dependencies added
+// to the split module via deps or DynamicDependerModule must exactly match a variant
+// that contains all the non-local variations.
 func (mctx *mutatorContext) CreateLocalVariations(variationNames ...string) []Module {
 	return mctx.createVariations(variationNames, true)
 }
@@ -854,6 +704,8 @@ func (mctx *mutatorContext) createVariations(variationNames []string, local bool
 	return ret
 }
 
+// Set all dangling dependencies on the current module to point to the variation
+// with given name.
 func (mctx *mutatorContext) SetDependencyVariation(variationName string) {
 	mctx.context.convertDepsToVariation(mctx.module, mctx.name, variationName, nil)
 }
@@ -866,6 +718,9 @@ func (mctx *mutatorContext) Module() Module {
 	return mctx.module.logicModule
 }
 
+// Add a dependency to the given module.
+// Does not affect the ordering of the current mutator pass, but will be ordered
+// correctly for all future mutator passes.
 func (mctx *mutatorContext) AddDependency(module Module, tag DependencyTag, deps ...string) {
 	for _, dep := range deps {
 		modInfo := mctx.context.moduleInfo[module]
@@ -876,6 +731,11 @@ func (mctx *mutatorContext) AddDependency(module Module, tag DependencyTag, deps
 	}
 }
 
+// Add a dependency from the destination to the given module.
+// Does not affect the ordering of the current mutator pass, but will be ordered
+// correctly for all future mutator passes.  All reverse dependencies for a destination module are
+// collected until the end of the mutator pass, sorted by name, and then appended to the destination
+// module's dependency list.
 func (mctx *mutatorContext) AddReverseDependency(module Module, tag DependencyTag, destName string) {
 	if _, ok := tag.(BaseDependencyTag); ok {
 		panic("BaseDependencyTag is not allowed to be used directly!")
@@ -893,6 +753,10 @@ func (mctx *mutatorContext) AddReverseDependency(module Module, tag DependencyTa
 	})
 }
 
+// AddVariationDependencies adds deps as dependencies of the current module, but uses the variations
+// argument to select which variant of the dependency to use.  A variant of the dependency must
+// exist that matches the all of the non-local variations of the current module, plus the variations
+// argument.
 func (mctx *mutatorContext) AddVariationDependencies(variations []Variation, tag DependencyTag,
 	deps ...string) {
 
@@ -904,6 +768,13 @@ func (mctx *mutatorContext) AddVariationDependencies(variations []Variation, tag
 	}
 }
 
+// AddFarVariationDependencies adds deps as dependencies of the current module, but uses the
+// variations argument to select which variant of the dependency to use.  A variant of the
+// dependency must exist that matches the variations argument, but may also have other variations.
+// For any unspecified variation the first variant will be used.
+//
+// Unlike AddVariationDependencies, the variations of the current module are ignored - the
+// depdendency only needs to match the supplied variations.
 func (mctx *mutatorContext) AddFarVariationDependencies(variations []Variation, tag DependencyTag,
 	deps ...string) {
 
@@ -919,6 +790,9 @@ func (mctx *mutatorContext) AddInterVariantDependency(tag DependencyTag, from, t
 	mctx.context.addInterVariantDependency(mctx.module, tag, from, to)
 }
 
+// ReplaceDependencies replaces all dependencies on the identical variant of the module with the
+// specified name with the current variant of this module.  Replacements don't take effect until
+// after the mutator pass is finished.
 func (mctx *mutatorContext) ReplaceDependencies(name string) {
 	target := mctx.context.moduleMatchingVariant(mctx.module, name)
 
@@ -930,17 +804,24 @@ func (mctx *mutatorContext) ReplaceDependencies(name string) {
 	mctx.replace = append(mctx.replace, replace{target, mctx.module})
 }
 
+func (mctx *mutatorContext) OtherModuleExists(name string) bool {
+	_, exists := mctx.context.nameInterface.ModuleFromName(name, mctx.module.namespace())
+	return exists
+}
+
+// Rename all variants of a module.  The new name is not visible to calls to ModuleName,
+// AddDependency or OtherModuleName until after this mutator pass is complete.
 func (mctx *mutatorContext) Rename(name string) {
 	mctx.rename = append(mctx.rename, rename{mctx.module.group, name})
 }
 
-func (mctx *mutatorContext) CreateModule(factory ModuleFactory, props ...interface{}) Module {
+// Create a new module by calling the factory method for the specified moduleType, and apply
+// the specified property structs to it as if the properties were set in a blueprint file.
+func (mctx *mutatorContext) CreateModule(factory ModuleFactory, props ...interface{}) {
 	module := mctx.context.newModule(factory)
 
 	module.relBlueprintsFile = mctx.module.relBlueprintsFile
 	module.pos = mctx.module.pos
-	module.propertyPos = mctx.module.propertyPos
-	module.createdBy = mctx.module
 
 	for _, p := range props {
 		err := proptools.AppendMatchingProperties(module.properties, p, nil)
@@ -950,8 +831,6 @@ func (mctx *mutatorContext) CreateModule(factory ModuleFactory, props ...interfa
 	}
 
 	mctx.newModules = append(mctx.newModules, module)
-
-	return module.logicModule
 }
 
 // SimpleName is an embeddable object to implement the ModuleContext.Name method using a property
diff --git a/ninja_strings.go b/ninja_strings.go
index 5b8767d..a13205f 100644
--- a/ninja_strings.go
+++ b/ninja_strings.go
@@ -277,20 +277,12 @@ func (n *ninjaString) Value(pkgNames map[*packageContext]string) string {
 func (n *ninjaString) ValueWithEscaper(pkgNames map[*packageContext]string,
 	escaper *strings.Replacer) string {
 
-	if len(n.strings) == 1 {
-		return escaper.Replace(n.strings[0])
-	}
-
-	str := strings.Builder{}
-	str.WriteString(escaper.Replace(n.strings[0]))
+	str := escaper.Replace(n.strings[0])
 	for i, v := range n.variables {
-		str.WriteString("${")
-		str.WriteString(v.fullName(pkgNames))
-		str.WriteString("}")
-		str.WriteString(escaper.Replace(n.strings[i+1]))
+		str += "${" + v.fullName(pkgNames) + "}"
+		str += escaper.Replace(n.strings[i+1])
 	}
-
-	return str.String()
+	return str
 }
 
 func (n *ninjaString) Eval(variables map[Variable]*ninjaString) (string, error) {
diff --git a/ninja_strings_test.go b/ninja_strings_test.go
index 0e0de64..6227ea6 100644
--- a/ninja_strings_test.go
+++ b/ninja_strings_test.go
@@ -16,8 +16,6 @@ package blueprint
 
 import (
 	"reflect"
-	"strconv"
-	"strings"
 	"testing"
 )
 
@@ -163,45 +161,3 @@ func TestParseNinjaStringWithImportedVar(t *testing.T) {
 		t.Errorf("       got: %#v", output)
 	}
 }
-
-func BenchmarkNinjaString_Value(b *testing.B) {
-	b.Run("constant", func(b *testing.B) {
-		for _, l := range []int{1, 10, 100, 1000} {
-			ns := simpleNinjaString(strings.Repeat("a", l))
-			b.Run(strconv.Itoa(l), func(b *testing.B) {
-				for n := 0; n < b.N; n++ {
-					ns.Value(nil)
-				}
-			})
-		}
-	})
-	b.Run("variable", func(b *testing.B) {
-		for _, l := range []int{1, 10, 100, 1000} {
-			scope := newLocalScope(nil, "")
-			scope.AddLocalVariable("a", strings.Repeat("b", l/3))
-			ns, _ := parseNinjaString(scope, strings.Repeat("a", l/3)+"${a}"+strings.Repeat("a", l/3))
-			b.Run(strconv.Itoa(l), func(b *testing.B) {
-				for n := 0; n < b.N; n++ {
-					ns.Value(nil)
-				}
-			})
-		}
-	})
-	b.Run("variables", func(b *testing.B) {
-		for _, l := range []int{1, 2, 3, 4, 5, 10, 100, 1000} {
-			scope := newLocalScope(nil, "")
-			str := strings.Repeat("a", 10)
-			for i := 0; i < l; i++ {
-				scope.AddLocalVariable("a"+strconv.Itoa(i), strings.Repeat("b", 10))
-				str += "${a" + strconv.Itoa(i) + "}"
-			}
-			ns, _ := parseNinjaString(scope, str)
-			b.Run(strconv.Itoa(l), func(b *testing.B) {
-				for n := 0; n < b.N; n++ {
-					ns.Value(nil)
-				}
-			})
-		}
-	})
-
-}
diff --git a/package_ctx.go b/package_ctx.go
index c55152a..e0a03c8 100644
--- a/package_ctx.go
+++ b/package_ctx.go
@@ -158,9 +158,8 @@ func callerName(skip int) (pkgPath, funcName string, ok bool) {
 	if n != 1 {
 		return "", "", false
 	}
-	frames := runtime.CallersFrames(pc[:])
-	frame, _ := frames.Next()
-	f := frame.Function
+
+	f := runtime.FuncForPC(pc[0]).Name()
 	s := pkgPathRe.FindStringSubmatch(f)
 	if len(s) < 3 {
 		panic(fmt.Errorf("failed to extract package path and function name from %q", f))
diff --git a/pathtools/fs.go b/pathtools/fs.go
index 8329392..4217487 100644
--- a/pathtools/fs.go
+++ b/pathtools/fs.go
@@ -61,9 +61,6 @@ func MockFs(files map[string][]byte) FileSystem {
 		fs.dirs[dir] = true
 	}
 
-	fs.dirs["."] = true
-	fs.dirs["/"] = true
-
 	for f := range fs.files {
 		fs.all = append(fs.all, f)
 	}
@@ -333,8 +330,8 @@ func (m *mockFs) glob(pattern string) ([]string, error) {
 			if err != nil {
 				return nil, err
 			}
-			if (f == "." || f == "/") && f != pattern {
-				// filepath.Glob won't return "." or "/" unless the pattern was "." or "/"
+			if f == "." && f != pattern {
+				// filepath.Glob won't return "." unless the pattern was "."
 				match = false
 			}
 			if match {
diff --git a/pathtools/glob.go b/pathtools/glob.go
index 727b725..67394d2 100644
--- a/pathtools/glob.go
+++ b/pathtools/glob.go
@@ -25,9 +25,8 @@ import (
 	"github.com/google/blueprint/deptools"
 )
 
-var GlobMultipleRecursiveErr = errors.New("pattern contains multiple '**'")
-var GlobLastRecursiveErr = errors.New("pattern has '**' as last path element")
-var GlobInvalidRecursiveErr = errors.New("pattern contains other characters between '**' and path separator")
+var GlobMultipleRecursiveErr = errors.New("pattern contains multiple **")
+var GlobLastRecursiveErr = errors.New("pattern ** as last path element")
 
 // Glob returns the list of files and directories that match the given pattern
 // but do not match the given exclude patterns, along with the list of
@@ -119,7 +118,7 @@ func glob(fs FileSystem, pattern string, hasRecursive bool,
 			// as a dependency.
 			var matchDirs []string
 			for len(matchDirs) == 0 {
-				pattern = filepath.Dir(pattern)
+				pattern, _ = saneSplit(pattern)
 				matchDirs, err = fs.glob(pattern)
 				if err != nil {
 					return matches, dirs, err
@@ -137,8 +136,6 @@ func glob(fs FileSystem, pattern string, hasRecursive bool,
 			return matches, dirs, GlobMultipleRecursiveErr
 		}
 		hasRecursive = true
-	} else if strings.Contains(file, "**") {
-		return matches, dirs, GlobInvalidRecursiveErr
 	}
 
 	dirMatches, dirs, err := glob(fs, dir, hasRecursive, follow)
@@ -245,7 +242,7 @@ func filterDotFiles(matches []string) []string {
 }
 
 // Match returns true if name matches pattern using the same rules as filepath.Match, but supporting
-// recursive globs (**).
+// hierarchical patterns (a/*) and recursive globs (**).
 func Match(pattern, name string) (bool, error) {
 	if filepath.Base(pattern) == "**" {
 		return false, GlobLastRecursiveErr
@@ -265,35 +262,16 @@ func Match(pattern, name string) (bool, error) {
 
 	for {
 		var patternFile, nameFile string
-		pattern, patternFile = filepath.Dir(pattern), filepath.Base(pattern)
+		pattern, patternFile = saneSplit(pattern)
+		name, nameFile = saneSplit(name)
 
 		if patternFile == "**" {
-			if strings.Contains(pattern, "**") {
-				return false, GlobMultipleRecursiveErr
-			}
-			// Test if the any prefix of name matches the part of the pattern before **
-			for {
-				if name == "." || name == "/" {
-					return name == pattern, nil
-				}
-				if match, err := filepath.Match(pattern, name); err != nil {
-					return false, err
-				} else if match {
-					return true, nil
-				}
-				name = filepath.Dir(name)
-			}
-		} else if strings.Contains(patternFile, "**") {
-			return false, GlobInvalidRecursiveErr
+			return matchPrefix(pattern, filepath.Join(name, nameFile))
 		}
 
-		name, nameFile = filepath.Dir(name), filepath.Base(name)
-
-		if nameFile == "." && patternFile == "." {
-			return true, nil
-		} else if nameFile == "/" && patternFile == "/" {
+		if nameFile == "" && patternFile == "" {
 			return true, nil
-		} else if nameFile == "." || patternFile == "." || nameFile == "/" || patternFile == "/" {
+		} else if nameFile == "" || patternFile == "" {
 			return false, nil
 		}
 
@@ -304,6 +282,56 @@ func Match(pattern, name string) (bool, error) {
 	}
 }
 
+// matchPrefix returns true if the beginning of name matches pattern using the same rules as
+// filepath.Match, but supporting hierarchical patterns (a/*).  Recursive globs (**) are not
+// supported, they should have been handled in Match().
+func matchPrefix(pattern, name string) (bool, error) {
+	if len(pattern) > 0 && pattern[0] == '/' {
+		if len(name) > 0 && name[0] == '/' {
+			pattern = pattern[1:]
+			name = name[1:]
+		} else {
+			return false, nil
+		}
+	}
+
+	for {
+		var patternElem, nameElem string
+		patternElem, pattern = saneSplitFirst(pattern)
+		nameElem, name = saneSplitFirst(name)
+
+		if patternElem == "." {
+			patternElem = ""
+		}
+		if nameElem == "." {
+			nameElem = ""
+		}
+
+		if patternElem == "**" {
+			return false, GlobMultipleRecursiveErr
+		}
+
+		if patternElem == "" {
+			return true, nil
+		} else if nameElem == "" {
+			return false, nil
+		}
+
+		match, err := filepath.Match(patternElem, nameElem)
+		if err != nil || !match {
+			return match, err
+		}
+	}
+}
+
+func saneSplitFirst(path string) (string, string) {
+	i := strings.IndexRune(path, filepath.Separator)
+	if i < 0 {
+		return path, ""
+	}
+	return path[:i], path[i+1:]
+}
+
 func GlobPatternList(patterns []string, prefix string) (globedList []string, depDirs []string, err error) {
 	var (
 		matches []string
diff --git a/pathtools/glob_test.go b/pathtools/glob_test.go
index a3a36ff..0265df6 100644
--- a/pathtools/glob_test.go
+++ b/pathtools/glob_test.go
@@ -18,7 +18,6 @@ import (
 	"os"
 	"path/filepath"
 	"reflect"
-	"strings"
 	"testing"
 )
 
@@ -216,14 +215,6 @@ var globTestCases = []globTestCase{
 		pattern: "**/**",
 		err:     GlobLastRecursiveErr,
 	},
-	{
-		pattern: "a**/",
-		err:     GlobInvalidRecursiveErr,
-	},
-	{
-		pattern: "**a/",
-		err:     GlobInvalidRecursiveErr,
-	},
 
 	// exclude tests
 	{
@@ -787,16 +778,6 @@ func TestMatch(t *testing.T) {
 		{"a/**/*/", "a/b/", true},
 		{"a/**/*/", "a/b/c", false},
 
-		{"**/*", "a/", false},
-		{"**/*", "a/a", true},
-		{"**/*", "a/b/", false},
-		{"**/*", "a/b/c", true},
-
-		{"**/*/", "a/", true},
-		{"**/*/", "a/a", false},
-		{"**/*/", "a/b/", true},
-		{"**/*/", "a/b/c", false},
-
 		{`a/\*\*/\*`, `a/**/*`, true},
 		{`a/\*\*/\*`, `a/a/*`, false},
 		{`a/\*\*/\*`, `a/**/a`, false},
@@ -845,38 +826,6 @@ func TestMatch(t *testing.T) {
 		{`a/?`, `a/a`, true},
 		{`a/\?`, `a/?`, true},
 		{`a/\?`, `a/a`, false},
-
-		{"/a/*", "/a/", false},
-		{"/a/*", "/a/a", true},
-		{"/a/*", "/a/b/", false},
-		{"/a/*", "/a/b/c", false},
-
-		{"/a/*/", "/a/", false},
-		{"/a/*/", "/a/a", false},
-		{"/a/*/", "/a/b/", true},
-		{"/a/*/", "/a/b/c", false},
-
-		{"/a/**/*", "/a/", false},
-		{"/a/**/*", "/a/a", true},
-		{"/a/**/*", "/a/b/", false},
-		{"/a/**/*", "/a/b/c", true},
-
-		{"/**/*", "/a/", false},
-		{"/**/*", "/a/a", true},
-		{"/**/*", "/a/b/", false},
-		{"/**/*", "/a/b/c", true},
-
-		{"/**/*/", "/a/", true},
-		{"/**/*/", "/a/a", false},
-		{"/**/*/", "/a/b/", true},
-		{"/**/*/", "/a/b/c", false},
-
-		{`a`, `/a`, false},
-		{`/a`, `a`, false},
-		{`*`, `/a`, false},
-		{`/*`, `a`, false},
-		{`**/*`, `/a`, false},
-		{`/**/*`, `a`, false},
 	}
 
 	for _, test := range testCases {
@@ -890,36 +839,4 @@ func TestMatch(t *testing.T) {
 			}
 		})
 	}
-
-	// Run the same test cases through Glob
-	for _, test := range testCases {
-		// Glob and Match disagree on matching directories
-		if strings.HasSuffix(test.name, "/") || strings.HasSuffix(test.pattern, "/") {
-			continue
-		}
-		t.Run("glob:"+test.pattern+","+test.name, func(t *testing.T) {
-			mockFiles := map[string][]byte{
-				test.name: nil,
-			}
-
-			mock := MockFs(mockFiles)
-
-			matches, _, err := mock.Glob(test.pattern, nil, DontFollowSymlinks)
-			t.Log(test.name, test.pattern, matches)
-			if err != nil {
-				t.Fatal(err)
-			}
-
-			match := false
-			for _, x := range matches {
-				if x == test.name {
-					match = true
-				}
-			}
-
-			if match != test.match {
-				t.Errorf("want: %v, got %v", test.match, match)
-			}
-		})
-	}
 }
diff --git a/proptools/extend.go b/proptools/extend.go
index b1a35d0..1f323cf 100644
--- a/proptools/extend.go
+++ b/proptools/extend.go
@@ -34,7 +34,7 @@ import (
 // embedded structs, pointers to structs, and interfaces containing
 // pointers to structs.  Appending the zero value of a property will always be a no-op.
 func AppendProperties(dst interface{}, src interface{}, filter ExtendPropertyFilterFunc) error {
-	return extendProperties(dst, src, filter, OrderAppend)
+	return extendProperties(dst, src, filter, orderAppend)
 }
 
 // PrependProperties prepends the values of properties in the property struct src to the property
@@ -52,7 +52,7 @@ func AppendProperties(dst interface{}, src interface{}, filter ExtendPropertyFil
 // embedded structs, pointers to structs, and interfaces containing
 // pointers to structs.  Prepending the zero value of a property will always be a no-op.
 func PrependProperties(dst interface{}, src interface{}, filter ExtendPropertyFilterFunc) error {
-	return extendProperties(dst, src, filter, OrderPrepend)
+	return extendProperties(dst, src, filter, orderPrepend)
 }
 
 // AppendMatchingProperties appends the values of properties in the property struct src to the
@@ -68,12 +68,12 @@ func PrependProperties(dst interface{}, src interface{}, filter ExtendPropertyFi
 // *ExtendPropertyError, and can have the property name and error extracted from it.
 //
 // The append operation is defined as appending strings, and slices of strings normally, OR-ing bool
-// values, replacing pointers to booleans or strings whether they are nil or not, and recursing into
+// values, replacing non-nil pointers to booleans or strings, and recursing into
 // embedded structs, pointers to structs, and interfaces containing
 // pointers to structs.  Appending the zero value of a property will always be a no-op.
 func AppendMatchingProperties(dst []interface{}, src interface{},
 	filter ExtendPropertyFilterFunc) error {
-	return extendMatchingProperties(dst, src, filter, OrderAppend)
+	return extendMatchingProperties(dst, src, filter, orderAppend)
 }
 
 // PrependMatchingProperties prepends the values of properties in the property struct src to the
@@ -89,12 +89,12 @@ func AppendMatchingProperties(dst []interface{}, src interface{},
 // *ExtendPropertyError, and can have the property name and error extracted from it.
 //
 // The prepend operation is defined as prepending strings, and slices of strings normally, OR-ing
-// bool values, replacing nil pointers to booleans or strings, and recursing into
+// bool values, replacing non-nil pointers to booleans or strings, and recursing into
 // embedded structs, pointers to structs, and interfaces containing
 // pointers to structs.  Prepending the zero value of a property will always be a no-op.
 func PrependMatchingProperties(dst []interface{}, src interface{},
 	filter ExtendPropertyFilterFunc) error {
-	return extendMatchingProperties(dst, src, filter, OrderPrepend)
+	return extendMatchingProperties(dst, src, filter, orderPrepend)
 }
 
 // ExtendProperties appends or prepends the values of properties in the property struct src to the
@@ -160,13 +160,13 @@ type ExtendPropertyOrderFunc func(property string,
 	dstField, srcField reflect.StructField,
 	dstValue, srcValue interface{}) (Order, error)
 
-func OrderAppend(property string,
+func orderAppend(property string,
 	dstField, srcField reflect.StructField,
 	dstValue, srcValue interface{}) (Order, error) {
 	return Append, nil
 }
 
-func OrderPrepend(property string,
+func orderPrepend(property string,
 	dstField, srcField reflect.StructField,
 	dstValue, srcValue interface{}) (Order, error) {
 	return Prepend, nil
diff --git a/proptools/filter.go b/proptools/filter.go
deleted file mode 100644
index 7a61b02..0000000
--- a/proptools/filter.go
+++ /dev/null
@@ -1,159 +0,0 @@
-// Copyright 2019 Google Inc. All rights reserved.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//     http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package proptools
-
-import (
-	"reflect"
-)
-
-type FilterFieldPredicate func(field reflect.StructField, string string) (bool, reflect.StructField)
-
-func filterPropertyStructFields(fields []reflect.StructField, prefix string, predicate FilterFieldPredicate) (filteredFields []reflect.StructField, filtered bool) {
-	for _, field := range fields {
-		var keep bool
-		if keep, field = predicate(field, prefix); !keep {
-			filtered = true
-			continue
-		}
-
-		subPrefix := field.Name
-		if prefix != "" {
-			subPrefix = prefix + "." + subPrefix
-		}
-
-		// Recurse into structs
-		switch field.Type.Kind() {
-		case reflect.Struct:
-			var subFiltered bool
-			field.Type, subFiltered = filterPropertyStruct(field.Type, subPrefix, predicate)
-			filtered = filtered || subFiltered
-			if field.Type == nil {
-				continue
-			}
-		case reflect.Ptr:
-			if field.Type.Elem().Kind() == reflect.Struct {
-				nestedType, subFiltered := filterPropertyStruct(field.Type.Elem(), subPrefix, predicate)
-				filtered = filtered || subFiltered
-				if nestedType == nil {
-					continue
-				}
-				field.Type = reflect.PtrTo(nestedType)
-			}
-		case reflect.Interface:
-			panic("Interfaces are not supported in filtered property structs")
-		}
-
-		filteredFields = append(filteredFields, field)
-	}
-
-	return filteredFields, filtered
-}
-
-// FilterPropertyStruct takes a reflect.Type that is either a struct or a pointer to a struct, and returns a
-// reflect.Type that only contains the fields in the original type for which predicate returns true, and a bool
-// that is true if the new struct type has fewer fields than the original type.  If there are no fields in the
-// original type for which predicate returns true it returns nil and true.
-func FilterPropertyStruct(prop reflect.Type, predicate FilterFieldPredicate) (filteredProp reflect.Type, filtered bool) {
-	return filterPropertyStruct(prop, "", predicate)
-}
-
-func filterPropertyStruct(prop reflect.Type, prefix string, predicate FilterFieldPredicate) (filteredProp reflect.Type, filtered bool) {
-	var fields []reflect.StructField
-
-	ptr := prop.Kind() == reflect.Ptr
-	if ptr {
-		prop = prop.Elem()
-	}
-
-	for i := 0; i < prop.NumField(); i++ {
-		fields = append(fields, prop.Field(i))
-	}
-
-	filteredFields, filtered := filterPropertyStructFields(fields, prefix, predicate)
-
-	if len(filteredFields) == 0 {
-		return nil, true
-	}
-
-	if !filtered {
-		if ptr {
-			return reflect.PtrTo(prop), false
-		}
-		return prop, false
-	}
-
-	ret := reflect.StructOf(filteredFields)
-	if ptr {
-		ret = reflect.PtrTo(ret)
-	}
-
-	return ret, true
-}
-
-// FilterPropertyStructSharded takes a reflect.Type that is either a sturct or a pointer to a struct, and returns a list
-// of reflect.Type that only contains the fields in the original type for which predicate returns true, and a bool that
-// is true if the new struct type has fewer fields than the original type.  If there are no fields in the original type
-// for which predicate returns true it returns nil and true.  Each returned struct type will have a maximum of 10 top
-// level fields in it to attempt to avoid hitting the 65535 byte type name length limit in reflect.StructOf
-// (reflect.nameFrom: name too long), although the limit can still be reached with a single struct field with many
-// fields in it.
-func FilterPropertyStructSharded(prop reflect.Type, predicate FilterFieldPredicate) (filteredProp []reflect.Type, filtered bool) {
-	var fields []reflect.StructField
-
-	ptr := prop.Kind() == reflect.Ptr
-	if ptr {
-		prop = prop.Elem()
-	}
-
-	for i := 0; i < prop.NumField(); i++ {
-		fields = append(fields, prop.Field(i))
-	}
-
-	fields, filtered = filterPropertyStructFields(fields, "", predicate)
-	if !filtered {
-		if ptr {
-			return []reflect.Type{reflect.PtrTo(prop)}, false
-		}
-		return []reflect.Type{prop}, false
-	}
-
-	if len(fields) == 0 {
-		return nil, true
-	}
-
-	shards := shardFields(fields, 10)
-
-	for _, shard := range shards {
-		s := reflect.StructOf(shard)
-		if ptr {
-			s = reflect.PtrTo(s)
-		}
-		filteredProp = append(filteredProp, s)
-	}
-
-	return filteredProp, true
-}
-
-func shardFields(fields []reflect.StructField, shardSize int) [][]reflect.StructField {
-	ret := make([][]reflect.StructField, 0, (len(fields)+shardSize-1)/shardSize)
-	for len(fields) > shardSize {
-		ret = append(ret, fields[0:shardSize])
-		fields = fields[shardSize:]
-	}
-	if len(fields) > 0 {
-		ret = append(ret, fields)
-	}
-	return ret
-}
diff --git a/proptools/filter_test.go b/proptools/filter_test.go
deleted file mode 100644
index 695549a..0000000
--- a/proptools/filter_test.go
+++ /dev/null
@@ -1,239 +0,0 @@
-// Copyright 2019 Google Inc. All rights reserved.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//     http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package proptools
-
-import (
-	"reflect"
-	"testing"
-)
-
-type Named struct {
-	A *string `keep:"true"`
-	B *string
-}
-
-type NamedAllFiltered struct {
-	A *string
-}
-
-type NamedNoneFiltered struct {
-	A *string `keep:"true"`
-}
-
-func TestFilterPropertyStruct(t *testing.T) {
-	tests := []struct {
-		name     string
-		in       interface{}
-		out      interface{}
-		filtered bool
-	}{
-		// Property tests
-		{
-			name: "basic",
-			in: &struct {
-				A *string `keep:"true"`
-				B *string
-			}{},
-			out: &struct {
-				A *string
-			}{},
-			filtered: true,
-		},
-		{
-			name: "all filtered",
-			in: &struct {
-				A *string
-			}{},
-			out:      nil,
-			filtered: true,
-		},
-		{
-			name: "none filtered",
-			in: &struct {
-				A *string `keep:"true"`
-			}{},
-			out: &struct {
-				A *string `keep:"true"`
-			}{},
-			filtered: false,
-		},
-
-		// Sub-struct tests
-		{
-			name: "substruct",
-			in: &struct {
-				A struct {
-					A *string `keep:"true"`
-					B *string
-				} `keep:"true"`
-			}{},
-			out: &struct {
-				A struct {
-					A *string
-				}
-			}{},
-			filtered: true,
-		},
-		{
-			name: "substruct all filtered",
-			in: &struct {
-				A struct {
-					A *string
-				} `keep:"true"`
-			}{},
-			out:      nil,
-			filtered: true,
-		},
-		{
-			name: "substruct none filtered",
-			in: &struct {
-				A struct {
-					A *string `keep:"true"`
-				} `keep:"true"`
-			}{},
-			out: &struct {
-				A struct {
-					A *string `keep:"true"`
-				} `keep:"true"`
-			}{},
-			filtered: false,
-		},
-
-		// Named sub-struct tests
-		{
-			name: "named substruct",
-			in: &struct {
-				A Named `keep:"true"`
-			}{},
-			out: &struct {
-				A struct {
-					A *string
-				}
-			}{},
-			filtered: true,
-		},
-		{
-			name: "substruct all filtered",
-			in: &struct {
-				A NamedAllFiltered `keep:"true"`
-			}{},
-			out:      nil,
-			filtered: true,
-		},
-		{
-			name: "substruct none filtered",
-			in: &struct {
-				A NamedNoneFiltered `keep:"true"`
-			}{},
-			out: &struct {
-				A NamedNoneFiltered `keep:"true"`
-			}{},
-			filtered: false,
-		},
-
-		// Pointer to sub-struct tests
-		{
-			name: "pointer substruct",
-			in: &struct {
-				A *struct {
-					A *string `keep:"true"`
-					B *string
-				} `keep:"true"`
-			}{},
-			out: &struct {
-				A *struct {
-					A *string
-				}
-			}{},
-			filtered: true,
-		},
-		{
-			name: "pointer substruct all filtered",
-			in: &struct {
-				A *struct {
-					A *string
-				} `keep:"true"`
-			}{},
-			out:      nil,
-			filtered: true,
-		},
-		{
-			name: "pointer substruct none filtered",
-			in: &struct {
-				A *struct {
-					A *string `keep:"true"`
-				} `keep:"true"`
-			}{},
-			out: &struct {
-				A *struct {
-					A *string `keep:"true"`
-				} `keep:"true"`
-			}{},
-			filtered: false,
-		},
-
-		// Pointer to named sub-struct tests
-		{
-			name: "pointer named substruct",
-			in: &struct {
-				A *Named `keep:"true"`
-			}{},
-			out: &struct {
-				A *struct {
-					A *string
-				}
-			}{},
-			filtered: true,
-		},
-		{
-			name: "pointer substruct all filtered",
-			in: &struct {
-				A *NamedAllFiltered `keep:"true"`
-			}{},
-			out:      nil,
-			filtered: true,
-		},
-		{
-			name: "pointer substruct none filtered",
-			in: &struct {
-				A *NamedNoneFiltered `keep:"true"`
-			}{},
-			out: &struct {
-				A *NamedNoneFiltered `keep:"true"`
-			}{},
-			filtered: false,
-		},
-	}
-
-	for _, test := range tests {
-		t.Run(test.name, func(t *testing.T) {
-			out, filtered := FilterPropertyStruct(reflect.TypeOf(test.in),
-				func(field reflect.StructField, prefix string) (bool, reflect.StructField) {
-					if HasTag(field, "keep", "true") {
-						field.Tag = ""
-						return true, field
-					}
-					return false, field
-				})
-			if filtered != test.filtered {
-				t.Errorf("expected filtered %v, got %v", test.filtered, filtered)
-			}
-			expected := reflect.TypeOf(test.out)
-			if out != expected {
-				t.Errorf("expected type %v, got %v", expected, out)
-			}
-		})
-	}
-}
diff --git a/proptools/proptools.go b/proptools/proptools.go
index 6881828..e6e3ae7 100644
--- a/proptools/proptools.go
+++ b/proptools/proptools.go
@@ -82,18 +82,3 @@ func StringDefault(s *string, def string) string {
 func String(s *string) string {
 	return StringDefault(s, "")
 }
-
-// IntDefault takes a pointer to an int64 and returns the value pointed to by the pointer cast to int
-// if it is non-nil, or def if the pointer is nil.
-func IntDefault(i *int64, def int) int {
-	if i != nil {
-		return int(*i)
-	}
-	return def
-}
-
-// Int takes a pointer to an int64 and returns the value pointed to by the pointer cast to int
-// if it is non-nil, or 0 if the pointer is nil.
-func Int(i *int64) int {
-	return IntDefault(i, 0)
-}
diff --git a/singleton_ctx.go b/singleton_ctx.go
index 3c0a24c..5ca8ee6 100644
--- a/singleton_ctx.go
+++ b/singleton_ctx.go
@@ -25,51 +25,23 @@ type Singleton interface {
 }
 
 type SingletonContext interface {
-	// Config returns the config object that was passed to Context.PrepareBuildActions.
 	Config() interface{}
 
-	// Name returns the name of the current singleton passed to Context.RegisterSingletonType
 	Name() string
 
-	// ModuleName returns the name of the given Module.  See BaseModuleContext.ModuleName for more information.
 	ModuleName(module Module) string
-
-	// ModuleDir returns the directory of the given Module.  See BaseModuleContext.ModuleDir for more information.
 	ModuleDir(module Module) string
-
-	// ModuleSubDir returns the unique subdirectory name of the given Module.  See ModuleContext.ModuleSubDir for
-	// more information.
 	ModuleSubDir(module Module) string
-
-	// ModuleType returns the type of the given Module.  See BaseModuleContext.ModuleType for more information.
 	ModuleType(module Module) string
-
-	// BlueprintFile returns the path of the Blueprint file that defined the given module.
 	BlueprintFile(module Module) string
 
-	// ModuleErrorf reports an error at the line number of the module type in the module definition.
 	ModuleErrorf(module Module, format string, args ...interface{})
-
-	// Errorf reports an error at the specified position of the module definition file.
 	Errorf(format string, args ...interface{})
-
-	// Failed returns true if any errors have been reported.  In most cases the singleton can continue with generating
-	// build rules after an error, allowing it to report additional errors in a single run, but in cases where the error
-	// has prevented the singleton from creating necessary data it can return early when Failed returns true.
 	Failed() bool
 
-	// Variable creates a new ninja variable scoped to the singleton.  It can be referenced by calls to Rule and Build
-	// in the same singleton.
 	Variable(pctx PackageContext, name, value string)
-
-	// Rule creates a new ninja rule scoped to the singleton.  It can be referenced by calls to Build in the same
-	// singleton.
 	Rule(pctx PackageContext, name string, params RuleParams, argNames ...string) Rule
-
-	// Build creates a new ninja build statement.
 	Build(pctx PackageContext, params BuildParams)
-
-	// RequireNinjaVersion sets the generated ninja manifest to require at least the specified version of ninja.
 	RequireNinjaVersion(major, minor, micro int)
 
 	// SetNinjaBuildDir sets the value of the top-level "builddir" Ninja variable
@@ -86,54 +58,17 @@ type SingletonContext interface {
 	// are expanded in the scope of the PackageContext.
 	Eval(pctx PackageContext, ninjaStr string) (string, error)
 
-	// VisitAllModules calls visit for each defined variant of each module in an unspecified order.
 	VisitAllModules(visit func(Module))
-
-	// VisitAllModules calls pred for each defined variant of each module in an unspecified order, and if pred returns
-	// true calls visit.
 	VisitAllModulesIf(pred func(Module) bool, visit func(Module))
-
-	// VisitDirectDeps calls visit for each direct dependency of the Module.  If there are
-	// multiple direct dependencies on the same module visit will be called multiple times on
-	// that module and OtherModuleDependencyTag will return a different tag for each.
-	//
-	// The Module passed to the visit function should not be retained outside of the visit
-	// function, it may be invalidated by future mutators.
-	VisitDirectDeps(module Module, visit func(Module))
-
-	// VisitDirectDepsIf calls pred for each direct dependency of the Module, and if pred
-	// returns true calls visit.  If there are multiple direct dependencies on the same module
-	// pred and visit will be called multiple times on that module and OtherModuleDependencyTag
-	// will return a different tag for each.
-	//
-	// The Module passed to the visit function should not be retained outside of the visit
-	// function, it may be invalidated by future mutators.
-	VisitDirectDepsIf(module Module, pred func(Module) bool, visit func(Module))
-
-	// VisitDepsDepthFirst calls visit for each transitive dependency, traversing the dependency tree in depth first
-	// order. visit will only be called once for any given module, even if there are multiple paths through the
-	// dependency tree to the module or multiple direct dependencies with different tags.
 	VisitDepsDepthFirst(module Module, visit func(Module))
-
-	// VisitDepsDepthFirst calls pred for each transitive dependency, and if pred returns true calls visit, traversing
-	// the dependency tree in depth first order.  visit will only be called once for any given module, even if there are
-	// multiple paths through the dependency tree to the module or multiple direct dependencies with different tags.
 	VisitDepsDepthFirstIf(module Module, pred func(Module) bool,
 		visit func(Module))
 
-	// VisitAllModuleVariants calls visit for each variant of the given module.
 	VisitAllModuleVariants(module Module, visit func(Module))
 
-	// PrimaryModule returns the first variant of the given module.  This can be used to perform
-	//	// singleton actions that are only done once for all variants of a module.
 	PrimaryModule(module Module) Module
-
-	// FinalModule returns the last variant of the given module.  This can be used to perform
-	// singleton actions that are only done once for all variants of a module.
 	FinalModule(module Module) Module
 
-	// AddNinjaFileDeps adds dependencies on the specified files to the rule that creates the ninja manifest.  The
-	// primary builder will be rerun whenever the specified files are modified.
 	AddNinjaFileDeps(deps ...string)
 
 	// GlobWithDeps returns a list of files and directories that match the
@@ -144,8 +79,6 @@ type SingletonContext interface {
 	// does not match the pattern is added to a searched directory.
 	GlobWithDeps(pattern string, excludes []string) ([]string, error)
 
-	// Fs returns a pathtools.Filesystem that can be used to interact with files.  Using the Filesystem interface allows
-	// the singleton to be used in build system tests that run against a mock filesystem.
 	Fs() pathtools.FileSystem
 }
 
@@ -290,7 +223,7 @@ func (s *singletonContext) VisitAllModules(visit func(Module)) {
 	defer func() {
 		if r := recover(); r != nil {
 			panic(newPanicErrorf(r, "VisitAllModules(%s) for module %s",
-				funcName(visit), s.context.moduleInfo[visitingModule]))
+				funcName(visit), visitingModule))
 		}
 	}()
 
@@ -306,14 +239,6 @@ func (s *singletonContext) VisitAllModulesIf(pred func(Module) bool,
 	s.context.VisitAllModulesIf(pred, visit)
 }
 
-func (s *singletonContext) VisitDirectDeps(module Module, visit func(Module)) {
-	s.context.VisitDirectDeps(module, visit)
-}
-
-func (s *singletonContext) VisitDirectDepsIf(module Module, pred func(Module) bool, visit func(Module)) {
-	s.context.VisitDirectDepsIf(module, pred, visit)
-}
-
 func (s *singletonContext) VisitDepsDepthFirst(module Module,
 	visit func(Module)) {