Repository navigation
fix: += must append to values inherited through object concatenation - #882
Open
kastoestoramadus wants to merge 14 commits into
Open
kastoestoramadus wants to merge 14 commits into
kastoestoramadus wants to merge 14 commits into
Conversation
…end#751) Resolving the issue lightbend#751 config crashes with BugOrBroken 'SimpleConfigObject.replaceChild did not find SimpleConfigList' when a duplicate key with a substitution (a delayed merge) sits inside a list that is a piece of a value concatenation. Add regression tests for the optional and the defined strict substitution variants. Co-Authored-By: GLM-5.3-Flash/effort not exposed through Claude Code <noreply@anthropic.com>
…htbend#751) Resolving a delayed merge inside a container that is a piece of a value concatenation threw BugOrBroken 'SimpleConfigObject.replaceChild did not find ...' (issues lightbend#751, also hit by lightbend#339's crash): the concatenation was never pushed onto ResolveSource's parent chain, so when the delayed merge replaced itself within its parent, the walk up the chain skipped the concatenation and tried to replace the container directly in its grandparent, where it cannot be found. ConfigConcatenation implements Container with working replaceChild and hasDescendant; resolveSubstitutions now pushes itself onto the source's parent chain like other containers do. Co-Authored-By: GLM-5.3-Flash/effort not exposed through Claude Code <noreply@anthropic.com>
kastoestoramadus
force-pushed
the
fix/plus-equals-inherited-list
branch
from
October 6, 2026 05:05
95bbcd7 to
bf3a8d2
Compare
Collaborator
|
Hi @xxxxxxxxxxxxx, Thank you for your contribution! We really value the time you've taken to put this together. Before we proceed with reviewing this pull request, please sign the Akka Contributors License Agreement: |
A self-referential list concatenation in a field of an object merged
over a ${d} substitution must look back at the earlier definition of
the field (lightbend#728), with one or several appended
elements. A sibling field of a merge piece must keep seeing the
piece's final value. Genuine cycles must keep being reported as
cycles: a = ${a}, mutually referring fields inside one object, and an
object or array with a substitution inside it on a field merged over
a substitution.
Co-Authored-By: GLM-5.3-Flash/effort not exposed through Claude Code <noreply@anthropic.com>
…ieces
d { x = [] }, c : ${d}, c { x : ${c.x}[1, 2] } fails with a false
"was part of a cycle of substitutions" (lightbend#728), and so does any
self-referential field inside a piece of a delayed merge stack.
HOCON.md requires that on a cycle a substitution "looks back" in the
merge stack. That look back is implemented by resolving the merge
against the pieces below the one being resolved, but only when the
piece itself is unmergeable (a substitution or a concatenation). When
the self-referential field sits inside a mergeable piece, a lookup of
the field's path re-enters the whole merge, restarts the stack at the
top piece, re-resolves the in-flight substitution, and the cycle
marker reports a cycle.
While a mergeable piece of the stack is being resolved it is marked on
the ResolveContext. If a lookup restricted to a leaf child of the
marked piece re-enters a value that is still on the resolve stack, the
leaf is dropped from the piece and the merge supplies the value from
the pieces below it, as the look back rule requires. The drop applies
only to the marked piece itself and only to leaves whose value is a
substitution or a value concatenation, so plain self-referential
objects and arrays keep reporting unbreakable cycles, as HOCON.md
prescribes.
Resolutions re-entered while the original is still on the resolve
stack are no longer memoized, since their result depends on that
in-flight look-back state.
Co-Authored-By: GLM-5.3-Flash/effort not exposed through Claude Code <noreply@anthropic.com>
…iece
Skipping the memo whenever a value was re-entered made every ${x} lookup
into a still-resolving root walk the whole root again, so resolving N
fields that refer to one key went from linear to quadratic (40,000 fields:
1.5 s -> 22.5 s). The result only depends on in-flight state while a merge
stack piece is marked, so skip the memo only then.
Co-Authored-By: sonnet-5.5/m through Claude Code <noreply@anthropic.com>
kastoestoramadus
force-pushed
the
fix/plus-equals-inherited-list
branch
from
October 6, 2026 05:13
bf3a8d2 to
6753db6
Compare
This was referenced Oct 6, 2026
Contributor
Author
kastoestoramadus
force-pushed
the
fix/plus-equals-inherited-list
branch
from
October 7, 2026 11:56
6753db6 to
5d6f98d
Compare
Co-Authored-By: gpt-6.1-sol/m through Codex <noreply@openai.com>
Co-Authored-By: gpt-6.1-sol/m through Codex <noreply@openai.com>
Co-Authored-By: GLM-5.3-Flash/effort not exposed through Claude Code <noreply@anthropic.com> Co-Authored-By: gpt-6.1-sol/m through Codex <noreply@openai.com>
Co-Authored-By: GLM-5.3-Flash/effort not exposed through Claude Code <noreply@anthropic.com> Co-Authored-By: gpt-6.1-sol/m through Codex <noreply@openai.com>
Co-Authored-By: GLM-5.3-Flash/effort not exposed through Claude Code <noreply@anthropic.com> Co-Authored-By: gpt-6.1-sol/m through Codex <noreply@openai.com>
Co-Authored-By: GLM-5.3-Flash/effort not exposed through Claude Code <noreply@anthropic.com> Co-Authored-By: gpt-6.1-sol/m through Codex <noreply@openai.com>
Co-Authored-By: GLM-5.3-Flash/not-exposed through Claude Code <noreply@anthropic.com> Co-Authored-By: gpt-6.1-sol/m through Codex <noreply@openai.com>
Co-Authored-By: sonnet-5.5/m through Claude Code <noreply@anthropic.com> Co-Authored-By: gpt-6.1-sol/m through Codex <noreply@openai.com>
Co-Authored-By: gpt-6.1-sol/m through Codex <noreply@openai.com>
Contributor
Author
Done, CLA signed |
kastoestoramadus
force-pushed
the
fix/plus-equals-inherited-list
branch
from
October 10, 2026 17:16
5d6f98d to
3317a8e
Compare
kastoestoramadus
marked this pull request as ready for review
October 10, 2026 17:43
Contributor
|
I think we have similar passion about Hocon, please look at my PR #889 .. For my implementation I ran into ambiguity of some implementations, so I decided to formulate it into formal test suite. |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TLDR:
+=on a list inherited through${x}appends to it instead of silently dropping the inherited elements. Fixes #375 and #608. After the #879 correction this PR only adds tests.The tests cover inherited depth, missing substitutions, object cycles and forward sibling lookups (
top { n = 2, seen = ${top.n} }keeps seeing2).Generated with GLM-5.3-Flash/effort not exposed through Claude Code
Reviewed by sonnet-5.5/m
Revised with gpt-6.1-sol/m through Codex