Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 5 additions & 28 deletions src/DynamoCore/Graph/Nodes/NodeModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1643,15 +1643,7 @@ public void UseLevelAndReplicationGuide(List<AssociativeNode> inputs)
if (inputs == null || !inputs.Any())
return;

// Variadic nodes pack tail inputs into a single argument, so per-port
// level/lacing handling for the variadic slots must happen before packing.
// The hook lets such nodes scope this post-pack pass to the non-variadic
// prefix, preventing double-wrap.
int boundedCount = Math.Min(inputs.Count, LevelAndReplicationGuideInputCount(inputs.Count));
if (boundedCount <= 0)
return;

for (int i = 0; i < boundedCount; i++)
for (int i = 0; i < inputs.Count; i++)
{
if (InPorts[i].UseLevels)
{
Expand All @@ -1662,15 +1654,15 @@ public void UseLevelAndReplicationGuide(List<AssociativeNode> inputs)
switch (ArgumentLacing)
{
case LacingStrategy.Auto:
for (int i = 0; i < boundedCount; ++i)
for (int i = 0; i < inputs.Count; ++i)
{
if (InPorts[i].UseLevels)
inputs[i] = AstFactory.AddReplicationGuide(inputs[i], new List<int> { 1 }, false);
}
break;

case LacingStrategy.Shortest:
for (int i = 0; i < boundedCount; ++i)
for (int i = 0; i < inputs.Count; ++i)
{
inputs[i] = AstFactory.AddReplicationGuide(inputs[i], new List<int> { 1 }, false);
}
Expand All @@ -1679,7 +1671,7 @@ public void UseLevelAndReplicationGuide(List<AssociativeNode> inputs)

case LacingStrategy.Longest:

for (int i = 0; i < boundedCount; ++i)
for (int i = 0; i < inputs.Count; ++i)
{
inputs[i] = AstFactory.AddReplicationGuide(inputs[i], new List<int> { 1 }, true);
}
Expand All @@ -1688,29 +1680,14 @@ public void UseLevelAndReplicationGuide(List<AssociativeNode> inputs)
case LacingStrategy.CrossProduct:

int guide = 1;
for (int i = 0; i < boundedCount; ++i)
for (int i = 0; i < inputs.Count; ++i)
{
inputs[i] = AstFactory.AddReplicationGuide(inputs[i], new List<int> { guide }, false);
guide++;
}
break;
}
}

/// <summary>
/// Returns the number of leading inputs that <see cref="UseLevelAndReplicationGuide"/>
/// should wrap with <c>AtLevel</c> / replication guides.
/// </summary>
/// <remarks>
/// Variadic nodes (e.g. <c>DSVarArgFunction</c>) pack their tail inputs into a single
/// packed argument before this post-pack pass runs. Wrapping the packed slot here would
/// be either a no-op or a double-wrap, so variadic nodes override this hook to expose only
/// the non-variadic prefix and handle per-port level / lacing on the unpacked variadic
/// inputs themselves.
/// </remarks>
/// <param name="rawInputCount">Number of raw input AST nodes available to the pass.</param>
/// <returns>Number of inputs (counted from index 0) that should be wrapped.</returns>
protected virtual int LevelAndReplicationGuideInputCount(int rawInputCount) => rawInputCount;
#endregion

#region Input and Output Connections
Expand Down
194 changes: 93 additions & 101 deletions src/DynamoCore/Graph/Nodes/ZeroTouch/DSVarArgFunction.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using Dynamo.Library;
using Newtonsoft.Json;
using ProtoCore.AST.AssociativeAST;
using ProtoCore.Utils;

namespace Dynamo.Graph.Nodes.ZeroTouch
{
Expand Down Expand Up @@ -99,25 +100,6 @@ internal override bool HandleModelEventCore(string eventName, int value, UndoRed
[JsonIgnore]
public VariableInputNodeController VarInputController { get; private set; }

/// <summary>
/// Scopes <see cref="NodeModel.UseLevelAndReplicationGuide"/> to the non-variadic
/// prefix of this node's inputs.
/// </summary>
/// <remarks>
/// Variadic zero-touch functions pack their tail inputs into a single
/// <c>ExprListNode</c> argument before <see cref="NodeModel.UseLevelAndReplicationGuide"/>
/// runs. AtLevel / replication-guide annotations on an <c>ExprListNode</c> are inert
/// at runtime, so per-port Use Levels and lacing on the variadic slots must be applied
/// pre-pack inside <c>ZeroTouchVarArgNodeController.BuildOutputAst</c>. This override
/// limits the post-pack pass to the non-variadic prefix so the packed variadic slot
/// is not touched twice.
/// </remarks>
protected override int LevelAndReplicationGuideInputCount(int rawInputCount)
{
int prefix = Controller.Definition.Parameters.Count() - 1;
return prefix < 0 ? 0 : prefix;
}

#region VarInput Controller
private sealed class ZeroTouchVarInputController : VariableInputNodeController
{
Expand Down Expand Up @@ -185,99 +167,109 @@ protected override void InitializeFunctionParameters(NodeModel model, IEnumerabl

protected override void BuildOutputAst(NodeModel model, List<AssociativeNode> inputAstNodes, List<AssociativeNode> resultAst)
{
// All inputs are provided, then we should pack all inputs that
// belong to variable input parameter into a single array.
// A variadic zero-touch function foo(x1, ..., xn, params y) is invoked at
// runtime as foo(x1, ..., xn, {y1, ..., ym}) -- every variadic input is
// packed into a single array argument. Suppose paramCount == n + 1 and the
// node inputs are i1, ..., in, y1, ..., ym.
if (!model.IsPartiallyApplied)
{
var paramCount = Definition.Parameters.Count();
int variadicStart = Definition.Parameters.Count() - 1;

// Suppose a function foo() with var args, its signature is:
//
// foo(x1, x2, ..., xn, params y)
//
// so paramCount == n + 1 here, and suppose inputs are
//
// i1, i2, ...., in, y1, y2, ..., ym
//
// Why: per-port Use Levels and lacing-driven replication guides
// on the variadic inputs (y1..ym) must be applied *before* we
// pack them into an ExprListNode. AtLevel / replication-guide
// annotations carried inside an ExprListNode are inert at runtime,
// so packing first would silently drop per-port settings on every
// variadic port (the original DYN-10572 defect). The non-variadic
// prefix (i1..in) is handled by the post-pack
// NodeModel.UseLevelAndReplicationGuide pass, which DSVarArgFunction
// scopes to indices [0, paramCount - 1) via the
// LevelAndReplicationGuideInputCount override -- preventing the
// packed slot from being wrapped twice.
int variadicStart = paramCount - 1;
if (variadicStart >= 0 && variadicStart < inputAstNodes.Count)
// When any port uses levels, per-port replication must be honored. AtLevel /
// replication-guide annotations are only consumed at function-call argument
// positions and are inert inside the packed ExprList literal, and DesignScript
// cannot call a params method with separate arguments. So we route through a
// generated wrapper whose formal parameters are the individual ports: the
// per-port AtLevel and the node's lacing replication guides land on the
// wrapper-call arguments (replicating exactly like a non-variadic node), and
// the wrapper body packs the already-replicated values per invocation. See
// DYN-10572. Graphs with no Use Levels keep the original pack-then-call path,
// so their behavior (including their lacing) is unchanged.
if (variadicStart >= 0 && variadicStart < inputAstNodes.Count
&& AnyPortUsesLevels(model, inputAstNodes.Count)
&& TryBuildReplicatingOutputAst(model, inputAstNodes, resultAst, variadicStart))
{
for (int i = variadicStart; i < inputAstNodes.Count; i++)
{
if (model.InPorts[i].UseLevels)
{
inputAstNodes[i] = AstFactory.AddAtLevel(
inputAstNodes[i],
-model.InPorts[i].Level,
model.InPorts[i].KeepListStructure);
}
}
return;
}

switch (model.ArgumentLacing)
{
case LacingStrategy.Auto:
for (int i = variadicStart; i < inputAstNodes.Count; i++)
{
if (model.InPorts[i].UseLevels)
{
inputAstNodes[i] = AstFactory.AddReplicationGuide(
inputAstNodes[i], new List<int> { 1 }, false);
}
}
break;
// Default path: pack all var arguments into an array {y1, ..., ym}
// (skipping the first n == paramCount - 1 inputs).
var argPack = AstFactory.BuildExprList(inputAstNodes.Skip(variadicStart).ToList());
inputAstNodes = inputAstNodes.Take(variadicStart).ToList();
inputAstNodes.Add(argPack);
}

case LacingStrategy.Shortest:
for (int i = variadicStart; i < inputAstNodes.Count; i++)
{
inputAstNodes[i] = AstFactory.AddReplicationGuide(
inputAstNodes[i], new List<int> { 1 }, false);
}
break;
base.BuildOutputAst(model, inputAstNodes, resultAst);
}

case LacingStrategy.Longest:
for (int i = variadicStart; i < inputAstNodes.Count; i++)
{
inputAstNodes[i] = AstFactory.AddReplicationGuide(
inputAstNodes[i], new List<int> { 1 }, true);
}
break;
/// <summary>
/// Returns true when any port has Use Levels enabled. Those settings cannot survive
/// the default pack-then-call path (they are inert inside the packed ExprList), so the
/// node must route through the per-port replicating wrapper instead.
/// </summary>
private static bool AnyPortUsesLevels(NodeModel model, int inputCount)
{
for (int i = 0; i < inputCount; i++)
{
if (model.InPorts[i].UseLevels)
return true;
}
return false;
}

case LacingStrategy.CrossProduct:
// Continue cross-product indices from where the prefix
// pass would have stopped: the prefix uses guides
// 1..paramCount-1, so the first variadic port starts at
// paramCount and each subsequent variadic port gets a
// distinct rank.
int guide = paramCount;
for (int i = variadicStart; i < inputAstNodes.Count; i++)
{
inputAstNodes[i] = AstFactory.AddReplicationGuide(
inputAstNodes[i], new List<int> { guide }, false);
guide++;
}
break;
}
}
/// <summary>
/// Emits a generated wrapper function that exposes every port as a separate formal
/// parameter and packs the variadic tail per invocation, then calls it with per-port
/// Use Levels and lacing replication guides applied to the individual arguments.
/// Returns false (leaving <paramref name="resultAst"/> untouched) if the wrapper
/// source cannot be parsed, so the caller falls back to the default pack path.
/// </summary>
private bool TryBuildReplicatingOutputAst(
NodeModel model, List<AssociativeNode> inputAstNodes, List<AssociativeNode> resultAst, int variadicStart)
{
// Build the call to the underlying function exactly as GetFunctionApplication
// would: Class.Function for member functions, the bare name for globals.
string callName = Definition.Type == FunctionType.GenericFunction
? Definition.FunctionName
: Definition.ClassName + "." + Definition.FunctionName;

// Here we pack all var arguments in an array {y1, y2, ..., ym}
// (skipping the first n == paramCount - 1 inputs)
var argPack = AstFactory.BuildExprList(inputAstNodes.Skip(paramCount - 1).ToList());
inputAstNodes = inputAstNodes.Take(paramCount - 1).ToList();
inputAstNodes.Add(argPack);
// Unique, deterministic per node + arity so repeated runs redefine the same
// wrapper (cache-stable) and distinct nodes never collide.
string wrapperName = "__vararg_" + model.GUID.ToString("N") + "_" + inputAstNodes.Count;

var paramNames = Enumerable.Range(0, inputAstNodes.Count).Select(i => "p" + i).ToList();
var prefixArgs = paramNames.Take(variadicStart);
var packedArg = "[" + string.Join(", ", paramNames.Skip(variadicStart)) + "]";
var innerArgs = string.Join(", ", prefixArgs.Append(packedArg));

string wrapperSource =
"def " + wrapperName + "(" + string.Join(", ", paramNames) + ")" +
" { return = " + callName + "(" + innerArgs + "); }";

FunctionDefinitionNode wrapperDef;
try
{
wrapperDef = ParserUtils.Parse(wrapperSource).Body
.OfType<FunctionDefinitionNode>()
.FirstOrDefault();
}
catch
{
return false;
}

base.BuildOutputAst(model, inputAstNodes, resultAst);
if (wrapperDef == null)
return false;

resultAst.Add(wrapperDef);

// Per-port AtLevel / lacing replication guides land on the wrapper-call
// arguments, where they are honored just as on a non-variadic node.
model.UseLevelAndReplicationGuide(inputAstNodes);

var rhs = AstFactory.BuildFunctionCall(wrapperName, inputAstNodes);
AssignIdentifiersForFunctionCall(model, rhs, resultAst);
return true;
}
}
}
1 change: 0 additions & 1 deletion src/DynamoCore/PublicAPI.Unshipped.txt
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,3 @@ Dynamo.Models.DynamoModel.DefaultStartConfiguration.EnableUnTrustedLocationsNoti
Dynamo.Models.DynamoModel.IStartConfiguration.EnableUnTrustedLocationsNotifications.get -> bool
Dynamo.Models.DynamoModel.OpenFileCommand.OpenFileCommand(System.String filePath, System.Boolean forceManualExecutionMode, System.Boolean isTemplate, System.Boolean forceBlockRun) -> void
Dynamo.Models.DynamoModel.InsertFileCommand.InsertFileCommand(System.String filePath, System.Boolean forceManualExecutionMode, System.Boolean forceBlockRun) -> void
virtual Dynamo.Graph.Nodes.NodeModel.LevelAndReplicationGuideInputCount(int rawInputCount) -> int
13 changes: 7 additions & 6 deletions test/DynamoCoreTests/Nodes/ListTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -109,16 +109,17 @@ public void TestConcatenateListsNormalInput()
public void TestListJoinUseLevelsOnVariadicPort()
{
// DYN-10572: List.Join is a pure-variadic DSVarArgFunction (no non-variadic
// prefix). With [IsLacingDisabled] on the C# method the node's lacing is
// Disabled, so per-port Use Levels must still be honored pre-pack even
// though no replication guides are applied. This test confirms the
// variadic Use Levels path runs without breaking the post-pack pass
// (which is a no-op here because LevelAndReplicationGuideInputCount is 0).
// prefix). Use Levels on a variadic port (list1 = [3,4] @L1) is now honored:
// the per-port AtLevel replicates the join at level 1, exactly as it would on
// a non-variadic node, independently of the other port. Before the fix the
// level was silently dropped and the result was the flat [1,2,3,4]; honoring
// it replicates element-wise to [[1,3],[2,4]].
string testFilePath = Path.Combine(listTestFolder, "TestListJoinUseLevelsOnVariadicPort.dyn");

RunModel(testFilePath);

AssertPreviewValue("dddd11110000222200003333dddd4444", new object[] { 1, 2, 3, 4 });
AssertPreviewValue("dddd11110000222200003333dddd4444",
new object[] { new object[] { 1, 3 }, new object[] { 2, 4 } });
}

#endregion
Expand Down
8 changes: 4 additions & 4 deletions test/DynamoCoreTests/Nodes/StringTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -230,10 +230,10 @@ public void TestJoinStringNormalInput()
public void TestJoinStringUseLevelsOnVariadicPort()
{
// DYN-10572: String.Join has both a non-variadic prefix port (separator)
// and variadic ports. Set UseLevels on the separator port to exercise the
// bounded post-pack pass (LevelAndReplicationGuideInputCount == 1) AND on
// a variadic port to exercise the new pre-pack handling, confirming both
// code paths cooperate correctly.
// and variadic ports. The separator (prefix port) broadcasts unchanged while
// Use Levels on a variadic port (string1 = ["a","b"] @L1) replicates the join,
// confirming the prefix port passes through correctly and per-port Use Levels
// is honored on the variadic ports.
string testFilePath = Path.Combine(localDynamoStringTestFolder, "TestJoinStringUseLevelsOnVariadicPort.dyn");

RunModel(testFilePath);
Expand Down
2 changes: 1 addition & 1 deletion test/core/string/TestJoinStringUseLevelsOnVariadicPort.dyn
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@
"Description": "String used to separate strings.\n\nstring",
"UsingDefaultValue": false,
"Level": 1,
"UseLevels": true,
"UseLevels": false,
"KeepListStructure": false
},
{
Expand Down
Loading