Merge pull request #222 from typesafehub/wip/havocp-resolve-fixes

Rewrite resolver to be more comprehensible

HOCON.md

@@ -825,6 +825,15 @@ working with ordered maps rather than unordered maps, which is too
 constraining. Implementations only have to track order for
 duplicate instances of the same field (i.e. merges).
 
+Implementations must set both `a` and `b` to the same value in
+this case, however. In practice this means that all substitutions
+must be memoized (resolved once, with the result
+retained). Memoization should be keyed by the substitution
+"instance" (the specific occurrence of the `${}` expression)
+rather than by the path inside the `${}` expression, because
+substitutions may be resolved differently depending on their
+position in the file.
+
 ### Includes
 
 #### Include syntax

config/.gitignore

@@ -0,0 +1 @@
+/bin

config/src/main/java/com/typesafe/config/impl/AbstractConfigObject.java

@@ -17,7 +17,7 @@ import com.typesafe.config.ConfigRenderOptions;
 import com.typesafe.config.ConfigValue;
 import com.typesafe.config.ConfigValueType;
 
-abstract class AbstractConfigObject extends AbstractConfigValue implements ConfigObject {
+abstract class AbstractConfigObject extends AbstractConfigValue implements ConfigObject, Container {
 
     final private SimpleConfig config;
 
@@ -56,7 +56,8 @@ abstract class AbstractConfigObject extends AbstractConfigValue implements Confi
     /**
      * This looks up the key with no transformation or type conversion of any
      * kind, and returns null if the key is not present. The object must be
-     * resolved; use attemptPeekWithPartialResolve() if it is not.
+     * resolved along the nodes needed to get the key or
+     * ConfigException.NotResolved will be thrown.
      *
      * @param key
      * @return the unmodified raw value or null
@@ -78,55 +79,23 @@ abstract class AbstractConfigObject extends AbstractConfigValue implements Confi
      *            key to look up
      * @return the value of the key, or null if known not to exist
      * @throws ConfigException.NotResolved
-     *             if can't figure out key's value or can't know whether it
-     *             exists
+     *             if can't figure out key's value (or existence) without more
+     *             resolving
      */
-    protected abstract AbstractConfigValue attemptPeekWithPartialResolve(String key);
+    abstract AbstractConfigValue attemptPeekWithPartialResolve(String key);
 
     /**
-     * Looks up the path with no transformation, type conversion, or exceptions
-     * (just returns null if path not found). Does however resolve the path, if
-     * resolver != null.
-     *
-     * @throws NotPossibleToResolve
-     *             if context is not null and resolution fails
+     * Looks up the path with no transformation or type conversion. Returns null
+     * if the path is not found; throws ConfigException.NotResolved if we need
+     * to go through an unresolved node to look up the path.
      */
-    protected AbstractConfigValue peekPath(Path path, ResolveContext context) throws NotPossibleToResolve {
-        return peekPath(this, path, context);
+    protected AbstractConfigValue peekPath(Path path) {
+        return peekPath(this, path);
     }
 
-    /**
-     * Looks up the path. Doesn't do any resolution, will throw if any is
-     * needed.
-     */
-    AbstractConfigValue peekPath(Path path) {
+    private static AbstractConfigValue peekPath(AbstractConfigObject self, Path path) {
         try {
-            return peekPath(this, path, null);
-        } catch (NotPossibleToResolve e) {
-            throw new ConfigException.BugOrBroken(
-                    "NotPossibleToResolve happened though we had no ResolveContext in peekPath");
-        }
-    }
-
-    // as a side effect, peekPath() will have to resolve all parents of the
-    // child being peeked, but NOT the child itself. Caller has to resolve
-    // the child itself if needed.
-    private static AbstractConfigValue peekPath(AbstractConfigObject self, Path path,
-            ResolveContext context) throws NotPossibleToResolve {
-        try {
-            if (context != null) {
-                // walk down through the path resolving only things along that
-                // path, and then recursively call ourselves with no resolve
-                // context.
-                AbstractConfigValue partiallyResolved = context.restrict(path).resolve(self);
-                if (partiallyResolved instanceof AbstractConfigObject) {
-                    return peekPath((AbstractConfigObject) partiallyResolved, path, null);
-                } else {
-                    throw new ConfigException.BugOrBroken("resolved object to non-object " + self
-                            + " to " + partiallyResolved);
-                }
-            } else {
-                // with no resolver, we'll fail if anything along the path can't
+            // we'll fail if anything along the path can't
             // be looked at without resolving.
             Path next = path.remainder();
             AbstractConfigValue v = self.attemptPeekWithPartialResolve(path.first());
@@ -135,12 +104,11 @@ abstract class AbstractConfigObject extends AbstractConfigValue implements Confi
                 return v;
             } else {
                 if (v instanceof AbstractConfigObject) {
-                        return peekPath((AbstractConfigObject) v, next, null);
+                    return peekPath((AbstractConfigObject) v, next);
                 } else {
                     return null;
                 }
             }
-            }
         } catch (ConfigException.NotResolved e) {
             throw ConfigImpl.improveNotResolved(path, e);
         }
@@ -209,7 +177,9 @@ abstract class AbstractConfigObject extends AbstractConfigValue implements Confi
     }
 
     @Override
-    abstract AbstractConfigObject resolveSubstitutions(ResolveContext context) throws NotPossibleToResolve;
+    abstract ResolveResult<? extends AbstractConfigObject> resolveSubstitutions(ResolveContext context,
+            ResolveSource source)
+            throws NotPossibleToResolve;
 
     @Override
     abstract AbstractConfigObject relativized(final Path prefix);

config/src/main/java/com/typesafe/config/impl/AbstractConfigValue.java

@@ -68,17 +68,51 @@ abstract class AbstractConfigValue implements ConfigValue, MergeableValue {
      *
      * @param context
      *            state of the current resolve
+     * @param source
+     *            where to look up values
      * @return a new value if there were changes, or this if no changes
      */
-    AbstractConfigValue resolveSubstitutions(ResolveContext context)
+    ResolveResult<? extends AbstractConfigValue> resolveSubstitutions(ResolveContext context, ResolveSource source)
             throws NotPossibleToResolve {
-        return this;
+        return ResolveResult.make(context, this);
     }
 
     ResolveStatus resolveStatus() {
         return ResolveStatus.RESOLVED;
     }
 
+    protected static List<AbstractConfigValue> replaceChildInList(List<AbstractConfigValue> list,
+            AbstractConfigValue child, AbstractConfigValue replacement) {
+        int i = 0;
+        while (i < list.size() && list.get(i) != child)
+            ++i;
+        if (i == list.size())
+            throw new ConfigException.BugOrBroken("tried to replace " + child + " which is not in " + list);
+        List<AbstractConfigValue> newStack = new ArrayList<AbstractConfigValue>(list);
+        if (replacement != null)
+            newStack.set(i, replacement);
+        else
+            newStack.remove(i);
+
+        if (newStack.isEmpty())
+            return null;
+        else
+            return newStack;
+    }
+
+    protected static boolean hasDescendantInList(List<AbstractConfigValue> list, AbstractConfigValue descendant) {
+        for (AbstractConfigValue v : list) {
+            if (v == descendant)
+                return true;
+        }
+        // now the expensive traversal
+        for (AbstractConfigValue v : list) {
+            if (v instanceof Container && ((Container) v).hasDescendant(descendant))
+                return true;
+        }
+        return false;
+    }
+
     /**
      * This is used when including one file in another; the included file is
      * relativized to the path it's included into in the parent file. The point

config/src/main/java/com/typesafe/config/impl/ConfigConcatenation.java

@@ -22,7 +22,7 @@ import com.typesafe.config.ConfigValueType;
  * concatenations of objects, but ConfigDelayedMerge should be used for that
  * since a concat of objects really will merge, not concatenate.
  */
-final class ConfigConcatenation extends AbstractConfigValue implements Unmergeable {
+final class ConfigConcatenation extends AbstractConfigValue implements Unmergeable, Container {
 
     final private List<AbstractConfigValue> pieces;
 
@@ -170,12 +170,35 @@ final class ConfigConcatenation extends AbstractConfigValue implements Unmergeab
     }
 
     @Override
-    AbstractConfigValue resolveSubstitutions(ResolveContext context) throws NotPossibleToResolve {
+    ResolveResult<? extends AbstractConfigValue> resolveSubstitutions(ResolveContext context, ResolveSource source)
+            throws NotPossibleToResolve {
+        if (ConfigImpl.traceSubstitutionsEnabled()) {
+            int indent = context.depth() + 2;
+            ConfigImpl.trace(indent - 1, "concatenation has " + pieces.size() + " pieces:");
+            int count = 0;
+            for (AbstractConfigValue v : pieces) {
+                ConfigImpl.trace(indent, count + ": " + v);
+                count += 1;
+            }
+        }
+
+        // Right now there's no reason to pushParent here because the
+        // content of ConfigConcatenation should not need to replaceChild,
+        // but if it did we'd have to do this.
+        ResolveSource sourceWithParent = source; // .pushParent(this);
+        ResolveContext newContext = context;
+
         List<AbstractConfigValue> resolved = new ArrayList<AbstractConfigValue>(pieces.size());
         for (AbstractConfigValue p : pieces) {
             // to concat into a string we have to do a full resolve,
-            // so unrestrict the context
-            AbstractConfigValue r = context.unrestricted().resolve(p);
+            // so unrestrict the context, then put restriction back afterward
+            Path restriction = newContext.restrictToChild();
+            ResolveResult<? extends AbstractConfigValue> result = newContext.unrestricted()
+                    .resolve(p, sourceWithParent);
+            AbstractConfigValue r = result.value;
+            newContext = result.context.restrict(restriction);
+            if (ConfigImpl.traceSubstitutionsEnabled())
+                ConfigImpl.trace(context.depth(), "resolved concat piece to " + r);
             if (r == null) {
                 // it was optional... omit
             } else {
@@ -188,11 +211,12 @@ final class ConfigConcatenation extends AbstractConfigValue implements Unmergeab
         // if unresolved is allowed we can just become another
         // ConfigConcatenation
         if (joined.size() > 1 && context.options().getAllowUnresolved())
-            return new ConfigConcatenation(this.origin(), joined);
+            return ResolveResult.make(newContext, new ConfigConcatenation(this.origin(), joined));
         else if (joined.isEmpty())
-            return null; // we had just a list of optional references using ${?}
+            // we had just a list of optional references using ${?}
+            return ResolveResult.make(newContext, null);
         else if (joined.size() == 1)
-            return joined.get(0);
+            return ResolveResult.make(newContext, joined.get(0));
         else
             throw new ConfigException.BugOrBroken("Bug in the library; resolved list was joined to too many values: "
                     + joined);
@@ -203,6 +227,20 @@ final class ConfigConcatenation extends AbstractConfigValue implements Unmergeab
         return ResolveStatus.UNRESOLVED;
     }
 
+    @Override
+    public ConfigConcatenation replaceChild(AbstractConfigValue child, AbstractConfigValue replacement) {
+        List<AbstractConfigValue> newPieces = replaceChildInList(pieces, child, replacement);
+        if (newPieces == null)
+            return null;
+        else
+            return new ConfigConcatenation(origin(), newPieces);
+    }
+
+    @Override
+    public boolean hasDescendant(AbstractConfigValue descendant) {
+        return hasDescendantInList(pieces, descendant);
+    }
+
     // when you graft a substitution into another object,
     // you have to prefix it with the location in that object
     // where you grafted it; but save prefixLength so
@@ -244,19 +282,4 @@ final class ConfigConcatenation extends AbstractConfigValue implements Unmergeab
             p.render(sb, indent, atRoot, options);
         }
     }
-
-    static List<AbstractConfigValue> valuesFromPieces(ConfigOrigin origin, List<Object> pieces) {
-        List<AbstractConfigValue> values = new ArrayList<AbstractConfigValue>(pieces.size());
-        for (Object p : pieces) {
-            if (p instanceof SubstitutionExpression) {
-                values.add(new ConfigReference(origin, (SubstitutionExpression) p));
-            } else if (p instanceof String) {
-                values.add(new ConfigString(origin, (String) p));
-            } else {
-                throw new ConfigException.BugOrBroken("Unexpected piece " + p);
-            }
-        }
-
-        return values;
-    }
 }

config/src/main/java/com/typesafe/config/impl/ConfigDelayedMerge.java

@@ -54,82 +54,117 @@ final class ConfigDelayedMerge extends AbstractConfigValue implements Unmergeabl
     }
 
     @Override
-    AbstractConfigValue resolveSubstitutions(ResolveContext context)
+    ResolveResult<? extends AbstractConfigValue> resolveSubstitutions(ResolveContext context, ResolveSource source)
             throws NotPossibleToResolve {
-        return resolveSubstitutions(this, stack, context);
+        return resolveSubstitutions(this, stack, context, source);
     }
 
     // static method also used by ConfigDelayedMergeObject
-    static AbstractConfigValue resolveSubstitutions(ReplaceableMergeStack replaceable,
-            List<AbstractConfigValue> stack, ResolveContext context) throws NotPossibleToResolve {
+    static ResolveResult<? extends AbstractConfigValue> resolveSubstitutions(ReplaceableMergeStack replaceable,
+            List<AbstractConfigValue> stack,
+            ResolveContext context, ResolveSource source) throws NotPossibleToResolve {
+        if (ConfigImpl.traceSubstitutionsEnabled()) {
+            ConfigImpl.trace(context.depth(), "delayed merge stack has " + stack.size() + " items:");
+            int count = 0;
+            for (AbstractConfigValue v : stack) {
+                ConfigImpl.trace(context.depth() + 1, count + ": " + v);
+                count += 1;
+            }
+        }
+
         // to resolve substitutions, we need to recursively resolve
         // the stack of stuff to merge, and merge the stack so
         // we won't be a delayed merge anymore. If restrictToChildOrNull
-        // is non-null, we may remain a delayed merge though.
+        // is non-null, or resolve options allow partial resolves,
+        // we may remain a delayed merge though.
 
+        ResolveContext newContext = context;
         int count = 0;
         AbstractConfigValue merged = null;
-        for (AbstractConfigValue v : stack) {
-            if (v instanceof ReplaceableMergeStack)
-                throw new ConfigException.BugOrBroken(
-                        "A delayed merge should not contain another one: " + replaceable);
+        for (AbstractConfigValue end : stack) {
+            // the end value may or may not be resolved already
 
-            boolean replaced = false;
-            // we only replace if we have a substitution, or
-            // value-concatenation containing one. The Unmergeable
-            // here isn't a delayed merge stack since we can't contain
-            // another stack (see assertion above).
-            if (v instanceof Unmergeable) {
-                // If, while resolving 'v' we come back to the same
-                // merge stack, we only want to look _below_ 'v'
+            ResolveSource sourceForEnd;
+
+            if (end instanceof ReplaceableMergeStack)
+                throw new ConfigException.BugOrBroken("A delayed merge should not contain another one: " + replaceable);
+            else if (end instanceof Unmergeable) {
+                // the remainder could be any kind of value, including another
+                // ConfigDelayedMerge
+                AbstractConfigValue remainder = replaceable.makeReplacement(context, count + 1);
+
+                if (ConfigImpl.traceSubstitutionsEnabled())
+                    ConfigImpl.trace(newContext.depth(), "remainder portion: " + remainder);
+
+                // If, while resolving 'end' we come back to the same
+                // merge stack, we only want to look _below_ 'end'
                 // in the stack. So we arrange to replace the
                 // ConfigDelayedMerge with a value that is only
                 // the remainder of the stack below this one.
 
-                context.source().replace((AbstractConfigValue) replaceable,
-                        replaceable.makeReplacer(count + 1));
-                replaced = true;
+                if (ConfigImpl.traceSubstitutionsEnabled())
+                    ConfigImpl.trace(newContext.depth(), "building sourceForEnd");
+
+                // we resetParents() here because we'll be resolving "end"
+                // against a root which does NOT contain "end"
+                sourceForEnd = source.replaceWithinCurrentParent((AbstractConfigValue) replaceable, remainder);
+
+                if (ConfigImpl.traceSubstitutionsEnabled())
+                    ConfigImpl.trace(newContext.depth(), "  sourceForEnd before reset parents but after replace: "
+                            + sourceForEnd);
+
+                sourceForEnd = sourceForEnd.resetParents();
+            } else {
+                if (ConfigImpl.traceSubstitutionsEnabled())
+                    ConfigImpl.trace(newContext.depth(),
+                            "will resolve end against the original source with parent pushed");
+
+                sourceForEnd = source.pushParent(replaceable);
             }
 
-            AbstractConfigValue resolved;
-            try {
-                resolved = context.resolve(v);
-            } finally {
-                if (replaced)
-                    context.source().unreplace((AbstractConfigValue) replaceable);
+            if (ConfigImpl.traceSubstitutionsEnabled()) {
+                ConfigImpl.trace(newContext.depth(), "sourceForEnd      =" + sourceForEnd);
             }
 
-            if (resolved != null) {
-                if (merged == null)
-                    merged = resolved;
-                else
-                    merged = merged.withFallback(resolved);
+            if (ConfigImpl.traceSubstitutionsEnabled())
+                ConfigImpl.trace(newContext.depth(), "Resolving highest-priority item in delayed merge " + end
+                        + " against " + sourceForEnd + " endWasRemoved=" + (source != sourceForEnd));
+            ResolveResult<? extends AbstractConfigValue> result = newContext.resolve(end, sourceForEnd);
+            AbstractConfigValue resolvedEnd = result.value;
+            newContext = result.context;
+
+            if (resolvedEnd != null) {
+                if (merged == null) {
+                    merged = resolvedEnd;
+                } else {
+                    if (ConfigImpl.traceSubstitutionsEnabled())
+                        ConfigImpl.trace(newContext.depth() + 1, "merging " + merged + " with fallback " + resolvedEnd);
+                    merged = merged.withFallback(resolvedEnd);
                 }
+            }
+
             count += 1;
+
+            if (ConfigImpl.traceSubstitutionsEnabled())
+                ConfigImpl.trace(newContext.depth(), "stack merged, yielding: " + merged);
         }
 
-        return merged;
+        return ResolveResult.make(newContext, merged);
     }
 
     @Override
-    public ResolveReplacer makeReplacer(final int skipping) {
-        return new ResolveReplacer() {
-            @Override
-            protected AbstractConfigValue makeReplacement(ResolveContext context)
-                    throws NotPossibleToResolve {
+    public AbstractConfigValue makeReplacement(ResolveContext context, int skipping) {
         return ConfigDelayedMerge.makeReplacement(context, stack, skipping);
     }
-        };
-    }
-
-    // static method also used by ConfigDelayedMergeObject
-    static AbstractConfigValue makeReplacement(ResolveContext context,
-            List<AbstractConfigValue> stack, int skipping) throws NotPossibleToResolve {
 
+    // static method also used by ConfigDelayedMergeObject; end may be null
+    static AbstractConfigValue makeReplacement(ResolveContext context, List<AbstractConfigValue> stack, int skipping) {
         List<AbstractConfigValue> subStack = stack.subList(skipping, stack.size());
 
         if (subStack.isEmpty()) {
-            throw new NotPossibleToResolve(context);
+            if (ConfigImpl.traceSubstitutionsEnabled())
+                ConfigImpl.trace(context.depth(), "Nothing else in the merge stack, replacing with null");
+            return null;
         } else {
             // generate a new merge stack from only the remaining items
             AbstractConfigValue merged = null;
@@ -148,6 +183,20 @@ final class ConfigDelayedMerge extends AbstractConfigValue implements Unmergeabl
         return ResolveStatus.UNRESOLVED;
     }
 
+    @Override
+    public AbstractConfigValue replaceChild(AbstractConfigValue child, AbstractConfigValue replacement) {
+        List<AbstractConfigValue> newStack = replaceChildInList(stack, child, replacement);
+        if (newStack == null)
+            return null;
+        else
+            return new ConfigDelayedMerge(origin(), newStack);
+    }
+
+    @Override
+    public boolean hasDescendant(AbstractConfigValue descendant) {
+        return hasDescendantInList(stack, descendant);
+    }
+
     @Override
     ConfigDelayedMerge relativized(Path prefix) {
         List<AbstractConfigValue> newStack = new ArrayList<AbstractConfigValue>();
@@ -203,7 +252,8 @@ final class ConfigDelayedMerge extends AbstractConfigValue implements Unmergeabl
         // note that "origin" is deliberately NOT part of equality
         if (other instanceof ConfigDelayedMerge) {
             return canEqual(other)
-                    && this.stack.equals(((ConfigDelayedMerge) other).stack);
+                    && (this.stack == ((ConfigDelayedMerge) other).stack || this.stack
+                            .equals(((ConfigDelayedMerge) other).stack));
         } else {
             return false;
         }

config/src/main/java/com/typesafe/config/impl/ConfigDelayedMergeObject.java

@@ -50,33 +50,37 @@ final class ConfigDelayedMergeObject extends AbstractConfigObject implements Unm
     }
 
     @Override
-    AbstractConfigObject resolveSubstitutions(ResolveContext context)
+    ResolveResult<? extends AbstractConfigObject> resolveSubstitutions(ResolveContext context, ResolveSource source)
             throws NotPossibleToResolve {
-        AbstractConfigValue merged = ConfigDelayedMerge.resolveSubstitutions(this, stack, context);
-        if (merged instanceof AbstractConfigObject) {
-            return (AbstractConfigObject) merged;
-        } else {
-            throw new ConfigException.BugOrBroken(
-                    "somehow brokenly merged an object and didn't get an object, got " + merged);
-        }
+        ResolveResult<? extends AbstractConfigValue> merged = ConfigDelayedMerge.resolveSubstitutions(this, stack,
+                context, source);
+        return merged.asObjectResult();
     }
 
     @Override
-    public ResolveReplacer makeReplacer(final int skipping) {
-        return new ResolveReplacer() {
-            @Override
-            protected AbstractConfigValue makeReplacement(ResolveContext context)
-                    throws NotPossibleToResolve {
+    public AbstractConfigValue makeReplacement(ResolveContext context, int skipping) {
         return ConfigDelayedMerge.makeReplacement(context, stack, skipping);
     }
-        };
-    }
 
     @Override
     ResolveStatus resolveStatus() {
         return ResolveStatus.UNRESOLVED;
     }
 
+    @Override
+    public AbstractConfigValue replaceChild(AbstractConfigValue child, AbstractConfigValue replacement) {
+        List<AbstractConfigValue> newStack = replaceChildInList(stack, child, replacement);
+        if (newStack == null)
+            return null;
+        else
+            return new ConfigDelayedMergeObject(origin(), newStack);
+    }
+
+    @Override
+    public boolean hasDescendant(AbstractConfigValue descendant) {
+        return hasDescendantInList(stack, descendant);
+    }
+
     @Override
     ConfigDelayedMergeObject relativized(Path prefix) {
         List<AbstractConfigValue> newStack = new ArrayList<AbstractConfigValue>();
@@ -165,8 +169,8 @@ final class ConfigDelayedMergeObject extends AbstractConfigObject implements Unm
         // note that "origin" is deliberately NOT part of equality
         if (other instanceof ConfigDelayedMergeObject) {
             return canEqual(other)
-                    && this.stack
-                            .equals(((ConfigDelayedMergeObject) other).stack);
+                    && (this.stack == ((ConfigDelayedMergeObject) other).stack || this.stack
+                            .equals(((ConfigDelayedMergeObject) other).stack));
         } else {
             return false;
         }

config/src/main/java/com/typesafe/config/impl/ConfigImpl.java

@@ -377,10 +377,12 @@ public class ConfigImpl {
 
     private static class DebugHolder {
         private static String LOADS = "loads";
+        private static String SUBSTITUTIONS = "substitutions";
 
         private static Map<String, Boolean> loadDiagnostics() {
             Map<String, Boolean> result = new HashMap<String, Boolean>();
             result.put(LOADS, false);
+            result.put(SUBSTITUTIONS, false);
 
             // People do -Dconfig.trace=foo,bar to enable tracing of different things
             String s = System.getProperty("config.trace");
@@ -391,6 +393,8 @@ public class ConfigImpl {
                 for (String k : keys) {
                     if (k.equals(LOADS)) {
                         result.put(LOADS, true);
+                    } else if (k.equals(SUBSTITUTIONS)) {
+                        result.put(SUBSTITUTIONS, true);
                     } else {
                         System.err.println("config.trace property contains unknown trace topic '"
                                 + k + "'");
@@ -403,10 +407,15 @@ public class ConfigImpl {
         private static final Map<String, Boolean> diagnostics = loadDiagnostics();
 
         private static final boolean traceLoadsEnabled = diagnostics.get(LOADS);
+        private static final boolean traceSubstitutionsEnabled = diagnostics.get(SUBSTITUTIONS);
 
         static boolean traceLoadsEnabled() {
             return traceLoadsEnabled;
         }
+
+        static boolean traceSubstitutionsEnabled() {
+            return traceSubstitutionsEnabled;
+        }
     }
 
     /** For use ONLY by library internals, DO NOT TOUCH not guaranteed ABI */
@@ -418,10 +427,26 @@ public class ConfigImpl {
         }
     }
 
+    public static boolean traceSubstitutionsEnabled() {
+        try {
+            return DebugHolder.traceSubstitutionsEnabled();
+        } catch (ExceptionInInitializerError e) {
+            throw ConfigImplUtil.extractInitializerError(e);
+        }
+    }
+
     public static void trace(String message) {
         System.err.println(message);
     }
 
+    public static void trace(int indentLevel, String message) {
+        while (indentLevel > 0) {
+            System.err.print("  ");
+            indentLevel -= 1;
+        }
+        System.err.println(message);
+    }
+
     // the basic idea here is to add the "what" and have a canonical
     // toplevel error message. the "original" exception may however have extra
     // detail about what happened. call this if you have a better "what" than

config/src/main/java/com/typesafe/config/impl/ConfigReference.java

@@ -65,31 +65,49 @@ final class ConfigReference extends AbstractConfigValue implements Unmergeable {
     // This way it's impossible for NotPossibleToResolve to "escape" since
     // any failure to resolve has to start with a ConfigReference.
     @Override
-    AbstractConfigValue resolveSubstitutions(ResolveContext context) {
-        context.source().replace(this, ResolveReplacer.cycleResolveReplacer);
-        try {
+    ResolveResult<? extends AbstractConfigValue> resolveSubstitutions(ResolveContext context, ResolveSource source) {
+        ResolveContext newContext = context.addCycleMarker(this);
         AbstractConfigValue v;
         try {
-                v = context.source().lookupSubst(context, expr, prefixLength);
+            ResolveSource.ResultWithPath resultWithPath = source.lookupSubst(newContext, expr, prefixLength);
+            newContext = resultWithPath.result.context;
+
+            if (resultWithPath.result.value != null) {
+                if (ConfigImpl.traceSubstitutionsEnabled())
+                    ConfigImpl.trace(newContext.depth(), "recursively resolving " + resultWithPath
+                            + " which was the resolution of " + expr + " against " + source);
+
+                ResolveSource recursiveResolveSource = (new ResolveSource(
+                        (AbstractConfigObject) resultWithPath.pathFromRoot.last(), resultWithPath.pathFromRoot));
+
+                if (ConfigImpl.traceSubstitutionsEnabled())
+                    ConfigImpl.trace(newContext.depth(), "will recursively resolve against " + recursiveResolveSource);
+
+                ResolveResult<? extends AbstractConfigValue> result = newContext.resolve(resultWithPath.result.value,
+                        recursiveResolveSource);
+                v = result.value;
+                newContext = result.context;
+            } else {
+                v = null;
+            }
         } catch (NotPossibleToResolve e) {
+            if (ConfigImpl.traceSubstitutionsEnabled())
+                ConfigImpl.trace(newContext.depth(),
+                        "not possible to resolve " + expr + ", cycle involved: " + e.traceString());
             if (expr.optional())
                 v = null;
             else
                 throw new ConfigException.UnresolvedSubstitution(origin(), expr
-                            + " was part of a cycle of substitutions involving " + e.traceString(),
-                            e);
+                        + " was part of a cycle of substitutions involving " + e.traceString(), e);
         }
 
         if (v == null && !expr.optional()) {
-                if (context.options().getAllowUnresolved())
-                    return this;
+            if (newContext.options().getAllowUnresolved())
+                return ResolveResult.make(newContext.removeCycleMarker(this), this);
             else
                 throw new ConfigException.UnresolvedSubstitution(origin(), expr.toString());
         } else {
-                return v;
-            }
-        } finally {
-            context.source().unreplace(this);
+            return ResolveResult.make(newContext.removeCycleMarker(this), v);
         }
     }
 

config/src/main/java/com/typesafe/config/impl/Container.java

@@ -0,0 +1,25 @@
+/**
+ *   Copyright (C) 2014 Typesafe Inc. <http://typesafe.com>
+ */
+package com.typesafe.config.impl;
+
+/**
+ * An AbstractConfigValue which contains other values. Java has no way to
+ * express "this has to be an AbstractConfigValue also" other than making
+ * AbstractConfigValue an interface which would be aggravating. But we can say
+ * we are a ConfigValue.
+ */
+interface Container extends com.typesafe.config.ConfigValue {
+    /**
+     * Replace a child of this value. CAUTION if replacement is null, delete the
+     * child, which may also delete the parent, or make the parent into a
+     * non-container.
+     */
+    AbstractConfigValue replaceChild(AbstractConfigValue child, AbstractConfigValue replacement);
+
+    /**
+     * Super-expensive full traversal to see if descendant is anywhere
+     * underneath this container.
+     */
+    boolean hasDescendant(AbstractConfigValue descendant);
+}

config/src/main/java/com/typesafe/config/impl/MemoKey.java

@@ -36,4 +36,9 @@ final class MemoKey {
             return false;
         }
     }
+
+    @Override
+    public final String toString() {
+        return "MemoKey(" + value + "@" + System.identityHashCode(value) + "," + restrictToChildOrNull + ")";
+    }
 }

config/src/main/java/com/typesafe/config/impl/ReplaceableMergeStack.java

@@ -5,10 +5,10 @@ package com.typesafe.config.impl;
  * that replaces itself during substitution resolution in order to implement
  * "look backwards only" semantics.
  */
-interface ReplaceableMergeStack {
+interface ReplaceableMergeStack extends Container {
     /**
-     * Make a replacer for this object, skipping the given number of items in
-     * the stack.
+     * Make a replacement for this object skipping the given number of elements
+     * which are lower in merge priority.
      */
-    ResolveReplacer makeReplacer(int skipping);
+    AbstractConfigValue makeReplacement(ResolveContext context, int skipping);
 }

config/src/main/java/com/typesafe/config/impl/ResolveContext.java

@@ -1,19 +1,16 @@
 package com.typesafe.config.impl;
 
+import java.util.Collections;
+import java.util.IdentityHashMap;
 import java.util.List;
 import java.util.ArrayList;
+import java.util.Set;
 
 import com.typesafe.config.ConfigException;
 import com.typesafe.config.ConfigResolveOptions;
 import com.typesafe.config.impl.AbstractConfigValue.NotPossibleToResolve;
 
 final class ResolveContext {
-    // this is unfortunately mutable so should only be shared among
-    // ResolveContext in the same traversal.
-    final private ResolveSource source;
-
-    // this is unfortunately mutable so should only be shared among
-    // ResolveContext in the same traversal.
     final private ResolveMemos memos;
 
     final private ConfigResolveOptions options;
@@ -24,29 +21,57 @@ final class ResolveContext {
     // CAN BE NULL for a full resolve.
     final private Path restrictToChild;
 
-    // another mutable unfortunate. This is
-    // used to make nice error messages when
-    // resolution fails.
-    final private List<SubstitutionExpression> expressionTrace;
+    // This is used for tracing and debugging and nice error messages;
+    // contains every node as we call resolve on it.
+    final private List<AbstractConfigValue> resolveStack;
 
-    ResolveContext(ResolveSource source, ResolveMemos memos, ConfigResolveOptions options,
-            Path restrictToChild, List<SubstitutionExpression> expressionTrace) {
-        this.source = source;
+    final private Set<AbstractConfigValue> cycleMarkers;
+
+    ResolveContext(ResolveMemos memos, ConfigResolveOptions options, Path restrictToChild,
+            List<AbstractConfigValue> resolveStack, Set<AbstractConfigValue> cycleMarkers) {
         this.memos = memos;
         this.options = options;
         this.restrictToChild = restrictToChild;
-        this.expressionTrace = expressionTrace;
+        this.resolveStack = Collections.unmodifiableList(resolveStack);
+        this.cycleMarkers = Collections.unmodifiableSet(cycleMarkers);
     }
 
-    ResolveContext(AbstractConfigObject root, ConfigResolveOptions options, Path restrictToChild) {
+    private static Set<AbstractConfigValue> newCycleMarkers() {
+        return Collections.newSetFromMap(new IdentityHashMap<AbstractConfigValue, Boolean>());
+    }
+
+    ResolveContext(ConfigResolveOptions options, Path restrictToChild) {
         // LinkedHashSet keeps the traversal order which is at least useful
         // in error messages if nothing else
-        this(new ResolveSource(root), new ResolveMemos(), options, restrictToChild,
-                new ArrayList<SubstitutionExpression>());
+        this(new ResolveMemos(), options, restrictToChild, new ArrayList<AbstractConfigValue>(), newCycleMarkers());
+        if (ConfigImpl.traceSubstitutionsEnabled())
+            ConfigImpl.trace(depth(), "ResolveContext restrict to child " + restrictToChild);
     }
 
-    ResolveSource source() {
-        return source;
+    ResolveContext addCycleMarker(AbstractConfigValue value) {
+        if (ConfigImpl.traceSubstitutionsEnabled())
+            ConfigImpl.trace(depth(), "++ Cycle marker " + value + "@" + System.identityHashCode(value));
+        if (cycleMarkers.contains(value))
+            throw new ConfigException.BugOrBroken("Added cycle marker twice " + value);
+        Set<AbstractConfigValue> copy = newCycleMarkers();
+        copy.addAll(cycleMarkers);
+        copy.add(value);
+        return new ResolveContext(memos, options, restrictToChild, resolveStack, copy);
+    }
+
+    ResolveContext removeCycleMarker(AbstractConfigValue value) {
+        if (ConfigImpl.traceSubstitutionsEnabled())
+            ConfigImpl.trace(depth(), "-- Cycle marker " + value + "@" + System.identityHashCode(value));
+
+        Set<AbstractConfigValue> copy = newCycleMarkers();
+        copy.addAll(cycleMarkers);
+        copy.remove(value);
+        return new ResolveContext(memos, options, restrictToChild, resolveStack, copy);
+    }
+
+    private ResolveContext memoize(MemoKey key, AbstractConfigValue value) {
+        ResolveMemos changed = memos.put(key, value);
+        return new ResolveContext(changed, options, restrictToChild, resolveStack, cycleMarkers);
     }
 
     ConfigResolveOptions options() {
@@ -61,38 +86,64 @@ final class ResolveContext {
         return restrictToChild;
     }
 
+    // restrictTo may be null to unrestrict
     ResolveContext restrict(Path restrictTo) {
         if (restrictTo == restrictToChild)
             return this;
         else
-            return new ResolveContext(source, memos, options, restrictTo, expressionTrace);
+            return new ResolveContext(memos, options, restrictTo, resolveStack, cycleMarkers);
     }
 
     ResolveContext unrestricted() {
         return restrict(null);
     }
 
-    void trace(SubstitutionExpression expr) {
-        expressionTrace.add(expr);
-    }
-
-    void untrace() {
-        expressionTrace.remove(expressionTrace.size() - 1);
-    }
-
     String traceString() {
         String separator = ", ";
         StringBuilder sb = new StringBuilder();
-        for (SubstitutionExpression expr : expressionTrace) {
-            sb.append(expr.toString());
+        for (AbstractConfigValue value : resolveStack) {
+            if (value instanceof ConfigReference) {
+                sb.append(((ConfigReference) value).expression().toString());
                 sb.append(separator);
             }
+        }
         if (sb.length() > 0)
             sb.setLength(sb.length() - separator.length());
         return sb.toString();
     }
 
-    AbstractConfigValue resolve(AbstractConfigValue original) throws NotPossibleToResolve {
+    private ResolveContext pushTrace(AbstractConfigValue value) {
+        if (ConfigImpl.traceSubstitutionsEnabled())
+            ConfigImpl.trace(depth(), "pushing trace " + value);
+        List<AbstractConfigValue> copy = new ArrayList<AbstractConfigValue>(resolveStack);
+        copy.add(value);
+        return new ResolveContext(memos, options, restrictToChild, copy, cycleMarkers);
+    }
+
+    ResolveContext popTrace() {
+        List<AbstractConfigValue> copy = new ArrayList<AbstractConfigValue>(resolveStack);
+        AbstractConfigValue old = copy.remove(resolveStack.size() - 1);
+        if (ConfigImpl.traceSubstitutionsEnabled())
+            ConfigImpl.trace(depth() - 1, "popped trace " + old);
+        return new ResolveContext(memos, options, restrictToChild, copy, cycleMarkers);
+    }
+
+    int depth() {
+        if (resolveStack.size() > 30)
+            throw new ConfigException.BugOrBroken("resolve getting too deep");
+        return resolveStack.size();
+    }
+
+    ResolveResult<? extends AbstractConfigValue> resolve(AbstractConfigValue original, ResolveSource source)
+            throws NotPossibleToResolve {
+        if (ConfigImpl.traceSubstitutionsEnabled())
+            ConfigImpl
+                    .trace(depth(), "resolving " + original + " restrictToChild=" + restrictToChild + " in " + source);
+        return pushTrace(original).realResolve(original, source).popTrace();
+    }
+
+    private ResolveResult<? extends AbstractConfigValue> realResolve(AbstractConfigValue original, ResolveSource source)
+            throws NotPossibleToResolve {
         // a fully-resolved (no restrictToChild) object can satisfy a
         // request for a restricted object, so always check that first.
         final MemoKey fullKey = new MemoKey(original, null);
@@ -109,16 +160,40 @@ final class ResolveContext {
         }
 
         if (cached != null) {
-            return cached;
+            if (ConfigImpl.traceSubstitutionsEnabled())
+                ConfigImpl.trace(depth(), "using cached resolution " + cached + " for " + original
+                        + " restrictToChild " + restrictToChild());
+            return ResolveResult.make(this, cached);
         } else {
-            AbstractConfigValue resolved = source.resolveCheckingReplacement(this, original);
+            if (ConfigImpl.traceSubstitutionsEnabled())
+                ConfigImpl.trace(depth(),
+                        "not found in cache, resolving " + original + "@" + System.identityHashCode(original));
+
+            if (cycleMarkers.contains(original)) {
+                if (ConfigImpl.traceSubstitutionsEnabled())
+                    ConfigImpl.trace(depth(),
+                            "Cycle detected, can't resolve; " + original + "@" + System.identityHashCode(original));
+                throw new NotPossibleToResolve(this);
+            }
+
+            ResolveResult<? extends AbstractConfigValue> result = original.resolveSubstitutions(this, source);
+            AbstractConfigValue resolved = result.value;
+
+            if (ConfigImpl.traceSubstitutionsEnabled())
+                ConfigImpl.trace(depth(), "resolved to " + resolved + "@" + System.identityHashCode(resolved)
+                        + " from " + original + "@" + System.identityHashCode(resolved));
+
+            ResolveContext withMemo = result.context;
 
             if (resolved == null || resolved.resolveStatus() == ResolveStatus.RESOLVED) {
                 // if the resolved object is fully resolved by resolving
                 // only the restrictToChildOrNull, then it can be cached
                 // under fullKey since the child we were restricted to
                 // turned out to be the only unresolved thing.
-                memos.put(fullKey, resolved);
+                if (ConfigImpl.traceSubstitutionsEnabled())
+                    ConfigImpl.trace(depth(), "caching " + fullKey + " result " + resolved);
+
+                withMemo = withMemo.memoize(fullKey, resolved);
             } else {
                 // if we have an unresolved object then either we did a
                 // partial resolve restricted to a certain child, or we are
@@ -128,25 +203,32 @@ final class ResolveContext {
                         throw new ConfigException.BugOrBroken(
                                 "restrictedKey should not be null here");
                     }
-                    memos.put(restrictedKey, resolved);
+                    if (ConfigImpl.traceSubstitutionsEnabled())
+                        ConfigImpl.trace(depth(), "caching " + restrictedKey + " result " + resolved);
+
+                    withMemo = withMemo.memoize(restrictedKey, resolved);
                 } else if (options().getAllowUnresolved()) {
-                    memos.put(fullKey, resolved);
+                    if (ConfigImpl.traceSubstitutionsEnabled())
+                        ConfigImpl.trace(depth(), "caching " + fullKey + " result " + resolved);
+
+                    withMemo = withMemo.memoize(fullKey, resolved);
                 } else {
                     throw new ConfigException.BugOrBroken(
                             "resolveSubstitutions() did not give us a resolved object");
                 }
             }
 
-            return resolved;
+            return ResolveResult.make(withMemo, resolved);
         }
     }
 
     static AbstractConfigValue resolve(AbstractConfigValue value, AbstractConfigObject root,
             ConfigResolveOptions options) {
-        ResolveContext context = new ResolveContext(root, options, null /* restrictToChild */);
+        ResolveSource source = new ResolveSource(root);
+        ResolveContext context = new ResolveContext(options, null /* restrictToChild */);
 
         try {
-            return context.resolve(value);
+            return context.resolve(value, source).value;
         } catch (NotPossibleToResolve e) {
             // ConfigReference was supposed to catch NotPossibleToResolve
             throw new ConfigException.BugOrBroken(

config/src/main/java/com/typesafe/config/impl/ResolveMemos.java

@@ -13,15 +13,23 @@ final class ResolveMemos {
     // rather than ConfigNull) so this map can have null values.
     final private Map<MemoKey, AbstractConfigValue> memos;
 
+    private ResolveMemos(Map<MemoKey, AbstractConfigValue> memos) {
+        this.memos = memos;
+    }
+
     ResolveMemos() {
-        this.memos = new HashMap<MemoKey, AbstractConfigValue>();
+        this(new HashMap<MemoKey, AbstractConfigValue>());
     }
 
     AbstractConfigValue get(MemoKey key) {
         return memos.get(key);
     }
 
-    void put(MemoKey key, AbstractConfigValue value) {
-        memos.put(key, value);
+    ResolveMemos put(MemoKey key, AbstractConfigValue value) {
+        // completely inefficient, but so far nobody cares about resolve()
+        // performance, we can clean it up someday...
+        Map<MemoKey, AbstractConfigValue> copy = new HashMap<MemoKey, AbstractConfigValue>(memos);
+        copy.put(key, value);
+        return new ResolveMemos(copy);
     }
 }

config/src/main/java/com/typesafe/config/impl/ResolveReplacer.java

@@ -1,30 +0,0 @@
-package com.typesafe.config.impl;
-
-import com.typesafe.config.impl.AbstractConfigValue.NotPossibleToResolve;
-
-/** Callback that generates a replacement to use for resolving a substitution. */
-abstract class ResolveReplacer {
-    // this is a "lazy val" in essence (we only want to
-    // make the replacement one time). Making it volatile
-    // is good enough for thread safety as long as this
-    // cache is only an optimization and making the replacement
-    // twice has no side effects, which it should not...
-    private volatile AbstractConfigValue replacement = null;
-
-    final AbstractConfigValue replace(ResolveContext context) throws NotPossibleToResolve {
-        if (replacement == null)
-            replacement = makeReplacement(context);
-        return replacement;
-    }
-
-    protected abstract AbstractConfigValue makeReplacement(ResolveContext context)
-            throws NotPossibleToResolve;
-
-    static final ResolveReplacer cycleResolveReplacer = new ResolveReplacer() {
-        @Override
-        protected AbstractConfigValue makeReplacement(ResolveContext context)
-                throws NotPossibleToResolve {
-            throw new NotPossibleToResolve(context);
-        }
-    };
-}

config/src/main/java/com/typesafe/config/impl/ResolveResult.java

@@ -0,0 +1,43 @@
+package com.typesafe.config.impl;
+
+import com.typesafe.config.ConfigException;
+
+// value is allowed to be null
+final class ResolveResult<V extends AbstractConfigValue> {
+    public final ResolveContext context;
+    public final V value;
+
+    private ResolveResult(ResolveContext context, V value) {
+        this.context = context;
+        this.value = value;
+    }
+
+    static <V extends AbstractConfigValue> ResolveResult<V> make(ResolveContext context, V value) {
+        return new ResolveResult<V>(context, value);
+    }
+
+    // better option? we don't have variance
+    @SuppressWarnings("unchecked")
+    ResolveResult<AbstractConfigObject> asObjectResult() {
+        if (!(value instanceof AbstractConfigObject))
+            throw new ConfigException.BugOrBroken("Expecting a resolve result to be an object, but it was " + value);
+        Object o = this;
+        return (ResolveResult<AbstractConfigObject>) o;
+    }
+
+    // better option? we don't have variance
+    @SuppressWarnings("unchecked")
+    ResolveResult<AbstractConfigValue> asValueResult() {
+        Object o = this;
+        return (ResolveResult<AbstractConfigValue>) o;
+    }
+
+    ResolveResult<V> popTrace() {
+        return make(context.popTrace(), value);
+    }
+
+    @Override
+    public String toString() {
+        return "ResolveResult(" + value + ")";
+    }
+}

config/src/main/java/com/typesafe/config/impl/ResolveSource.java

@@ -1,8 +1,5 @@
 package com.typesafe.config.impl;
 
-import java.util.IdentityHashMap;
-import java.util.Map;
-
 import com.typesafe.config.ConfigException;
 import com.typesafe.config.impl.AbstractConfigValue.NotPossibleToResolve;
 
@@ -10,106 +7,344 @@ import com.typesafe.config.impl.AbstractConfigValue.NotPossibleToResolve;
  * This class is the source for values for a substitution like ${foo}.
  */
 final class ResolveSource {
-    final private AbstractConfigObject root;
-    // Conceptually, we transform the ResolveSource whenever we traverse
-    // a substitution or delayed merge stack, in order to remove the
-    // traversed node and therefore avoid circular dependencies.
-    // We implement it with this somewhat hacky "patch a replacement"
-    // mechanism instead of actually transforming the tree.
-    final private Map<AbstractConfigValue, ResolveReplacer> replacements;
+
+    final AbstractConfigObject root;
+    // This is used for knowing the chain of parents we used to get here.
+    // null if we should assume we are not a descendant of the root.
+    // the root itself should be a node in this if non-null.
+    final Node<Container> pathFromRoot;
+
+    ResolveSource(AbstractConfigObject root, Node<Container> pathFromRoot) {
+        this.root = root;
+        this.pathFromRoot = pathFromRoot;
+    }
 
     ResolveSource(AbstractConfigObject root) {
         this.root = root;
-        this.replacements = new IdentityHashMap<AbstractConfigValue, ResolveReplacer>();
+        this.pathFromRoot = null;
     }
 
-    static private AbstractConfigValue findInObject(AbstractConfigObject obj,
-            ResolveContext context, SubstitutionExpression subst)
+    // if we replace the root with a non-object, use an empty
+    // object with nothing in it instead.
+    private AbstractConfigObject rootMustBeObj(Container value) {
+        if (value instanceof AbstractConfigObject) {
+            return (AbstractConfigObject) value;
+        } else {
+            return SimpleConfigObject.empty();
+        }
+    }
+
+    // as a side effect, findInObject() will have to resolve all parents of the
+    // child being peeked, but NOT the child itself. Caller has to resolve
+    // the child itself if needed. ValueWithPath.value can be null but
+    // the ValueWithPath instance itself should not be.
+    static private ResultWithPath findInObject(AbstractConfigObject obj, ResolveContext context, Path path)
             throws NotPossibleToResolve {
-        return obj.peekPath(subst.path(), context);
+        // resolve ONLY portions of the object which are along our path
+        if (ConfigImpl.traceSubstitutionsEnabled())
+            ConfigImpl.trace("*** finding '" + path + "' in " + obj);
+        Path restriction = context.restrictToChild();
+        ResolveResult<? extends AbstractConfigValue> partiallyResolved = context.restrict(path).resolve(obj,
+                new ResolveSource(obj));
+        ResolveContext newContext = partiallyResolved.context.restrict(restriction);
+        if (partiallyResolved.value instanceof AbstractConfigObject) {
+            ValueWithPath pair = findInObject((AbstractConfigObject) partiallyResolved.value, path);
+            return new ResultWithPath(ResolveResult.make(newContext, pair.value), pair.pathFromRoot);
+        } else {
+            throw new ConfigException.BugOrBroken("resolved object to non-object " + obj + " to " + partiallyResolved);
+        }
     }
 
-    AbstractConfigValue lookupSubst(ResolveContext context, SubstitutionExpression subst,
-            int prefixLength) throws NotPossibleToResolve {
-        context.trace(subst);
+    static private ValueWithPath findInObject(AbstractConfigObject obj, Path path) {
         try {
+            // we'll fail if anything along the path can't
+            // be looked at without resolving.
+            return findInObject(obj, path, null);
+        } catch (ConfigException.NotResolved e) {
+            throw ConfigImpl.improveNotResolved(path, e);
+        }
+    }
+
+    static private ValueWithPath findInObject(AbstractConfigObject obj, Path path, Node<Container> parents) {
+        String key = path.first();
+        Path next = path.remainder();
+        if (ConfigImpl.traceSubstitutionsEnabled())
+            ConfigImpl.trace("*** looking up '" + key + "' in " + obj);
+        AbstractConfigValue v = obj.attemptPeekWithPartialResolve(key);
+        Node<Container> newParents = parents == null ? new Node<Container>(obj) : parents.prepend(obj);
+
+        if (next == null) {
+            return new ValueWithPath(v, newParents);
+        } else {
+            if (v instanceof AbstractConfigObject) {
+                return findInObject((AbstractConfigObject) v, next, newParents);
+            } else {
+                return new ValueWithPath(null, newParents);
+            }
+        }
+    }
+
+    ResultWithPath lookupSubst(ResolveContext context, SubstitutionExpression subst,
+            int prefixLength)
+            throws NotPossibleToResolve {
+        if (ConfigImpl.traceSubstitutionsEnabled())
+            ConfigImpl.trace(context.depth(), "searching for " + subst);
+
+        if (ConfigImpl.traceSubstitutionsEnabled())
+            ConfigImpl.trace(context.depth(), subst + " - looking up relative to file it occurred in");
         // First we look up the full path, which means relative to the
         // included file if we were not a root file
-            AbstractConfigValue result = findInObject(root, context, subst);
+        ResultWithPath result = findInObject(root, context, subst.path());
 
-            if (result == null) {
+        if (result.result.value == null) {
             // Then we want to check relative to the root file. We don't
             // want the prefix we were included at to be used when looking
             // up env variables either.
-                SubstitutionExpression unprefixed = subst.changePath(subst.path().subPath(
-                        prefixLength));
-
-                // replace the debug trace path
-                context.untrace();
-                context.trace(unprefixed);
+            Path unprefixed = subst.path().subPath(prefixLength);
 
             if (prefixLength > 0) {
-                    result = findInObject(root, context, unprefixed);
+                if (ConfigImpl.traceSubstitutionsEnabled())
+                    ConfigImpl.trace(result.result.context.depth(), unprefixed
+                            + " - looking up relative to parent file");
+                result = findInObject(root, result.result.context, unprefixed);
             }
 
-                if (result == null && context.options().getUseSystemEnvironment()) {
-                    result = findInObject(ConfigImpl.envVariablesAsConfigObject(), context,
-                            unprefixed);
+            if (result.result.value == null && result.result.context.options().getUseSystemEnvironment()) {
+                if (ConfigImpl.traceSubstitutionsEnabled())
+                    ConfigImpl.trace(result.result.context.depth(), unprefixed + " - looking up in system environment");
+                result = findInObject(ConfigImpl.envVariablesAsConfigObject(), context, unprefixed);
             }
         }
 
-            if (result != null) {
-                result = context.resolve(result);
-            }
+        if (ConfigImpl.traceSubstitutionsEnabled())
+            ConfigImpl.trace(result.result.context.depth(), "resolved to " + result);
 
         return result;
-        } finally {
-            context.untrace();
-        }
     }
 
-    void replace(AbstractConfigValue value, ResolveReplacer replacer) {
-        ResolveReplacer old = replacements.put(value, replacer);
-        if (old != null)
-            throw new ConfigException.BugOrBroken("should not have replaced the same value twice: "
-                    + value);
-    }
+    ResolveSource pushParent(Container parent) {
+        if (parent == null)
+            throw new ConfigException.BugOrBroken("can't push null parent");
 
-    void unreplace(AbstractConfigValue value) {
-        ResolveReplacer replacer = replacements.remove(value);
-        if (replacer == null)
-            throw new ConfigException.BugOrBroken("unreplace() without replace(): " + value);
-    }
+        if (ConfigImpl.traceSubstitutionsEnabled())
+            ConfigImpl.trace("pushing parent " + parent + " ==root " + (parent == root) + " onto " + this);
 
-    private AbstractConfigValue replacement(ResolveContext context, AbstractConfigValue value)
-            throws NotPossibleToResolve {
-        ResolveReplacer replacer = replacements.get(value);
-        if (replacer == null) {
-            return value;
+        if (pathFromRoot == null) {
+            if (parent == root) {
+                return new ResolveSource(root, new Node<Container>(parent));
             } else {
-            return replacer.replace(context);
+                if (ConfigImpl.traceSubstitutionsEnabled()) {
+                    // this hasDescendant check is super-expensive so it's a
+                    // trace message rather than an assertion
+                    if (root.hasDescendant((AbstractConfigValue) parent))
+                        ConfigImpl.trace("***** BUG ***** tried to push parent " + parent
+                                + " without having a path to it in " + this);
+                }
+                // ignore parents if we aren't proceeding from the
+                // root
+                return this;
+            }
+        } else {
+            Container parentParent = pathFromRoot.head();
+            if (ConfigImpl.traceSubstitutionsEnabled()) {
+                // this hasDescendant check is super-expensive so it's a
+                // trace message rather than an assertion
+                if (parentParent != null && !parentParent.hasDescendant((AbstractConfigValue) parent))
+                    ConfigImpl.trace("***** BUG ***** trying to push non-child of " + parentParent + ", non-child was "
+                            + parent);
+            }
+
+            return new ResolveSource(root, pathFromRoot.prepend(parent));
         }
     }
 
-    /**
-     * Conceptually, this is key.value().resolveSubstitutions() but using the
-     * replacement for key.value() if any.
+    ResolveSource resetParents() {
+        if (pathFromRoot == null)
+            return this;
+        else
+            return new ResolveSource(root);
+    }
+
+    // returns null if the replacement results in deleting all the nodes.
+    private static Node<Container> replace(Node<Container> list, Container old, AbstractConfigValue replacement) {
+        Container child = list.head();
+        if (child != old)
+            throw new ConfigException.BugOrBroken("Can only replace() the top node we're resolving; had " + child
+                    + " on top and tried to replace " + old + " overall list was " + list);
+        Container parent = list.tail() == null ? null : list.tail().head();
+        if (replacement == null || !(replacement instanceof Container)) {
+            if (parent == null) {
+                return null;
+            } else {
+                /*
+                 * we are deleting the child from the stack of containers
+                 * because it's either going away or not a container
                  */
-    AbstractConfigValue resolveCheckingReplacement(ResolveContext context,
-            AbstractConfigValue original) throws NotPossibleToResolve {
-        AbstractConfigValue replacement;
+                AbstractConfigValue newParent = parent.replaceChild((AbstractConfigValue) old, null);
 
-        replacement = replacement(context, original);
-
-        if (replacement != original) {
-            // start over, checking if replacement was memoized
-            return context.resolve(replacement);
+                return replace(list.tail(), parent, newParent);
+            }
         } else {
-            AbstractConfigValue resolved;
+            /* we replaced the container with another container */
+            if (parent == null) {
+                return new Node<Container>((Container) replacement);
+            } else {
+                AbstractConfigValue newParent = parent.replaceChild((AbstractConfigValue) old, replacement);
+                Node<Container> newTail = replace(list.tail(), parent, newParent);
+                if (newTail != null)
+                    return newTail.prepend((Container) replacement);
+                else
+                    return new Node<Container>((Container) replacement);
+            }
+        }
+    }
 
-            resolved = original.resolveSubstitutions(context);
+    ResolveSource replaceCurrentParent(Container old, Container replacement) {
+        if (ConfigImpl.traceSubstitutionsEnabled())
+            ConfigImpl.trace("replaceCurrentParent old " + old + "@" + System.identityHashCode(old) + " replacement "
+                + replacement + "@" + System.identityHashCode(old) + " in " + this);
+        if (old == replacement) {
+            return this;
+        } else if (pathFromRoot != null) {
+            Node<Container> newPath = replace(pathFromRoot, old, (AbstractConfigValue) replacement);
+            if (ConfigImpl.traceSubstitutionsEnabled()) {
+                ConfigImpl.trace("replaced " + old + " with " + replacement + " in " + this);
+                ConfigImpl.trace("path was: " + pathFromRoot + " is now " + newPath);
+            }
+            // if we end up nuking the root object itself, we replace it with an
+            // empty root
+            if (newPath != null)
+                return new ResolveSource((AbstractConfigObject) newPath.last(), newPath);
+            else
+                return new ResolveSource(SimpleConfigObject.empty());
+        } else {
+            if (old == root) {
+                return new ResolveSource(rootMustBeObj(replacement));
+            } else {
+                throw new ConfigException.BugOrBroken("attempt to replace root " + root + " with " + replacement);
+                // return this;
+            }
+        }
+    }
 
-            return resolved;
+    // replacement may be null to delete
+    ResolveSource replaceWithinCurrentParent(AbstractConfigValue old, AbstractConfigValue replacement) {
+        if (ConfigImpl.traceSubstitutionsEnabled())
+            ConfigImpl.trace("replaceWithinCurrentParent old " + old + "@" + System.identityHashCode(old)
+                    + " replacement " + replacement + "@" + System.identityHashCode(old) + " in " + this);
+        if (old == replacement) {
+            return this;
+        } else if (pathFromRoot != null) {
+            Container parent = pathFromRoot.head();
+            AbstractConfigValue newParent = parent.replaceChild(old, replacement);
+            return replaceCurrentParent(parent, (newParent instanceof Container) ? (Container) newParent : null);
+        } else {
+            if (old == root && replacement instanceof Container) {
+                return new ResolveSource(rootMustBeObj((Container) replacement));
+            } else {
+                throw new ConfigException.BugOrBroken("replace in parent not possible " + old + " with " + replacement
+                        + " in " + this);
+                // return this;
+            }
+        }
+    }
+
+    @Override
+    public String toString() {
+        return "ResolveSource(root=" + root + ", pathFromRoot=" + pathFromRoot + ")";
+    }
+
+    // a persistent list
+    static final class Node<T> {
+        final T value;
+        final Node<T> next;
+
+        Node(T value, Node<T> next) {
+            this.value = value;
+            this.next = next;
+        }
+
+        Node(T value) {
+            this(value, null);
+        }
+
+        Node<T> prepend(T value) {
+            return new Node<T>(value, this);
+        }
+
+        T head() {
+            return value;
+        }
+
+        Node<T> tail() {
+            return next;
+        }
+
+        T last() {
+            Node<T> i = this;
+            while (i.next != null)
+                i = i.next;
+            return i.value;
+        }
+
+        Node<T> reverse() {
+            if (next == null) {
+                return this;
+            } else {
+                Node<T> reversed = new Node<T>(value);
+                Node<T> i = next;
+                while (i != null) {
+                    reversed = reversed.prepend(i.value);
+                    i = i.next;
+                }
+                return reversed;
+            }
+        }
+
+        @Override
+        public String toString() {
+            StringBuffer sb = new StringBuffer();
+            sb.append("[");
+            Node<T> toAppendValue = this.reverse();
+            while (toAppendValue != null) {
+                sb.append(toAppendValue.value.toString());
+                if (toAppendValue.next != null)
+                    sb.append(" <= ");
+                toAppendValue = toAppendValue.next;
+            }
+            sb.append("]");
+            return sb.toString();
+        }
+    }
+
+    // value is allowed to be null
+    static final class ValueWithPath {
+        final AbstractConfigValue value;
+        final Node<Container> pathFromRoot;
+
+        ValueWithPath(AbstractConfigValue value, Node<Container> pathFromRoot) {
+            this.value = value;
+            this.pathFromRoot = pathFromRoot;
+        }
+
+        @Override
+        public String toString() {
+            return "ValueWithPath(value=" + value + ", pathFromRoot=" + pathFromRoot + ")";
+        }
+    }
+
+    static final class ResultWithPath {
+        final ResolveResult<? extends AbstractConfigValue> result;
+        final Node<Container> pathFromRoot;
+
+        ResultWithPath(ResolveResult<? extends AbstractConfigValue> result, Node<Container> pathFromRoot) {
+            this.result = result;
+            this.pathFromRoot = pathFromRoot;
+        }
+
+        @Override
+        public String toString() {
+            return "ResultWithPath(result=" + result + ", pathFromRoot=" + pathFromRoot + ")";
         }
     }
 }

config/src/main/java/com/typesafe/config/impl/SimpleConfigList.java

@@ -18,7 +18,7 @@ import com.typesafe.config.ConfigRenderOptions;
 import com.typesafe.config.ConfigValue;
 import com.typesafe.config.ConfigValueType;
 
-final class SimpleConfigList extends AbstractConfigValue implements ConfigList, Serializable {
+final class SimpleConfigList extends AbstractConfigValue implements ConfigList, Container, Serializable {
 
     private static final long serialVersionUID = 2L;
 
@@ -61,6 +61,23 @@ final class SimpleConfigList extends AbstractConfigValue implements ConfigList,
         return ResolveStatus.fromBoolean(resolved);
     }
 
+    @Override
+    public SimpleConfigList replaceChild(AbstractConfigValue child, AbstractConfigValue replacement) {
+        List<AbstractConfigValue> newList = replaceChildInList(value, child, replacement);
+        if (newList == null) {
+            return null;
+        } else {
+            // we use the constructor flavor that will recompute the resolve
+            // status
+            return new SimpleConfigList(origin(), newList);
+        }
+    }
+
+    @Override
+    public boolean hasDescendant(AbstractConfigValue descendant) {
+        return hasDescendantInList(value, descendant);
+    }
+
     private SimpleConfigList modify(NoExceptionsModifier modifier, ResolveStatus newResolveStatus) {
         try {
             return modifyMayThrow(modifier, newResolveStatus);
@@ -108,24 +125,38 @@ final class SimpleConfigList extends AbstractConfigValue implements ConfigList,
         }
     }
 
+    private static class ResolveModifier implements Modifier {
+        ResolveContext context;
+        final ResolveSource source;
+        ResolveModifier(ResolveContext context, ResolveSource source) {
+            this.context = context;
+            this.source = source;
+        }
+
         @Override
-    SimpleConfigList resolveSubstitutions(final ResolveContext context) throws NotPossibleToResolve {
+        public AbstractConfigValue modifyChildMayThrow(String key, AbstractConfigValue v)
+                    throws NotPossibleToResolve {
+            ResolveResult<? extends AbstractConfigValue> result = context.resolve(v, source);
+            context = result.context;
+            return result.value;
+            }
+    }
+
+    @Override
+    ResolveResult<? extends SimpleConfigList> resolveSubstitutions(ResolveContext context, ResolveSource source)
+            throws NotPossibleToResolve {
         if (resolved)
-            return this;
+            return ResolveResult.make(context, this);
 
         if (context.isRestrictedToChild()) {
             // if a list restricts to a child path, then it has no child paths,
             // so nothing to do.
-            return this;
+            return ResolveResult.make(context, this);
         } else {
             try {
-                return modifyMayThrow(new Modifier() {
-                    @Override
-                    public AbstractConfigValue modifyChildMayThrow(String key, AbstractConfigValue v)
-                            throws NotPossibleToResolve {
-                        return context.resolve(v);
-                    }
-                }, context.options().getAllowUnresolved() ? null : ResolveStatus.RESOLVED);
+                ResolveModifier modifier = new ResolveModifier(context, source.pushParent(this));
+                SimpleConfigList value = modifyMayThrow(modifier, context.options().getAllowUnresolved() ? null : ResolveStatus.RESOLVED);
+                return ResolveResult.make(modifier.context, value);
             } catch (NotPossibleToResolve e) {
                 throw e;
             } catch (RuntimeException e) {
@@ -157,7 +188,8 @@ final class SimpleConfigList extends AbstractConfigValue implements ConfigList,
         // note that "origin" is deliberately NOT part of equality
         if (other instanceof SimpleConfigList) {
             // optimization to avoid unwrapped() for two ConfigList
-            return canEqual(other) && value.equals(((SimpleConfigList) other).value);
+            return canEqual(other)
+                    && (value == ((SimpleConfigList) other).value || value.equals(((SimpleConfigList) other).value));
         } else {
             return false;
         }

config/src/main/java/com/typesafe/config/impl/SimpleConfigObject.java

@@ -198,6 +198,38 @@ final class SimpleConfigObject extends AbstractConfigObject implements Serializa
         return ResolveStatus.fromBoolean(resolved);
     }
 
+    @Override
+    public SimpleConfigObject replaceChild(AbstractConfigValue child, AbstractConfigValue replacement) {
+        HashMap<String, AbstractConfigValue> newChildren = new HashMap<String, AbstractConfigValue>(value);
+        for (Map.Entry<String, AbstractConfigValue> old : newChildren.entrySet()) {
+            if (old.getValue() == child) {
+                if (replacement != null)
+                    old.setValue(replacement);
+                else
+                    newChildren.remove(old.getKey());
+
+                return new SimpleConfigObject(origin(), newChildren, ResolveStatus.fromValues(newChildren.values()),
+                        ignoresFallbacks);
+            }
+        }
+        throw new ConfigException.BugOrBroken("SimpleConfigObject.replaceChild did not find " + child + " in " + this);
+    }
+
+    @Override
+    public boolean hasDescendant(AbstractConfigValue descendant) {
+        for (AbstractConfigValue child : value.values()) {
+            if (child == descendant)
+                return true;
+        }
+        // now do the expensive search
+        for (AbstractConfigValue child : value.values()) {
+            if (child instanceof Container && ((Container) child).hasDescendant(descendant))
+                return true;
+        }
+
+        return false;
+    }
+
     @Override
     protected boolean ignoresFallbacks() {
         return ignoresFallbacks;
@@ -312,22 +344,28 @@ final class SimpleConfigObject extends AbstractConfigObject implements Serializa
         }
     }
 
-    @Override
-    AbstractConfigObject resolveSubstitutions(final ResolveContext context) throws NotPossibleToResolve {
-        if (resolveStatus() == ResolveStatus.RESOLVED)
-            return this;
+    private static final class ResolveModifier implements Modifier {
 
-        try {
-            return modifyMayThrow(new Modifier() {
+        final Path originalRestrict;
+        ResolveContext context;
+        final ResolveSource source;
+
+        ResolveModifier(ResolveContext context, ResolveSource source) {
+            this.context = context;
+            this.source = source;
+            originalRestrict = context.restrictToChild();
+        }
 
         @Override
-                public AbstractConfigValue modifyChildMayThrow(String key, AbstractConfigValue v)
-                        throws NotPossibleToResolve {
+        public AbstractConfigValue modifyChildMayThrow(String key, AbstractConfigValue v) throws NotPossibleToResolve {
             if (context.isRestrictedToChild()) {
                 if (key.equals(context.restrictToChild().first())) {
                     Path remainder = context.restrictToChild().remainder();
                     if (remainder != null) {
-                                return context.restrict(remainder).resolve(v);
+                        ResolveResult<? extends AbstractConfigValue> result = context.restrict(remainder).resolve(v,
+                                source);
+                        context = result.context.unrestricted().restrict(originalRestrict);
+                        return result.value;
                     } else {
                         // we don't want to resolve the leaf child.
                         return v;
@@ -338,11 +376,27 @@ final class SimpleConfigObject extends AbstractConfigObject implements Serializa
                 }
             } else {
                 // no restrictToChild, resolve everything
-                        return context.unrestricted().resolve(v);
+                ResolveResult<? extends AbstractConfigValue> result = context.unrestricted().resolve(v, source);
+                context = result.context.unrestricted().restrict(originalRestrict);
+                return result.value;
             }
         }
 
-            });
+    }
+
+    @Override
+    ResolveResult<? extends AbstractConfigObject> resolveSubstitutions(ResolveContext context, ResolveSource source)
+            throws NotPossibleToResolve {
+        if (resolveStatus() == ResolveStatus.RESOLVED)
+            return ResolveResult.make(context, this);
+
+        final ResolveSource sourceWithParent = source.pushParent(this);
+
+        try {
+            ResolveModifier modifier = new ResolveModifier(context, sourceWithParent);
+
+            AbstractConfigValue value = modifyMayThrow(modifier);
+            return ResolveResult.make(modifier.context, value).asObjectResult();
         } catch (NotPossibleToResolve e) {
             throw e;
         } catch (RuntimeException e) {
@@ -443,6 +497,9 @@ final class SimpleConfigObject extends AbstractConfigObject implements Serializa
     }
 
     private static boolean mapEquals(Map<String, ConfigValue> a, Map<String, ConfigValue> b) {
+        if (a == b)
+            return true;
+
         Set<String> aKeys = a.keySet();
         Set<String> bKeys = b.keySet();
 

config/src/test/scala/com/typesafe/config/impl/ConcatenationTest.scala

@@ -360,13 +360,43 @@ class ConcatenationTest extends TestUtils {
     // to get there, see https://github.com/typesafehub/config/issues/160
     @Test
     def plusEqualsMultipleTimesNestedInPlusEquals() {
-        System.err.println("==============")
         val e = intercept[ConfigException.Parse] {
             val conf = parseConfig("""x += { a += 1, a += 2, a += 3 } """).resolve()
             assertEquals(Seq(1, 2, 3), conf.getObjectList("x").asScala.toVector(0).toConfig.getIntList("a").asScala.toList)
         }
         assertTrue(e.getMessage.contains("limitation"))
-        System.err.println("==============")
+    }
+
+    // from https://github.com/typesafehub/config/issues/177
+    @Test
+    def arrayConcatenationInDoubleNestedDelayedMerge() {
+        val unresolved = parseConfig("""d { x = [] }, c : ${d}, c { x += 1, x += 2 }""")
+        val conf = unresolved.resolve()
+        assertEquals(Seq(1, 2), conf.getIntList("c.x").asScala)
+    }
+
+    // from https://github.com/typesafehub/config/issues/177
+    @Test
+    def arrayConcatenationAsPartOfDelayedMerge() {
+        val unresolved = parseConfig(""" c { x: [], x : ${c.x}[1], x : ${c.x}[2] }""")
+        val conf = unresolved.resolve()
+        assertEquals(Seq(1, 2), conf.getIntList("c.x").asScala)
+    }
+
+    // from https://github.com/typesafehub/config/issues/177
+    @Test
+    def arrayConcatenationInDoubleNestedDelayedMerge2() {
+        val unresolved = parseConfig("""d { x = [] }, c : ${d}, c { x : ${c.x}[1], x : ${c.x}[2] }""")
+        val conf = unresolved.resolve()
+        assertEquals(Seq(1, 2), conf.getIntList("c.x").asScala)
+    }
+
+    // from https://github.com/typesafehub/config/issues/177
+    @Test
+    def arrayConcatenationInTripleNestedDelayedMerge() {
+        val unresolved = parseConfig("""{ r: { d.x=[] }, q: ${r}, q : { d { x = [] }, c : ${q.d}, c { x : ${q.c.x}[1], x : ${q.c.x}[2] } } }""")
+        val conf = unresolved.resolve()
+        assertEquals(Seq(1, 2), conf.getIntList("q.c.x").asScala)
     }
 
     @Test

config/src/test/scala/com/typesafe/config/impl/ConfigSubstitutionTest.scala

@@ -207,6 +207,16 @@ class ConfigSubstitutionTest extends TestUtils {
         assertEquals(2, resolved.getInt("b"))
     }
 
+    @Test
+    def throwOnIncrediblyTrivialCycle() {
+        val s = subst("a")
+        val e = intercept[ConfigException.UnresolvedSubstitution] {
+            val v = resolveWithoutFallbacks(s, parseObject("a: ${a}"))
+        }
+        assertTrue("Wrong exception: " + e.getMessage, e.getMessage().contains("cycle"))
+        assertTrue("Wrong exception: " + e.getMessage, e.getMessage().contains("${a}"))
+    }
+
     private val substCycleObject = {
         parseObject("""
 {
@@ -456,7 +466,8 @@ class ConfigSubstitutionTest extends TestUtils {
     private val delayedMergeObjectResolveProblem5 = {
         parseObject("""
   defaults {
-    a = ${item1.b} // tricky cycle
+    a = ${item1.b} // tricky cycle - we won't see ${defaults}
+                   // as we resolve this
     b = 2
   }
 
@@ -476,9 +487,9 @@ class ConfigSubstitutionTest extends TestUtils {
 
         val resolved = resolveWithoutFallbacks(delayedMergeObjectResolveProblem5)
 
-        assertEquals(2, resolved.getInt("item1.b"))
-        assertEquals(2, resolved.getInt("item2.b"))
-        assertEquals(2, resolved.getInt("defaults.a"))
+        assertEquals("item1.b", 2, resolved.getInt("item1.b"))
+        assertEquals("item2.b", 2, resolved.getInt("item2.b"))
+        assertEquals("defaults.a", 7, resolved.getInt("defaults.a"))
     }
 
     private val delayedMergeObjectResolveProblem6 = {

config/src/test/scala/com/typesafe/config/impl/TokenizerTest.scala

@@ -145,7 +145,7 @@ class TokenizerTest extends TestUtils {
         assertEquals('6', "\\u0046"(5))
 
         val tests = List[UnescapeTest]((""" "" """, ""),
-            (" \"\\u0000\" ", "\0"), // nul byte
+            (" \"\\u0000\" ", Character.toString(0)), // nul byte
             (""" "\"\\\/\b\f\n\r\t" """, "\"\\/\b\f\n\r\t"),
             ("\"\\u0046\"", "F"),
             ("\"\\u0046\\u0046\"", "FF"))