fix(template-engine): collision detection sees all slots of a repeatedly-composed template
Claude-Session: https://claude.ai/code/session_01MtdgwpEeCUn6cUA5f1LMPj
This commit is contained in:
@@ -123,24 +123,37 @@ public static class CollisionDetector
|
|||||||
List<ResolvedMember> members,
|
List<ResolvedMember> members,
|
||||||
HashSet<int> visited)
|
HashSet<int> visited)
|
||||||
{
|
{
|
||||||
|
// `visited` is the current RECURSION PATH, not a global visited-set: an id is
|
||||||
|
// added on entry and REMOVED on exit (finally), so a template legitimately
|
||||||
|
// composed into more than one slot is re-descended for each slot. The early
|
||||||
|
// return fires only on a genuine cycle — an id already on the ACTIVE path.
|
||||||
|
// (A global visited-set would blind the detector to every collision reachable
|
||||||
|
// only through the second-and-later slot of a repeatedly-composed template.)
|
||||||
if (!visited.Add(template.Id))
|
if (!visited.Add(template.Id))
|
||||||
return;
|
return;
|
||||||
|
|
||||||
// Add direct members of this composed template with the prefix. Inherited
|
try
|
||||||
// placeholder rows are KEPT here: the composed-module walk does not climb the
|
|
||||||
// composed template's parent chain, so the placeholders are the only
|
|
||||||
// representation of a derived module's inherited members.
|
|
||||||
CollectDirectMembers(template, prefix, $"module '{prefix}'", members, skipInherited: false);
|
|
||||||
|
|
||||||
// Recurse into nested compositions
|
|
||||||
foreach (var composition in template.Compositions)
|
|
||||||
{
|
{
|
||||||
if (lookup.TryGetValue(composition.ComposedTemplateId, out var nested))
|
// Add direct members of this composed template with the prefix. Inherited
|
||||||
|
// placeholder rows are KEPT here: the composed-module walk does not climb the
|
||||||
|
// composed template's parent chain, so the placeholders are the only
|
||||||
|
// representation of a derived module's inherited members.
|
||||||
|
CollectDirectMembers(template, prefix, $"module '{prefix}'", members, skipInherited: false);
|
||||||
|
|
||||||
|
// Recurse into nested compositions
|
||||||
|
foreach (var composition in template.Compositions)
|
||||||
{
|
{
|
||||||
var nestedPrefix = $"{prefix}.{composition.InstanceName}";
|
if (lookup.TryGetValue(composition.ComposedTemplateId, out var nested))
|
||||||
CollectComposedMembers(nested, nestedPrefix, lookup, members, visited);
|
{
|
||||||
|
var nestedPrefix = $"{prefix}.{composition.InstanceName}";
|
||||||
|
CollectComposedMembers(nested, nestedPrefix, lookup, members, visited);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
finally
|
||||||
|
{
|
||||||
|
visited.Remove(template.Id);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
private static void CollectInheritedMembers(
|
private static void CollectInheritedMembers(
|
||||||
|
|||||||
@@ -188,4 +188,64 @@ public class CollisionDetectorTests
|
|||||||
Assert.NotEmpty(collisions);
|
Assert.NotEmpty(collisions);
|
||||||
Assert.Contains(collisions, c => c.Contains("dup.Shared"));
|
Assert.Contains(collisions, c => c.Contains("dup.Shared"));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ========================================================================
|
||||||
|
// Task 10: repeated composition blindness. A template composed into TWO
|
||||||
|
// different slots was added to the global visited-set on the first slot, so
|
||||||
|
// the SECOND slot's subtree was never collected — the detector went blind to
|
||||||
|
// any collision reachable only through the second slot. The visited-set must
|
||||||
|
// be a recursion PATH set (removed on the way back out), not a global set, so
|
||||||
|
// siblings/other slots can re-descend the same template.
|
||||||
|
// ========================================================================
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void DetectCollisions_TemplateComposedTwiceWithinSubtree_SeesCollisionUnderSecondSlot()
|
||||||
|
{
|
||||||
|
// Z carries the leaf member "Val".
|
||||||
|
var z = new Template("Z") { Id = 4 };
|
||||||
|
z.Attributes.Add(new TemplateAttribute("Val") { Id = 40, TemplateId = 4, DataType = DataType.Float });
|
||||||
|
|
||||||
|
// Y composes Z as "z".
|
||||||
|
var y = new Template("Y") { Id = 3 };
|
||||||
|
y.Compositions.Add(new TemplateComposition("z") { Id = 1, TemplateId = 3, ComposedTemplateId = 4 });
|
||||||
|
|
||||||
|
// W composes Y into TWO slots (y1, y2). Because both slots are visited within
|
||||||
|
// the SAME CollectComposedMembers walk, they share one visited set — the
|
||||||
|
// second slot's subtree was previously never collected.
|
||||||
|
var w = new Template("W") { Id = 2 };
|
||||||
|
w.Compositions.Add(new TemplateComposition("y1") { Id = 2, TemplateId = 2, ComposedTemplateId = 3 });
|
||||||
|
w.Compositions.Add(new TemplateComposition("y2") { Id = 3, TemplateId = 2, ComposedTemplateId = 3 });
|
||||||
|
|
||||||
|
// X composes W as "w" AND declares a direct attribute whose path collides with
|
||||||
|
// a member reachable only via the SECOND slot: "w.y2.z.Val".
|
||||||
|
var x = new Template("X") { Id = 1 };
|
||||||
|
x.Attributes.Add(new TemplateAttribute("w.y2.z.Val") { Id = 10, TemplateId = 1, DataType = DataType.Float });
|
||||||
|
x.Compositions.Add(new TemplateComposition("w") { Id = 4, TemplateId = 1, ComposedTemplateId = 2 });
|
||||||
|
|
||||||
|
var all = new List<Template> { x, w, y, z };
|
||||||
|
var collisions = CollisionDetector.DetectCollisions(x, all);
|
||||||
|
|
||||||
|
// "w.y2.z.Val" exists both as X's direct attribute and via the w→y2→z→Val subtree.
|
||||||
|
Assert.NotEmpty(collisions);
|
||||||
|
Assert.Contains(collisions, c => c.Contains("w.y2.z.Val"));
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void DetectCollisions_CyclicComposition_DoesNotLoopForever()
|
||||||
|
{
|
||||||
|
// Genuine cycle guard: A composes B, B composes A. The recursion-path set
|
||||||
|
// must still short-circuit a template already on the ACTIVE path so the
|
||||||
|
// walk terminates instead of recursing infinitely.
|
||||||
|
var a = new Template("A") { Id = 1 };
|
||||||
|
a.Compositions.Add(new TemplateComposition("b") { Id = 1, TemplateId = 1, ComposedTemplateId = 2 });
|
||||||
|
|
||||||
|
var b = new Template("B") { Id = 2 };
|
||||||
|
b.Compositions.Add(new TemplateComposition("a") { Id = 2, TemplateId = 2, ComposedTemplateId = 1 });
|
||||||
|
|
||||||
|
var all = new List<Template> { a, b };
|
||||||
|
|
||||||
|
// Must return (terminate) rather than stack-overflow / hang.
|
||||||
|
var collisions = CollisionDetector.DetectCollisions(a, all);
|
||||||
|
Assert.Empty(collisions);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user