Merge pull request #4 from colincross/bugfixes

Bugfixes

context.go

@@ -1024,6 +1024,30 @@ func (c *Context) updateDependencies() (errs []error) {
 
 	var check func(group *moduleGroup) []*moduleGroup
 
+	cycleError := func(cycle []*moduleGroup) {
+		// We are the "start" of the cycle, so we're responsible
+		// for generating the errors.  The cycle list is in
+		// reverse order because all the 'check' calls append
+		// their own module to the list.
+		errs = append(errs, &Error{
+			Err: fmt.Errorf("encountered dependency cycle:"),
+			Pos: cycle[len(cycle)-1].pos,
+		})
+
+		// Iterate backwards through the cycle list.
+		curGroup := cycle[len(cycle)-1]
+		for i := len(cycle) - 1; i >= 0; i-- {
+			nextGroup := cycle[i]
+			errs = append(errs, &Error{
+				Err: fmt.Errorf("    %q depends on %q",
+					curGroup.properties.Name,
+					nextGroup.properties.Name),
+				Pos: curGroup.propertyPos["deps"],
+			})
+			curGroup = nextGroup
+		}
+	}
+
 	check = func(group *moduleGroup) []*moduleGroup {
 		visited[group] = true
 		checking[group] = true
@@ -1053,23 +1077,7 @@ func (c *Context) updateDependencies() (errs []error) {
 						// for generating the errors.  The cycle list is in
 						// reverse order because all the 'check' calls append
 						// their own module to the list.
-						errs = append(errs, &Error{
-							Err: fmt.Errorf("encountered dependency cycle:"),
-							Pos: group.pos,
-						})
-
-						// Iterate backwards through the cycle list.
-						curGroup := group
-						for i := len(cycle) - 1; i >= 0; i-- {
-							nextGroup := cycle[i]
-							errs = append(errs, &Error{
-								Err: fmt.Errorf("    %q depends on %q",
-									curGroup.properties.Name,
-									nextGroup.properties.Name),
-								Pos: curGroup.propertyPos["deps"],
-							})
-							curGroup = nextGroup
-						}
+						cycleError(cycle)
 
 						// We can continue processing this module's children to
 						// find more cycles.  Since all the modules that were
@@ -1095,7 +1103,10 @@ func (c *Context) updateDependencies() (errs []error) {
 		if !visited[group] {
 			cycle := check(group)
 			if cycle != nil {
-				panic("inconceivable!")
+				if cycle[len(cycle)-1] != group {
+					panic("inconceivable!")
+				}
+				cycleError(cycle)
 			}
 		}
 	}

parser/parser.go

@@ -321,11 +321,11 @@ func evaluateOperator(value1, value2 Value, operator rune, pos scanner.Position)
 
 func addMaps(map1, map2 []*Property, pos scanner.Position) ([]*Property, error) {
 	ret := make([]*Property, 0, len(map1))
-	
+
 	inMap1 := make(map[string]*Property)
 	inMap2 := make(map[string]*Property)
 	inBoth := make(map[string]*Property)
-	
+
 	for _, prop1 := range map1 {
 		inMap1[prop1.Name.Name] = prop1
 	}
@@ -336,7 +336,7 @@ func addMaps(map1, map2 []*Property, pos scanner.Position) ([]*Property, error)
 			inBoth[prop2.Name.Name] = prop2
 		}
 	}
-	
+
 	for _, prop1 := range map1 {
 		if prop2, ok := inBoth[prop1.Name.Name]; ok {
 			var err error
@@ -356,7 +356,7 @@ func addMaps(map1, map2 []*Property, pos scanner.Position) ([]*Property, error)
 			ret = append(ret, prop2)
 		}
 	}
-	
+
 	return ret, nil
 }
 

proptools/proptools.go

@@ -64,10 +64,12 @@ func CopyProperties(dstValue, srcValue reflect.Value) {
 					panic(fmt.Errorf("can't copy field %q: slice elements are "+
 						"not strings", field.Name))
 				}
-				newSlice := reflect.MakeSlice(field.Type, srcFieldValue.Len(),
-					srcFieldValue.Len())
-				dstFieldValue.Set(newSlice)
-				reflect.Copy(dstFieldValue, srcFieldValue)
+				if srcFieldValue != dstFieldValue {
+					newSlice := reflect.MakeSlice(field.Type, srcFieldValue.Len(),
+						srcFieldValue.Len())
+					reflect.Copy(newSlice, srcFieldValue)
+					dstFieldValue.Set(newSlice)
+				}
 			} else {
 				dstFieldValue.Set(srcFieldValue)
 			}