Merge pull request #166 from colincross/create

Add TopDownMutatorContext.CreateModule

9 files changed, 381 insertions, 207 deletions

diff --git a/.travis.yml b/.travis.yml
index dd1c743..72b7f79 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -19,6 +19,7 @@ script:
     - export GOROOT=$(go env GOROOT)
     - ./.travis.gofmt.sh
     - go test ./...
+    - go test -race -short ./...
     - ./tests/test.sh
     - ./tests/test_tree_tests.sh
     - ./tests/test_tree_tests.sh -t
diff --git a/bootstrap/command.go b/bootstrap/command.go
index 0c0fbe7..e4518f1 100644
--- a/bootstrap/command.go
+++ b/bootstrap/command.go
@@ -128,10 +128,11 @@ func Main(ctx *blueprint.Context, config interface{}, extraNinjaFileDeps ...stri
 	// Add extra ninja file dependencies
 	deps = append(deps, extraNinjaFileDeps...)
 
-	errs = ctx.ResolveDependencies(config)
+	extraDeps, errs := ctx.ResolveDependencies(config)
 	if len(errs) > 0 {
 		fatalErrors(errs)
 	}
+	deps = append(deps, extraDeps...)
 
 	if docFile != "" {
 		err := writeDocs(ctx, filepath.Dir(bootstrapConfig.topLevelBlueprintsFile), docFile)
@@ -141,7 +142,7 @@ func Main(ctx *blueprint.Context, config interface{}, extraNinjaFileDeps ...stri
 		return
 	}
 
-	extraDeps, errs := ctx.PrepareBuildActions(config)
+	extraDeps, errs = ctx.PrepareBuildActions(config)
 	if len(errs) > 0 {
 		fatalErrors(errs)
 	}
diff --git a/context.go b/context.go
index 4b9ea72..414871d 100644
--- a/context.go
+++ b/context.go
@@ -158,6 +158,7 @@ type moduleGroup struct {
 type moduleInfo struct {
 	// set during Parse
 	typeName          string
+	factory           ModuleFactory
 	relBlueprintsFile string
 	pos               scanner.Position
 	propertyPos       map[string]scanner.Position
@@ -166,9 +167,9 @@ type moduleInfo struct {
 	variant           variationMap
 	dependencyVariant variationMap
 
-	logicModule      Module
-	group            *moduleGroup
-	moduleProperties []interface{}
+	logicModule Module
+	group       *moduleGroup
+	properties  []interface{}
 
 	// set during ResolveDependencies
 	directDeps  []depInfo
@@ -261,12 +262,8 @@ type mutatorInfo struct {
 	parallel        bool
 }
 
-// NewContext creates a new Context object.  The created context initially has
-// no module or singleton factories registered, so the RegisterModuleFactory and
-// RegisterSingletonFactory methods must be called before it can do anything
-// useful.
-func NewContext() *Context {
-	ctx := &Context{
+func newContext() *Context {
+	return &Context{
 		moduleFactories:  make(map[string]ModuleFactory),
 		moduleNames:      make(map[string]*moduleGroup),
 		moduleInfo:       make(map[Module]*moduleInfo),
@@ -274,6 +271,14 @@ func NewContext() *Context {
 		globs:            make(map[string]GlobPath),
 		fs:               pathtools.OsFs,
 	}
+}
+
+// NewContext creates a new Context object.  The created context initially has
+// no module or singleton factories registered, so the RegisterModuleFactory and
+// RegisterSingletonFactory methods must be called before it can do anything
+// useful.
+func NewContext() *Context {
+	ctx := newContext()
 
 	ctx.RegisterBottomUpMutator("blueprint_deps", blueprintDepsMutator)
 
@@ -958,21 +963,15 @@ func getStringFromScope(scope *parser.Scope, v string) (string, scanner.Position
 // property values.  Any values stored in the module object that are not stored in properties
 // structs will be lost.
 func (c *Context) cloneLogicModule(origModule *moduleInfo) (Module, []interface{}) {
-	typeName := origModule.typeName
-	factory, ok := c.moduleFactories[typeName]
-	if !ok {
-		panic(fmt.Sprintf("unrecognized module type %q during cloning", typeName))
-	}
-
-	newLogicModule, newProperties := factory()
+	newLogicModule, newProperties := origModule.factory()
 
-	if len(newProperties) != len(origModule.moduleProperties) {
+	if len(newProperties) != len(origModule.properties) {
 		panic("mismatched properties array length in " + origModule.Name())
 	}
 
 	for i := range newProperties {
 		dst := reflect.ValueOf(newProperties[i]).Elem()
-		src := reflect.ValueOf(origModule.moduleProperties[i]).Elem()
+		src := reflect.ValueOf(origModule.properties[i]).Elem()
 
 		proptools.CopyProperties(dst, src)
 	}
@@ -1000,7 +999,7 @@ func (c *Context) createVariations(origModule *moduleInfo, mutatorName string,
 			// Reuse the existing module for the first new variant
 			// This both saves creating a new module, and causes the insertion in c.moduleInfo below
 			// with logicModule as the key to replace the original entry in c.moduleInfo
-			newLogicModule, newProperties = origModule.logicModule, origModule.moduleProperties
+			newLogicModule, newProperties = origModule.logicModule, origModule.properties
 		} else {
 			newLogicModule, newProperties = c.cloneLogicModule(origModule)
 		}
@@ -1014,7 +1013,7 @@ func (c *Context) createVariations(origModule *moduleInfo, mutatorName string,
 		newModule.logicModule = newLogicModule
 		newModule.variant = newVariant
 		newModule.dependencyVariant = origModule.dependencyVariant.clone()
-		newModule.moduleProperties = newProperties
+		newModule.properties = newProperties
 
 		if variationName != "" {
 			if newModule.variantName == "" {
@@ -1080,6 +1079,19 @@ func (c *Context) prettyPrintVariant(variant variationMap) string {
 	return strings.Join(names, ", ")
 }
 
+func (c *Context) newModule(factory ModuleFactory) *moduleInfo {
+	logicModule, properties := factory()
+
+	module := &moduleInfo{
+		logicModule: logicModule,
+		factory:     factory,
+	}
+
+	module.properties = properties
+
+	return module
+}
+
 func (c *Context) processModuleDef(moduleDef *parser.Module,
 	relBlueprintsFile string) (*moduleInfo, []error) {
 
@@ -1097,17 +1109,12 @@ func (c *Context) processModuleDef(moduleDef *parser.Module,
 		}
 	}
 
-	logicModule, properties := factory()
-
-	module := &moduleInfo{
-		logicModule:       logicModule,
-		typeName:          moduleDef.Type,
-		relBlueprintsFile: relBlueprintsFile,
-	}
+	module := c.newModule(factory)
+	module.typeName = moduleDef.Type
 
-	module.moduleProperties = properties
+	module.relBlueprintsFile = relBlueprintsFile
 
-	propertyMap, errs := unpackProperties(moduleDef.Properties, properties...)
+	propertyMap, errs := unpackProperties(moduleDef.Properties, module.properties...)
 	if len(errs) > 0 {
 		return nil, errs
 	}
@@ -1163,21 +1170,21 @@ func (c *Context) addModule(module *moduleInfo) []error {
 // modules defined in the parsed Blueprints files are valid.  This means that
 // the modules depended upon are defined and that no circular dependencies
 // exist.
-func (c *Context) ResolveDependencies(config interface{}) []error {
-	errs := c.updateDependencies()
+func (c *Context) ResolveDependencies(config interface{}) (deps []string, errs []error) {
+	errs = c.updateDependencies()
 	if len(errs) > 0 {
-		return errs
+		return nil, errs
 	}
 
-	errs = c.runMutators(config)
+	deps, errs = c.runMutators(config)
 	if len(errs) > 0 {
-		return errs
+		return nil, errs
 	}
 
 	c.cloneModules()
 
 	c.dependenciesReady = true
-	return nil
+	return deps, nil
 }
 
 // Default dependencies handling.  If the module implements the (deprecated)
@@ -1644,10 +1651,11 @@ func (c *Context) PrepareBuildActions(config interface{}) (deps []string, errs [
 	c.buildActionsReady = false
 
 	if !c.dependenciesReady {
-		errs := c.ResolveDependencies(config)
+		extraDeps, errs := c.ResolveDependencies(config)
 		if len(errs) > 0 {
 			return nil, errs
 		}
+		deps = append(deps, extraDeps...)
 	}
 
 	liveGlobals := newLiveTracker(config)
@@ -1664,7 +1672,8 @@ func (c *Context) PrepareBuildActions(config interface{}) (deps []string, errs [
 		return nil, errs
 	}
 
-	deps = append(depsModules, depsSingletons...)
+	deps = append(deps, depsModules...)
+	deps = append(deps, depsSingletons...)
 
 	if c.ninjaBuildDir != nil {
 		liveGlobals.addNinjaStringDeps(c.ninjaBuildDir)
@@ -1687,26 +1696,28 @@ func (c *Context) PrepareBuildActions(config interface{}) (deps []string, errs [
 	return deps, nil
 }
 
-func (c *Context) runMutators(config interface{}) (errs []error) {
+func (c *Context) runMutators(config interface{}) (deps []string, errs []error) {
 	var mutators []*mutatorInfo
 
 	mutators = append(mutators, c.earlyMutatorInfo...)
 	mutators = append(mutators, c.mutatorInfo...)
 
 	for _, mutator := range mutators {
+		var newDeps []string
 		if mutator.topDownMutator != nil {
-			errs = c.runMutator(config, mutator, topDownMutator)
+			newDeps, errs = c.runMutator(config, mutator, topDownMutator)
 		} else if mutator.bottomUpMutator != nil {
-			errs = c.runMutator(config, mutator, bottomUpMutator)
+			newDeps, errs = c.runMutator(config, mutator, bottomUpMutator)
 		} else {
 			panic("no mutator set on " + mutator.name)
 		}
 		if len(errs) > 0 {
-			return errs
+			return nil, errs
 		}
+		deps = append(deps, newDeps...)
 	}
 
-	return nil
+	return deps, nil
 }
 
 type mutatorDirection interface {
@@ -1754,7 +1765,7 @@ type reverseDep struct {
 }
 
 func (c *Context) runMutator(config interface{}, mutator *mutatorInfo,
-	direction mutatorDirection) (errs []error) {
+	direction mutatorDirection) (deps []string, errs []error) {
 
 	newModuleInfo := make(map[Module]*moduleInfo)
 	for k, v := range c.moduleInfo {
@@ -1762,18 +1773,21 @@ func (c *Context) runMutator(config interface{}, mutator *mutatorInfo,
 	}
 
 	type globalStateChange struct {
-		reverse []reverseDep
-		rename  []rename
-		replace []replace
+		reverse    []reverseDep
+		rename     []rename
+		replace    []replace
+		newModules []*moduleInfo
+		deps       []string
 	}
 
 	reverseDeps := make(map[*moduleInfo][]depInfo)
 	var rename []rename
 	var replace []replace
+	var newModules []*moduleInfo
 
 	errsCh := make(chan []error)
 	globalStateCh := make(chan globalStateChange)
-	newModulesCh := make(chan []*moduleInfo)
+	newVariationsCh := make(chan []*moduleInfo)
 	done := make(chan bool)
 
 	c.depsModified = 0
@@ -1812,15 +1826,17 @@ func (c *Context) runMutator(config interface{}, mutator *mutatorInfo,
 			return true
 		}
 
-		if len(mctx.newModules) > 0 {
-			newModulesCh <- mctx.newModules
+		if len(mctx.newVariations) > 0 {
+			newVariationsCh <- mctx.newVariations
 		}
 
-		if len(mctx.reverseDeps) > 0 || len(mctx.replace) > 0 || len(mctx.rename) > 0 {
+		if len(mctx.reverseDeps) > 0 || len(mctx.replace) > 0 || len(mctx.rename) > 0 || len(mctx.newModules) > 0 {
 			globalStateCh <- globalStateChange{
-				reverse: mctx.reverseDeps,
-				replace: mctx.replace,
-				rename:  mctx.rename,
+				reverse:    mctx.reverseDeps,
+				replace:    mctx.replace,
+				rename:     mctx.rename,
+				newModules: mctx.newModules,
+				deps:       mctx.ninjaFileDeps,
 			}
 		}
 
@@ -1839,8 +1855,10 @@ func (c *Context) runMutator(config interface{}, mutator *mutatorInfo,
 				}
 				replace = append(replace, globalStateChange.replace...)
 				rename = append(rename, globalStateChange.rename...)
-			case newModules := <-newModulesCh:
-				for _, m := range newModules {
+				newModules = append(newModules, globalStateChange.newModules...)
+				deps = append(deps, globalStateChange.deps...)
+			case newVariations := <-newVariationsCh:
+				for _, m := range newVariations {
 					newModuleInfo[m.logicModule] = m
 				}
 			case <-done:
@@ -1858,7 +1876,7 @@ func (c *Context) runMutator(config interface{}, mutator *mutatorInfo,
 	done <- true
 
 	if len(errs) > 0 {
-		return errs
+		return nil, errs
 	}
 
 	c.moduleInfo = newModuleInfo
@@ -1888,24 +1906,32 @@ func (c *Context) runMutator(config interface{}, mutator *mutatorInfo,
 		c.depsModified++
 	}
 
+	for _, module := range newModules {
+		errs = c.addModule(module)
+		if len(errs) > 0 {
+			return nil, errs
+		}
+		atomic.AddUint32(&c.depsModified, 1)
+	}
+
 	errs = c.handleRenames(rename)
 	if len(errs) > 0 {
-		return errs
+		return nil, errs
 	}
 
 	errs = c.handleReplacements(replace)
 	if len(errs) > 0 {
-		return errs
+		return nil, errs
 	}
 
 	if c.depsModified > 0 {
 		errs = c.updateDependencies()
 		if len(errs) > 0 {
-			return errs
+			return nil, errs
 		}
 	}
 
-	return errs
+	return deps, errs
 }
 
 // Replaces every build logic module with a clone of itself.  Prevents introducing problems where
@@ -1921,7 +1947,7 @@ func (c *Context) cloneModules() {
 	for _, m := range c.modulesSorted {
 		go func(m *moduleInfo) {
 			origLogicModule := m.logicModule
-			m.logicModule, m.moduleProperties = c.cloneLogicModule(m)
+			m.logicModule, m.properties = c.cloneLogicModule(m)
 			ch <- update{origLogicModule, m}
 		}(m)
 	}
@@ -3025,8 +3051,7 @@ func (c *Context) writeAllModuleActions(nw *ninjaWriter) error {
 		relPos.Filename = module.relBlueprintsFile
 
 		// Get the name and location of the factory function for the module.
-		factory := c.moduleFactories[module.typeName]
-		factoryFunc := runtime.FuncForPC(reflect.ValueOf(factory).Pointer())
+		factoryFunc := runtime.FuncForPC(reflect.ValueOf(module.factory).Pointer())
 		factoryName := factoryFunc.Name()
 
 		infoMap := map[string]interface{}{
diff --git a/context_test.go b/context_test.go
index b05c541..b9fb913 100644
--- a/context_test.go
+++ b/context_test.go
@@ -16,6 +16,7 @@ package blueprint
 
 import (
 	"bytes"
+	"strings"
 	"testing"
 )
 
@@ -104,7 +105,7 @@ func TestContextParse(t *testing.T) {
 		t.FailNow()
 	}
 
-	errs = ctx.ResolveDependencies(nil)
+	_, errs = ctx.ResolveDependencies(nil)
 	if len(errs) > 0 {
 		t.Errorf("unexpected dep errors:")
 		for _, err := range errs {
@@ -169,7 +170,7 @@ func TestWalkDeps(t *testing.T) {
 		t.FailNow()
 	}
 
-	errs = ctx.ResolveDependencies(nil)
+	_, errs = ctx.ResolveDependencies(nil)
 	if len(errs) > 0 {
 		t.Errorf("unexpected dep errors:")
 		for _, err := range errs {
@@ -201,3 +202,81 @@ func TestWalkDeps(t *testing.T) {
 		t.Fatalf("unexpected walkDeps behaviour: %s\nup should be: GFC", outputUp)
 	}
 }
+
+func TestCreateModule(t *testing.T) {
+	ctx := newContext()
+	ctx.MockFileSystem(map[string][]byte{
+		"Blueprints": []byte(`
+			foo_module {
+			    name: "A",
+			    deps: ["B", "C"],
+			}
+		`),
+	})
+
+	ctx.RegisterTopDownMutator("create", createTestMutator)
+	ctx.RegisterBottomUpMutator("deps", blueprintDepsMutator)
+
+	ctx.RegisterModuleType("foo_module", newFooModule)
+	ctx.RegisterModuleType("bar_module", newBarModule)
+	_, errs := ctx.ParseBlueprintsFiles("Blueprints")
+	if len(errs) > 0 {
+		t.Errorf("unexpected parse errors:")
+		for _, err := range errs {
+			t.Errorf("  %s", err)
+		}
+		t.FailNow()
+	}
+
+	_, errs = ctx.ResolveDependencies(nil)
+	if len(errs) > 0 {
+		t.Errorf("unexpected dep errors:")
+		for _, err := range errs {
+			t.Errorf("  %s", err)
+		}
+		t.FailNow()
+	}
+
+	a := ctx.modulesFromName("A")[0].logicModule.(*fooModule)
+	b := ctx.modulesFromName("B")[0].logicModule.(*barModule)
+	c := ctx.modulesFromName("C")[0].logicModule.(*barModule)
+	d := ctx.modulesFromName("D")[0].logicModule.(*fooModule)
+
+	checkDeps := func(m Module, expected string) {
+		var deps []string
+		ctx.VisitDirectDeps(m, func(m Module) {
+			deps = append(deps, ctx.ModuleName(m))
+		})
+		got := strings.Join(deps, ",")
+		if got != expected {
+			t.Errorf("unexpected %q dependencies, got %q expected %q",
+				ctx.ModuleName(m), got, expected)
+		}
+	}
+
+	checkDeps(a, "B,C")
+	checkDeps(b, "D")
+	checkDeps(c, "D")
+	checkDeps(d, "")
+}
+
+func createTestMutator(ctx TopDownMutatorContext) {
+	type props struct {
+		Name string
+		Deps []string
+	}
+
+	ctx.CreateModule(newBarModule, &props{
+		Name: "B",
+		Deps: []string{"D"},
+	})
+
+	ctx.CreateModule(newBarModule, &props{
+		Name: "C",
+		Deps: []string{"D"},
+	})
+
+	ctx.CreateModule(newFooModule, &props{
+		Name: "D",
+	})
+}
diff --git a/module_ctx.go b/module_ctx.go
index e23caac..580f354 100644
--- a/module_ctx.go
+++ b/module_ctx.go
@@ -20,6 +20,7 @@ import (
 	"text/scanner"
 
 	"github.com/google/blueprint/pathtools"
+	"github.com/google/blueprint/proptools"
 )
 
 // A Module handles generating all of the Ninja build actions needed to build a
@@ -137,6 +138,7 @@ type BaseModuleContext interface {
 	GlobWithDeps(pattern string, excludes []string) ([]string, error)
 
 	Fs() pathtools.FileSystem
+	AddNinjaFileDeps(deps ...string)
 
 	moduleInfo() *moduleInfo
 	error(err error)
@@ -166,8 +168,6 @@ type ModuleContext interface {
 	Rule(pctx PackageContext, name string, params RuleParams, argNames ...string) Rule
 	Build(pctx PackageContext, params BuildParams)
 
-	AddNinjaFileDeps(deps ...string)
-
 	PrimaryModule() Module
 	FinalModule() Module
 	VisitAllModuleVariants(visit func(Module))
@@ -184,6 +184,7 @@ type baseModuleContext struct {
 	errs           []error
 	visitingParent *moduleInfo
 	visitingDep    depInfo
+	ninjaFileDeps  []string
 }
 
 func (d *baseModuleContext) moduleInfo() *moduleInfo {
@@ -273,7 +274,6 @@ var _ ModuleContext = (*moduleContext)(nil)
 type moduleContext struct {
 	baseModuleContext
 	scope              *localScope
-	ninjaFileDeps      []string
 	actionDefs         localBuildActions
 	handledMissingDeps bool
 }
@@ -430,6 +430,10 @@ func (m *baseModuleContext) WalkDeps(visit func(Module, Module) bool) {
 	m.visitingDep = depInfo{}
 }
 
+func (m *baseModuleContext) AddNinjaFileDeps(deps ...string) {
+	m.ninjaFileDeps = append(m.ninjaFileDeps, deps...)
+}
+
 func (m *moduleContext) ModuleSubDir() string {
 	return m.module.variantName
 }
@@ -471,10 +475,6 @@ func (m *moduleContext) Build(pctx PackageContext, params BuildParams) {
 	m.actionDefs.buildDefs = append(m.actionDefs.buildDefs, def)
 }
 
-func (m *moduleContext) AddNinjaFileDeps(deps ...string) {
-	m.ninjaFileDeps = append(m.ninjaFileDeps, deps...)
-}
-
 func (m *moduleContext) PrimaryModule() Module {
 	return m.module.group.modules[0].logicModule
 }
@@ -498,11 +498,12 @@ func (m *moduleContext) GetMissingDependencies() []string {
 
 type mutatorContext struct {
 	baseModuleContext
-	name        string
-	reverseDeps []reverseDep
-	rename      []rename
-	replace     []replace
-	newModules  []*moduleInfo
+	name          string
+	reverseDeps   []reverseDep
+	rename        []rename
+	replace       []replace
+	newVariations []*moduleInfo // new variants of existing modules
+	newModules    []*moduleInfo // brand new modules
 }
 
 type baseMutatorContext interface {
@@ -527,6 +528,8 @@ type TopDownMutatorContext interface {
 	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)
 
@@ -620,10 +623,10 @@ func (mctx *mutatorContext) createVariations(variationNames []string, local bool
 		}
 	}
 
-	if mctx.newModules != nil {
+	if mctx.newVariations != nil {
 		panic("module already has variations from this mutator")
 	}
-	mctx.newModules = modules
+	mctx.newVariations = modules
 
 	if len(ret) != len(variationNames) {
 		panic("oops!")
@@ -737,6 +740,21 @@ func (mctx *mutatorContext) Rename(name string) {
 	mctx.rename = append(mctx.rename, rename{mctx.module.group, name})
 }
 
+// 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)
+
+	for _, p := range props {
+		err := proptools.AppendMatchingProperties(module.properties, p, nil)
+		if err != nil {
+			panic(err)
+		}
+	}
+
+	mctx.newModules = append(mctx.newModules, module)
+}
+
 // SimpleName is an embeddable object to implement the ModuleContext.Name method using a property
 // called "name".  Modules that embed it must also add SimpleName.Properties to their property
 // structure list.
diff --git a/proptools/extend.go b/proptools/extend.go
index 6f735ce..16f48c3 100644
--- a/proptools/extend.go
+++ b/proptools/extend.go
@@ -238,7 +238,7 @@ func extendMatchingProperties(dst []interface{}, src interface{}, filter ExtendP
 
 func extendPropertiesRecursive(dstValues []reflect.Value, srcValue reflect.Value,
 	prefix string, filter ExtendPropertyFilterFunc, sameTypes bool,
-	order ExtendPropertyOrderFunc) error {
+	orderFunc ExtendPropertyOrderFunc) error {
 
 	srcType := srcValue.Type()
 	for i, srcField := range typeFields(srcType) {
@@ -373,9 +373,10 @@ func extendPropertiesRecursive(dstValues []reflect.Value, srcValue reflect.Value
 				}
 			}
 
-			prepend := false
-			if order != nil {
-				b, err := order(propertyName, dstField, srcField,
+			order := Append
+			if orderFunc != nil {
+				var err error
+				order, err = orderFunc(propertyName, dstField, srcField,
 					dstFieldInterface, srcFieldInterface)
 				if err != nil {
 					return &ExtendPropertyError{
@@ -383,69 +384,14 @@ func extendPropertiesRecursive(dstValues []reflect.Value, srcValue reflect.Value
 						Err:      err,
 					}
 				}
-				prepend = b == Prepend
 			}
 
-			switch srcFieldValue.Kind() {
-			case reflect.Bool:
-				// Boolean OR
-				dstFieldValue.Set(reflect.ValueOf(srcFieldValue.Bool() || dstFieldValue.Bool()))
-			case reflect.String:
-				// Append the extension string.
-				if prepend {
-					dstFieldValue.SetString(srcFieldValue.String() +
-						dstFieldValue.String())
-				} else {
-					dstFieldValue.SetString(dstFieldValue.String() +
-						srcFieldValue.String())
-				}
-			case reflect.Slice:
-				if srcFieldValue.IsNil() {
-					break
-				}
-
-				newSlice := reflect.MakeSlice(srcFieldValue.Type(), 0,
-					dstFieldValue.Len()+srcFieldValue.Len())
-				if prepend {
-					newSlice = reflect.AppendSlice(newSlice, srcFieldValue)
-					newSlice = reflect.AppendSlice(newSlice, dstFieldValue)
-				} else {
-					newSlice = reflect.AppendSlice(newSlice, dstFieldValue)
-					newSlice = reflect.AppendSlice(newSlice, srcFieldValue)
-				}
-				dstFieldValue.Set(newSlice)
-			case reflect.Ptr:
-				if srcFieldValue.IsNil() {
-					break
-				}
-
-				switch ptrKind := srcFieldValue.Type().Elem().Kind(); ptrKind {
-				case reflect.Bool:
-					if prepend {
-						if dstFieldValue.IsNil() {
-							dstFieldValue.Set(reflect.ValueOf(BoolPtr(srcFieldValue.Elem().Bool())))
-						}
-					} else {
-						// For append, replace the original value.
-						dstFieldValue.Set(reflect.ValueOf(BoolPtr(srcFieldValue.Elem().Bool())))
-					}
-				case reflect.String:
-					if prepend {
-						if dstFieldValue.IsNil() {
-							dstFieldValue.Set(reflect.ValueOf(StringPtr(srcFieldValue.Elem().String())))
-						}
-					} else {
-						// For append, replace the original value.
-						dstFieldValue.Set(reflect.ValueOf(StringPtr(srcFieldValue.Elem().String())))
-					}
-				default:
-					panic(fmt.Errorf("unexpected pointer kind %s", ptrKind))
-				}
-			}
+			ExtendBasicType(dstFieldValue, srcFieldValue, order)
 		}
+
 		if len(recurse) > 0 {
 			err := extendPropertiesRecursive(recurse, srcFieldValue,
-				propertyName+".", filter, sameTypes, order)
+				propertyName+".", filter, sameTypes, orderFunc)
 			if err != nil {
 				return err
 			}
@@ -457,6 +403,66 @@ func extendPropertiesRecursive(dstValues []reflect.Value, srcValue reflect.Value
 	return nil
 }
 
+func ExtendBasicType(dstFieldValue, srcFieldValue reflect.Value, order Order) {
+	prepend := order == Prepend
+
+	switch srcFieldValue.Kind() {
+	case reflect.Bool:
+		// Boolean OR
+		dstFieldValue.Set(reflect.ValueOf(srcFieldValue.Bool() || dstFieldValue.Bool()))
+	case reflect.String:
+		if prepend {
+			dstFieldValue.SetString(srcFieldValue.String() +
+				dstFieldValue.String())
+		} else {
+			dstFieldValue.SetString(dstFieldValue.String() +
+				srcFieldValue.String())
+		}
+	case reflect.Slice:
+		if srcFieldValue.IsNil() {
+			break
+		}
+
+		newSlice := reflect.MakeSlice(srcFieldValue.Type(), 0,
+			dstFieldValue.Len()+srcFieldValue.Len())
+		if prepend {
+			newSlice = reflect.AppendSlice(newSlice, srcFieldValue)
+			newSlice = reflect.AppendSlice(newSlice, dstFieldValue)
+		} else {
+			newSlice = reflect.AppendSlice(newSlice, dstFieldValue)
+			newSlice = reflect.AppendSlice(newSlice, srcFieldValue)
+		}
+		dstFieldValue.Set(newSlice)
+	case reflect.Ptr:
+		if srcFieldValue.IsNil() {
+			break
+		}
+
+		switch ptrKind := srcFieldValue.Type().Elem().Kind(); ptrKind {
+		case reflect.Bool:
+			if prepend {
+				if dstFieldValue.IsNil() {
+					dstFieldValue.Set(reflect.ValueOf(BoolPtr(srcFieldValue.Elem().Bool())))
+				}
+			} else {
+				// For append, replace the original value.
+				dstFieldValue.Set(reflect.ValueOf(BoolPtr(srcFieldValue.Elem().Bool())))
+			}
+		case reflect.String:
+			if prepend {
+				if dstFieldValue.IsNil() {
+					dstFieldValue.Set(reflect.ValueOf(StringPtr(srcFieldValue.Elem().String())))
+				}
+			} else {
+				// For append, replace the original value.
+				dstFieldValue.Set(reflect.ValueOf(StringPtr(srcFieldValue.Elem().String())))
+			}
+		default:
+			panic(fmt.Errorf("unexpected pointer kind %s", ptrKind))
+		}
+	}
+}
+
 type getStructEmptyError struct{}
 
 func (getStructEmptyError) Error() string { return "interface containing nil pointer" }
diff --git a/unpack.go b/unpack.go
index 48d5105..4591f1a 100644
--- a/unpack.go
+++ b/unpack.go
@@ -240,27 +240,7 @@ func unpackStructValue(namePrefix string, structValue reflect.Value,
 
 		var newErrs []error
 
-		switch kind := fieldValue.Kind(); kind {
-		case reflect.Bool:
-			newErrs = unpackBool(fieldValue, packedProperty.property)
-		case reflect.String:
-			newErrs = unpackString(fieldValue, packedProperty.property)
-		case reflect.Slice:
-			newErrs = unpackSlice(fieldValue, packedProperty.property)
-		case reflect.Ptr:
-			switch ptrKind := fieldValue.Type().Elem().Kind(); ptrKind {
-			case reflect.Bool:
-				newValue := reflect.New(fieldValue.Type().Elem())
-				newErrs = unpackBool(newValue.Elem(), packedProperty.property)
-				fieldValue.Set(newValue)
-			case reflect.String:
-				newValue := reflect.New(fieldValue.Type().Elem())
-				newErrs = unpackString(newValue.Elem(), packedProperty.property)
-				fieldValue.Set(newValue)
-			default:
-				panic(fmt.Errorf("unexpected pointer kind %s", ptrKind))
-			}
-		case reflect.Struct:
+		if fieldValue.Kind() == reflect.Struct {
 			localFilterKey, localFilterValue := filterKey, filterValue
 			if k, v, err := HasFilter(field.Tag); err != nil {
 				errs = append(errs, err)
@@ -280,66 +260,87 @@ func unpackStructValue(namePrefix string, structValue reflect.Value,
 			}
 			newErrs = unpackStruct(propertyName+".", fieldValue,
 				packedProperty.property, propertyMap, localFilterKey, localFilterValue)
-		default:
-			panic(fmt.Errorf("unexpected kind %s", kind))
+
+			errs = append(errs, newErrs...)
+			if len(errs) >= maxErrors {
+				return errs
+			}
+
+			continue
 		}
-		errs = append(errs, newErrs...)
-		if len(errs) >= maxErrors {
-			return errs
+
+		// Handle basic types and pointers to basic types
+
+		propertyValue, err := propertyToValue(fieldValue.Type(), packedProperty.property)
+		if err != nil {
+			errs = append(errs, err)
+			if len(errs) >= maxErrors {
+				return errs
+			}
 		}
+
+		proptools.ExtendBasicType(fieldValue, propertyValue, proptools.Append)
 	}
 
 	return errs
 }
 
-func unpackBool(boolValue reflect.Value, property *parser.Property) []error {
-	b, ok := property.Value.Eval().(*parser.Bool)
-	if !ok {
-		return []error{
-			fmt.Errorf("%s: can't assign %s value to bool property %q",
-				property.Value.Pos(), property.Value.Type(), property.Name),
-		}
+func propertyToValue(typ reflect.Type, property *parser.Property) (reflect.Value, error) {
+	var value reflect.Value
+
+	var ptr bool
+	if typ.Kind() == reflect.Ptr {
+		typ = typ.Elem()
+		ptr = true
 	}
-	boolValue.SetBool(b.Value)
-	return nil
-}
 
-func unpackString(stringValue reflect.Value,
-	property *parser.Property) []error {
+	switch kind := typ.Kind(); kind {
+	case reflect.Bool:
+		b, ok := property.Value.Eval().(*parser.Bool)
+		if !ok {
+			return value, fmt.Errorf("%s: can't assign %s value to bool property %q",
+				property.Value.Pos(), property.Value.Type(), property.Name)
+		}
+		value = reflect.ValueOf(b.Value)
 
-	s, ok := property.Value.Eval().(*parser.String)
-	if !ok {
-		return []error{
-			fmt.Errorf("%s: can't assign %s value to string property %q",
-				property.Value.Pos(), property.Value.Type(), property.Name),
+	case reflect.String:
+		s, ok := property.Value.Eval().(*parser.String)
+		if !ok {
+			return value, fmt.Errorf("%s: can't assign %s value to string property %q",
+				property.Value.Pos(), property.Value.Type(), property.Name)
 		}
-	}
-	stringValue.SetString(s.Value)
-	return nil
-}
+		value = reflect.ValueOf(s.Value)
 
-func unpackSlice(sliceValue reflect.Value, property *parser.Property) []error {
+	case reflect.Slice:
+		l, ok := property.Value.Eval().(*parser.List)
+		if !ok {
+			return value, fmt.Errorf("%s: can't assign %s value to list property %q",
+				property.Value.Pos(), property.Value.Type(), property.Name)
+		}
 
-	l, ok := property.Value.Eval().(*parser.List)
-	if !ok {
-		return []error{
-			fmt.Errorf("%s: can't assign %s value to list property %q",
-				property.Value.Pos(), property.Value.Type(), property.Name),
+		list := make([]string, len(l.Values))
+		for i, value := range l.Values {
+			s, ok := value.Eval().(*parser.String)
+			if !ok {
+				// The parser should not produce this.
+				panic(fmt.Errorf("non-string value %q found in list", value))
+			}
+			list[i] = s.Value
 		}
+
+		value = reflect.ValueOf(list)
+
+	default:
+		panic(fmt.Errorf("unexpected kind %s", kind))
 	}
 
-	list := make([]string, len(l.Values))
-	for i, value := range l.Values {
-		s, ok := value.Eval().(*parser.String)
-		if !ok {
-			// The parser should not produce this.
-			panic(fmt.Errorf("non-string value %q found in list", value))
-		}
-		list[i] = s.Value
+	if ptr {
+		ptrValue := reflect.New(value.Type())
+		ptrValue.Elem().Set(value)
+		value = ptrValue
 	}
 
-	sliceValue.Set(reflect.ValueOf(list))
-	return nil
+	return value, nil
 }
 
 func unpackStruct(namePrefix string, structValue reflect.Value,
diff --git a/unpack_test.go b/unpack_test.go
index f470356..b65fa3f 100644
--- a/unpack_test.go
+++ b/unpack_test.go
@@ -498,6 +498,49 @@ var validUnpackTestCases = []struct {
 			},
 		},
 	},
+
+	// Factory set properties
+	{
+		input: `
+			m {
+				string: "abc",
+				string_ptr: "abc",
+				bool: false,
+				bool_ptr: false,
+				list: ["a", "b", "c"],
+			}
+		`,
+		output: []interface{}{
+			struct {
+				String     string
+				String_ptr *string
+				Bool       bool
+				Bool_ptr   *bool
+				List       []string
+			}{
+				String:     "012abc",
+				String_ptr: proptools.StringPtr("abc"),
+				Bool:       true,
+				Bool_ptr:   proptools.BoolPtr(false),
+				List:       []string{"0", "1", "2", "a", "b", "c"},
+			},
+		},
+		empty: []interface{}{
+			&struct {
+				String     string
+				String_ptr *string
+				Bool       bool
+				Bool_ptr   *bool
+				List       []string
+			}{
+				String:     "012",
+				String_ptr: proptools.StringPtr("012"),
+				Bool:       true,
+				Bool_ptr:   proptools.BoolPtr(true),
+				List:       []string{"0", "1", "2"},
+			},
+		},
+	},
 }
 
 type EmbeddedStruct struct{ Name string }
diff --git a/visit_test.go b/visit_test.go
index e1768f1..d423c7a 100644
--- a/visit_test.go
+++ b/visit_test.go
@@ -126,7 +126,7 @@ func setupVisitTest(t *testing.T) *Context {
 		t.FailNow()
 	}
 
-	errs = ctx.ResolveDependencies(nil)
+	_, errs = ctx.ResolveDependencies(nil)
 	if len(errs) > 0 {
 		t.Errorf("unexpected dep errors:")
 		for _, err := range errs {