Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix node2code for array indexing expressions #12805

Merged
merged 6 commits into from
Apr 18, 2022
Merged
Show file tree
Hide file tree
Changes from 4 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
10 changes: 5 additions & 5 deletions src/DynamoCore/Engine/NodeToCode/NodeToCode.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1102,10 +1102,10 @@ internal static NodeToCodeResult NodeToCode(
IEnumerable<NodeModel> nodes,
INamingProvider namingProvider)
{
// The basic worflow is:
// 1. Compile each node to get its cde in AST format
// The basic workflow is:
// 1. Compile each node to get its code in AST format
//
// 2. Variable numbering to avoid confliction. For example, two
// 2. Variable numbering to avoid conflicts. For example, two
// code block nodes both have assignment "y = x", we need to
// rename "y" to "y1" and "y2" respectively.
//
Expand All @@ -1120,7 +1120,7 @@ internal static NodeToCodeResult NodeToCode(
// 4. Generate short name for long name variables. Typically they
// are from output ports from other nodes.
//
// 5. Do constant progation to optimize the generated code.
// 5. Do constant propagation to optimize the generated code.
#region Step 1 AST compilation

AstBuilder builder = new AstBuilder(null);
Expand All @@ -1130,7 +1130,7 @@ internal static NodeToCodeResult NodeToCode(

#endregion

#region Step 2 Varialbe numbering
#region Step 2 Variable numbering

// External inputs will be in input map
// Internal non-cbn will be input map & output map
Expand Down
58 changes: 58 additions & 0 deletions src/DynamoCore/Engine/ShortestQualifiedNameReplacer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ namespace Dynamo.Engine
/// automatically shortened to partial namespaces so that they can still be resolved uniquely.
/// E.g., given {"A.B.C.D.E", "X.Y.A.B.E.C.E", "X.Y.A.C.B.E"}, all with the same class E,
/// their shortest unique names would be: {"D.E", "E.E", "C.B.E"}.
///
/// Also replaces DesignScript.BuiltIn.Get.ValueAtIndex(exp1, exp2) calls to exp1[exp2] expressions.
/// </summary>
internal class ShortestQualifiedNameReplacer : AstReplacer
{
Expand Down Expand Up @@ -69,12 +71,68 @@ public override AssociativeNode VisitTypedIdentifierNode(TypedIdentifierNode nod
return node;
}

/// <summary>
/// If node is a DesignScript.BuiltIn.Get.ValueAtIndex(exp1, exp2) function call node.
/// it is transformed into a node with indexing operator e.g. exp1[exp2].
/// </summary>
/// <param name="node">input node</param>
/// <param name="indexExp">output node</param>
/// <returns>true if input expression is of the form DesignScript.BuiltIn.Get.ValueAtIndex(exp1, exp2), false otherwise</returns>
private bool TryBuildIndexingExpression(IdentifierListNode node, out ArrayNameNode indexExp)
{
indexExp = null;
if(node.RightNode is FunctionCallNode fcn)
{
if(fcn.Function.Name == ProtoCore.AST.Node.BuiltinValueAtIndexMethodName)
{
if(node.LeftNode.ToString() == ProtoCore.AST.Node.BuiltinGetValueAtIndexTypeName)
{
var exp1 = fcn.FormalArguments[0];
var exp2 = fcn.FormalArguments[1];

if (exp1 is ArrayNameNode ann)
{
if(exp1 is IdentifierListNode iln1)
{
if (!TryBuildIndexingExpression(iln1, out indexExp))
{
indexExp = ann.CreateInstance();
}
}
else indexExp = ann.CreateInstance();

if (exp2 is IdentifierListNode iln2)
{
if (TryBuildIndexingExpression(iln2, out ArrayNameNode index))
{
if (indexExp.ArrayDimensions != null)
{
indexExp = new IdentifierNode(indexExp.ToString());
}
indexExp.ArrayDimensions = new ArrayNode(index, null);
return true;
}
}
if(indexExp.ArrayDimensions != null)
{
indexExp = new IdentifierNode(indexExp.ToString());
}
indexExp.ArrayDimensions = new ArrayNode(exp2, null);
return true;
}
}
}
}
return false;
}

public override AssociativeNode VisitIdentifierListNode(IdentifierListNode node)
{
if (node == null)
return null;

if(TryBuildIndexingExpression(node, out ArrayNameNode indexExp)) return indexExp;
aparajit-pratap marked this conversation as resolved.
Show resolved Hide resolved

// First pass attempt to resolve the node class name
// and shorten it before traversing it deeper
AssociativeNode shortNameNode;
Expand Down
3 changes: 2 additions & 1 deletion src/DynamoCore/Graph/Nodes/CodeBlockNode.cs
Original file line number Diff line number Diff line change
Expand Up @@ -586,7 +586,8 @@ public override ProtoCore.Type GetTypeHintForOutput(int index)
{
func = targetClass.ProcTable.GetPropertyGetterByName(funcName);
}
type = func.ReturnType;
if(func != null) type = func.ReturnType;

return type;
}

Expand Down
37 changes: 36 additions & 1 deletion src/Engine/ProtoCore/Parser/AssociativeAST.cs
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,11 @@ public override IEnumerable<Node> Children()

public class ArrayNameNode : AssociativeNode
{
internal virtual ArrayNameNode CreateInstance()
aparajit-pratap marked this conversation as resolved.
Show resolved Hide resolved
{
return new ArrayNameNode(this);
}

public ArrayNode ArrayDimensions
{
get;
Expand Down Expand Up @@ -498,6 +503,11 @@ public override IEnumerable<Node> Children()

public class GroupExpressionNode : ArrayNameNode
{
internal override ArrayNameNode CreateInstance()
{
return new GroupExpressionNode(this);
}

public AssociativeNode Expression
{
get;
Expand Down Expand Up @@ -557,6 +567,11 @@ public override IEnumerable<Node> Children()

public class IdentifierNode : ArrayNameNode
{
internal override ArrayNameNode CreateInstance()
{
return new IdentifierNode(this);
}

public IdentifierNode(string identName = null)
{
datatype = TypeSystem.BuildPrimitiveTypeObject(PrimitiveType.InvalidType, 0);
Expand Down Expand Up @@ -659,6 +674,11 @@ public override TResult Accept<TResult>(IAstVisitor<TResult> visitor)

public class IdentifierListNode : ArrayNameNode
{
internal override ArrayNameNode CreateInstance()
{
return new IdentifierListNode(this);
}

public bool IsLastSSAIdentListFactor { get; set; }

public AssociativeNode LeftNode
Expand Down Expand Up @@ -716,7 +736,7 @@ public override int GetHashCode()

public override string ToString()
{
return LeftNode + "." + RightNode;
return LeftNode + "." + RightNode + base.ToString();
}

public override AstKind Kind
Expand Down Expand Up @@ -1065,6 +1085,11 @@ public class FunctionCallNode : ArrayNameNode
public AssociativeNode Function { get; set; }
public List<AssociativeNode> FormalArguments { get; set; }

internal override ArrayNameNode CreateInstance()
{
return new FunctionCallNode(this);
}

public FunctionCallNode()
{
FormalArguments = new List<AssociativeNode>();
Expand Down Expand Up @@ -2268,6 +2293,11 @@ public class RangeExprNode : ArrayNameNode
public RangeStepOperator StepOperator { get; set; }
public bool HasRangeAmountOperator { get; set; }

internal override ArrayNameNode CreateInstance()
{
return new RangeExprNode(this);
}

public RangeExprNode()
{
}
Expand Down Expand Up @@ -2375,6 +2405,11 @@ public override IEnumerable<Node> Children()

public class ExprListNode : ArrayNameNode
{
internal override ArrayNameNode CreateInstance()
{
return new ExprListNode(this);
}

public ExprListNode()
{
Exprs = new List<AssociativeNode>();
Expand Down
64 changes: 64 additions & 0 deletions test/DynamoCoreTests/NodeToCodeTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1364,6 +1364,70 @@ public void TestFilePathToCode()
Assert.IsTrue(cbn.Code.Contains(@"D:\\foo\\bar"));
}

[Test, Category("Failure")]
public void ImperativeArrayIndexingInCodeBlockToCode()
{
OpenModel(@"core\node2code\imperative_indexing.dyn");
var nodes = CurrentDynamoModel.CurrentWorkspace.Nodes;

SelectAll(nodes);
var command = new DynamoModel.ConvertNodesToCodeCommand();
CurrentDynamoModel.ExecuteCommand(command);

var cbn = CurrentDynamoModel.CurrentWorkspace.Nodes.OfType<CodeBlockNodeModel>().FirstOrDefault();
Assert.IsNotNull(cbn);

Assert.IsTrue(cbn.Code.Contains("return = p[0].DistanceTo(p[1]);"));
}

[Test]
public void ArrayIndexingInCodeBlockToCode()
{
OpenModel(@"core\node2code\array_indexing.dyn");
var nodes = CurrentDynamoModel.CurrentWorkspace.Nodes;

SelectAll(nodes);
var command = new DynamoModel.ConvertNodesToCodeCommand();
CurrentDynamoModel.ExecuteCommand(command);

var cbn = CurrentDynamoModel.CurrentWorkspace.Nodes.OfType<CodeBlockNodeModel>().FirstOrDefault();
Assert.IsNotNull(cbn);

Assert.IsTrue(cbn.Code.Contains("p[0].DistanceTo(p[1]);"));
}

[Test]
public void StringIndexingInCodeBlockToCode()
{
OpenModel(@"core\node2code\string_indexing.dyn");
var nodes = CurrentDynamoModel.CurrentWorkspace.Nodes;

SelectAll(nodes);
var command = new DynamoModel.ConvertNodesToCodeCommand();
CurrentDynamoModel.ExecuteCommand(command);

var cbn = CurrentDynamoModel.CurrentWorkspace.Nodes.OfType<CodeBlockNodeModel>().FirstOrDefault();
Assert.IsNotNull(cbn);

Assert.IsTrue(cbn.Code.Contains("t1 = s[1..2];"));
}

[Test]
public void DictIndexingInCodeBlockToCode()
{
OpenModel(@"core\node2code\dict_indexing.dyn");
var nodes = CurrentDynamoModel.CurrentWorkspace.Nodes;

SelectAll(nodes);
var command = new DynamoModel.ConvertNodesToCodeCommand();
CurrentDynamoModel.ExecuteCommand(command);

var cbn = CurrentDynamoModel.CurrentWorkspace.Nodes.OfType<CodeBlockNodeModel>().FirstOrDefault();
Assert.IsNotNull(cbn);

Assert.IsTrue(cbn.Code.Contains("dict[\"abc\"];"));
}

[Test]
public void TestDirectoryToCode()
{
Expand Down
Loading