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

crash when undoing deletion of function definition code blocks #11220

Merged
merged 12 commits into from
Nov 4, 2020
4 changes: 2 additions & 2 deletions src/Engine/ProtoCore/CodeGen.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1022,7 +1022,7 @@ protected bool VerifyAllocation(string name, int classScope, int functionScope,

bool hasThisSymbol;
AddressType addressType;
symbolIndex = ClassUtils.GetSymbolIndex(thisClass, name, classScope, functionScope, currentCodeBlock.codeBlockId, core.CompleteCodeBlockList, out hasThisSymbol, out addressType);
symbolIndex = ClassUtils.GetSymbolIndex(thisClass, name, classScope, functionScope, currentCodeBlock.codeBlockId, core.CompleteCodeBlockDict, out hasThisSymbol, out addressType);
if (Constants.kInvalidIndex != symbolIndex)
{
// It is static member, then get node from code block
Expand Down Expand Up @@ -1080,7 +1080,7 @@ protected bool IsProperty(string name)
ProtoCore.DSASM.AddressType addressType;
ProtoCore.DSASM.ClassNode classnode = core.ClassTable.ClassNodes[globalClassIndex];

int symbolIndex = ClassUtils.GetSymbolIndex(classnode, name, globalClassIndex, globalProcIndex, 0, core.CompleteCodeBlockList, out hasThisSymbol, out addressType);
int symbolIndex = ClassUtils.GetSymbolIndex(classnode, name, globalClassIndex, globalProcIndex, 0, core.CompleteCodeBlockDict, out hasThisSymbol, out addressType);
if (symbolIndex == ProtoCore.DSASM.Constants.kInvalidIndex)
{
return false;
Expand Down
18 changes: 11 additions & 7 deletions src/Engine/ProtoCore/Core.cs
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,13 @@ public struct ErrorEntry
public List<CodeBlock> CodeBlockList { get; set; }
// The Complete Code Block list contains all the code blocks
// unlike the codeblocklist which only stores the outer most code blocks
public List<CodeBlock> CompleteCodeBlockList { get; set; }
[Obsolete("Property will be deprecated in Dynamo 3.0")]
public List<CodeBlock> CompleteCodeBlockList
{
get { return CompleteCodeBlockDict.Select(x => x.Value).ToList(); }
set { value.ForEach(x => CompleteCodeBlockDict.Add(x.codeBlockId, x)); }
}
internal SortedDictionary<int, CodeBlock> CompleteCodeBlockDict { get; set; }

/// <summary>
/// ForLoopBlockIndex tracks the current number of new for loop blocks created at compile time for every new compile phase
Expand Down Expand Up @@ -347,7 +353,7 @@ public void SetFunctionInactive(FunctionDefinitionNode functionDef)
// Remove codeblock defined in procNode from CodeBlockList and CompleteCodeBlockList
foreach (int cbID in procNode.ChildCodeBlocks)
{
CompleteCodeBlockList.RemoveAll(x => x.codeBlockId == cbID);
CompleteCodeBlockDict.Remove(cbID);

foreach (CodeBlock cb in CodeBlockList)
{
Expand Down Expand Up @@ -438,7 +444,7 @@ private void ResetAll(Options options)
CodeBlockIndex = 0;
RuntimeTableIndex = 0;
CodeBlockList = new List<CodeBlock>();
CompleteCodeBlockList = new List<CodeBlock>();
CompleteCodeBlockDict = new SortedDictionary<int, CodeBlock>();
CallsiteGuidMap = new Dictionary<Guid, int>();

AssocNode = null;
Expand Down Expand Up @@ -717,9 +723,7 @@ public void GenerateExecutable()
// Create the code block list data
DSExecutable.CodeBlocks = new List<CodeBlock>();
DSExecutable.CodeBlocks.AddRange(CodeBlockList);
DSExecutable.CompleteCodeBlocks = new List<CodeBlock>();
DSExecutable.CompleteCodeBlocks.AddRange(CompleteCodeBlockList);

DSExecutable.CompleteCodeBlockDict = new SortedDictionary<int, CodeBlock>(CompleteCodeBlockDict);

// Retrieve the class table directly since it is a global table
DSExecutable.classTable = ClassTable;
Expand Down Expand Up @@ -770,7 +774,7 @@ public void GenerateExecutable()
internal int GetRuntimeTableSize()
{
// Due to the way this list is constructed, the largest id is the one of the last block.
var lastBlock = CompleteCodeBlockList.LastOrDefault();
var lastBlock = CompleteCodeBlockDict.LastOrDefault().Value;
// If there are no code blocks yet, then the required size for tables is 0.
// This happens when the first code block is being created and its id is being generated.
if (lastBlock == null)
Expand Down
13 changes: 10 additions & 3 deletions src/Engine/ProtoCore/DSASM/Executable.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System;
using System.Collections.Generic;
using System.Linq;
using ProtoCore.AssociativeGraph;
using ProtoCore.Lang;
using ProtoFFI;
Expand Down Expand Up @@ -46,7 +47,13 @@ public class Executable
public TypeSystem TypeSystem { get; set; }

public List<CodeBlock> CodeBlocks { get; set; }
public List<CodeBlock> CompleteCodeBlocks { get; set; }

[Obsolete("Property will be deprecated in Dynamo 3.0")]
public List<CodeBlock> CompleteCodeBlocks {
get { return CompleteCodeBlockDict.Select(x => x.Value).ToList(); }
set { value.ForEach(x => CompleteCodeBlockDict.Add(x.codeBlockId, x)); }
}
internal SortedDictionary<int, CodeBlock> CompleteCodeBlockDict { get; set; }

public InstructionStream[] instrStreamList { get; set; }
public InstructionStream iStreamCanvas { get; set; }
Expand Down Expand Up @@ -94,7 +101,7 @@ public void Reset()
instrStreamList = null;
iStreamCanvas = null;
CodeBlocks = null;
CompleteCodeBlocks = null;
CompleteCodeBlockDict = null;
ContextDataMngr = null;
CodeToLocation = null;
CurrentDSFileName = string.Empty;
Expand Down Expand Up @@ -156,7 +163,7 @@ public CodeBlock(Guid guid, CodeBlockType type, ProtoCore.Language langId, int c

isBreakable = isBreakableBlock;
codeBlockId = core.GetRuntimeTableSize();
core.CompleteCodeBlockList.Add(this);
core.CompleteCodeBlockDict.Add(codeBlockId, this);

symbols.RuntimeIndex = codeBlockId;

Expand Down
26 changes: 21 additions & 5 deletions src/Engine/ProtoCore/DSASM/Executive.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1513,8 +1513,15 @@ private void XLangUpdateDependencyGraph(int currentLangBlock)
// happens.
if (graphNode.isLanguageBlock && currentLangBlock != Constants.kInvalidIndex)
{
if (graphNode.languageBlockId == currentLangBlock
|| exe.CompleteCodeBlocks[currentLangBlock].IsMyAncestorBlock(graphNode.languageBlockId))
if (graphNode.languageBlockId == currentLangBlock)
{
continue;
}

bool found = exe.CompleteCodeBlockDict.TryGetValue(currentLangBlock, out CodeBlock cb);
Validity.Assert(found, $"Could not find code block with codeBlockId {currentLangBlock}");

if (cb.IsMyAncestorBlock(graphNode.languageBlockId))
{
continue;
}
Expand Down Expand Up @@ -2708,7 +2715,7 @@ private bool ResolveDynamicFunction(Instruction instr, out bool isMemberFunction
SymbolNode node = null;
bool isStatic = false;
ClassNode classNode = exe.classTable.ClassNodes[type];
int symbolIndex = ClassUtils.GetSymbolIndex(classNode, procName, type, Constants.kGlobalScope, runtimeCore.RunningBlock, exe.CompleteCodeBlocks, out hasThisSymbol, out addressType);
int symbolIndex = ClassUtils.GetSymbolIndex(classNode, procName, type, Constants.kGlobalScope, runtimeCore.RunningBlock, exe.CompleteCodeBlockDict, out hasThisSymbol, out addressType);

if (Constants.kInvalidIndex != symbolIndex)
{
Expand Down Expand Up @@ -2904,7 +2911,10 @@ public void GCCodeBlock(int blockId, int functionIndex = Constants.kGlobalScope,

public void ReturnSiteGC(int blockId, int classIndex, int functionIndex)
{
foreach (CodeBlock cb in exe.CompleteCodeBlocks[blockId].children)
bool found = exe.CompleteCodeBlockDict.TryGetValue(blockId, out CodeBlock codeBlock);
Validity.Assert(found, $"Could find code block with codeBlockId {blockId}");

foreach (CodeBlock cb in codeBlock.children)
{
if (cb.blockType == CodeBlockType.Construct)
GCCodeBlock(cb.codeBlockId, functionIndex, classIndex);
Expand All @@ -2927,6 +2937,10 @@ public AssociativeGraph.GraphNode GetFirstGraphNode(string varName, out int bloc
for (int n = 0; n < exe.instrStreamList.Length; ++n)
{
InstructionStream stream = exe.instrStreamList[n];
if (stream == null)
{
continue;
}
for (int i = 0; i < stream.dependencyGraph.GraphList.Count; ++i)
{
AssociativeGraph.GraphNode node = stream.dependencyGraph.GraphList[i];
Expand Down Expand Up @@ -4409,7 +4423,9 @@ private void RETCN_Handler(Instruction instruction)
StackValue op1 = instruction.op1;
int blockId = op1.BlockIndex;

CodeBlock codeBlock = exe.CompleteCodeBlocks[blockId];
bool found = exe.CompleteCodeBlockDict.TryGetValue(blockId, out CodeBlock codeBlock);
Validity.Assert(found, $"Could find code block with codeBlockId {blockId}");

runtimeVerify(codeBlock.blockType == CodeBlockType.Construct);
GCCodeBlock(blockId);
pc++;
Expand Down
21 changes: 16 additions & 5 deletions src/Engine/ProtoCore/DSASM/Mirror/ExecutionMirror.cs
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,8 @@ private int GetSymbolIndex(string name, out int ci, ref int block, out SymbolNod
}
else
{
CodeBlock searchBlock = runtimeCore.DSExecutable.CompleteCodeBlocks[block];
bool found = runtimeCore.DSExecutable.CompleteCodeBlockDict.TryGetValue(block, out CodeBlock searchBlock);
Validity.Assert(found, $"Could not find code block with codeBlockId {block}");

// To detal with the case that a language block defined in a function
//
Expand Down Expand Up @@ -718,10 +719,15 @@ public StackValue GetValue(string name, int startBlock = 0)

for (int block = startBlock; block < exe.runtimeSymbols.Length; block++)
{
int index = exe.runtimeSymbols[block].IndexOf(name, Constants.kInvalidIndex, Constants.kGlobalScope);
var symbolTable = exe.runtimeSymbols[block];
if (symbolTable == null)
{
continue;
}
int index = symbolTable.IndexOf(name, Constants.kInvalidIndex, Constants.kGlobalScope);
if (Constants.kInvalidIndex != index)
{
SymbolNode symNode = exe.runtimeSymbols[block].symbolList[index];
SymbolNode symNode = symbolTable.symbolList[index];
if (symNode.absoluteFunctionIndex == Constants.kGlobalScope)
{
return MirrorTarget.rmem.GetAtRelative(symNode.index);
Expand All @@ -738,11 +744,16 @@ public StackValue GetRawFirstValue(string name, int startBlock = 0, int classcop

for (int block = startBlock; block < exe.runtimeSymbols.Length; block++)
{
int index = exe.runtimeSymbols[block].IndexOf(name, classcope, Constants.kGlobalScope);
var symbolTable = exe.runtimeSymbols[block];
if (symbolTable == null)
{
continue;
}
int index = symbolTable.IndexOf(name, classcope, Constants.kGlobalScope);
if (Constants.kInvalidIndex != index)
{
//Q(Luke): This seems to imply that the LHS is an array index?
var symbol = exe.runtimeSymbols[block].symbolList[index];
var symbol = symbolTable.symbolList[index];
return MirrorTarget.rmem.GetSymbolValue(symbol);
}
}
Expand Down
8 changes: 6 additions & 2 deletions src/Engine/ProtoCore/FunctionPointerTable.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System;
using ProtoCore.Utils;
using System;
using System.Collections.Generic;

namespace ProtoCore.DSASM
Expand Down Expand Up @@ -38,7 +39,10 @@ public bool TryGetFunction(StackValue functionPointer, RuntimeCore runtimeCore,
}
else
{
procNode = runtimeCore.DSExecutable.CompleteCodeBlocks[blockId].procedureTable.Procedures[functionIndex];
bool found = runtimeCore.DSExecutable.CompleteCodeBlockDict.TryGetValue(blockId, out CodeBlock codeBlock);
Validity.Assert(found, $"Could find code block with codeBlockId {blockId}");

procNode = codeBlock.procedureTable.Procedures[functionIndex];
}

return true;
Expand Down
3 changes: 2 additions & 1 deletion src/Engine/ProtoCore/Properties/AssemblyInfo.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,5 @@
[assembly:InternalsVisibleTo("DynamoCore")]
[assembly: InternalsVisibleTo("DynamoCoreTests")]
[assembly: InternalsVisibleTo("DynamoCoreWpfTests")]

[assembly: InternalsVisibleTo("ProtoImperative")]
[assembly: InternalsVisibleTo("ProtoScript")]
15 changes: 7 additions & 8 deletions src/Engine/ProtoCore/Utils/ClassUtils.cs
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ public static int GetUpcastCountTo(ClassNode from, ClassNode to, RuntimeCore run
//
// 2.3 Otherwise, classScope == kInvalidIndex && functionScope == kInvalidIndex
// Return public member in derived class, or public member in base classes
public static int GetSymbolIndex(ClassNode classNode, string name, int classScope, int functionScope, int blockId, List<CodeBlock> codeblockList, out bool hasThisSymbol, out ProtoCore.DSASM.AddressType addressType)
public static int GetSymbolIndex(ClassNode classNode, string name, int classScope, int functionScope, int blockId, SortedDictionary<int, CodeBlock> codeblocks, out bool hasThisSymbol, out ProtoCore.DSASM.AddressType addressType)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pinzart I'm sorry I missed this - you can't change this public signature - please follow the same pattern of obsoleting this and adding an overload.

{
hasThisSymbol = false;
addressType = ProtoCore.DSASM.AddressType.Invalid;
Expand All @@ -92,7 +92,7 @@ public static int GetSymbolIndex(ClassNode classNode, string name, int classScop
classNode.ProcTable.Procedures[functionScope].IsStatic;

// Try for member function variables
var blocks = GetAncestorBlockIdsOfBlock(blockId, codeblockList);
var blocks = GetAncestorBlockIdsOfBlock(blockId, codeblocks);
blocks.Insert(0, blockId);

Dictionary<int, SymbolNode> symbolOfBlockScope = new Dictionary<int, SymbolNode>();
Expand Down Expand Up @@ -154,15 +154,14 @@ public static int GetSymbolIndex(ClassNode classNode, string name, int classScop
return Constants.kInvalidIndex;
}

public static List<int> GetAncestorBlockIdsOfBlock(int blockId, List<CodeBlock> codeblockList)
public static List<int> GetAncestorBlockIdsOfBlock(int blockId, SortedDictionary<int, CodeBlock> codeblocks)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pinzart I'm sorry I missed this - you can't change this public signature - please follow the same pattern of obsoleting this and adding an overload.

{
if (blockId >= codeblockList.Count || blockId < 0)
var ancestors = new List<int>();
if (!codeblocks.TryGetValue(blockId, out CodeBlock thisBlock))
{
return new List<int>();
return ancestors;
}
CodeBlock thisBlock = codeblockList[blockId];

var ancestors = new List<int>();

CodeBlock codeBlock = thisBlock.parent;
while (codeBlock != null)
{
Expand Down
2 changes: 0 additions & 2 deletions src/Engine/ProtoCore/Utils/CoreUtils.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@ namespace ProtoCore.Utils
{
public static class CoreUtils
{


public static void InsertPredefinedAndBuiltinMethods(Core core, CodeBlockNode root)
{
if (DSASM.InterpreterMode.Normal == core.Options.RunMode)
Expand Down
17 changes: 8 additions & 9 deletions src/Engine/ProtoImperative/CodeGen.cs
Original file line number Diff line number Diff line change
Expand Up @@ -881,25 +881,24 @@ private void EmitLanguageBlockNode(ImperativeNode node, ref ProtoCore.Type infer
if (propogateUpdateGraphNode != null)
{
propogateUpdateGraphNode.languageBlockId = blockId;
CodeBlock childBlock = core.CompleteCodeBlockList[blockId];
bool foundChild = core.CompleteCodeBlockDict.TryGetValue(blockId, out CodeBlock childBlock);
Validity.Assert(foundChild, $"Could find code block with codeBlockId {blockId}");

foreach (var subGraphNode in childBlock.instrStream.dependencyGraph.GraphList)
{
foreach (var depentNode in subGraphNode.dependentList)
{
if (depentNode.updateNodeRefList != null
&& depentNode.updateNodeRefList.Count > 0
if (depentNode.updateNodeRefList != null
&& depentNode.updateNodeRefList.Count > 0
&& depentNode.updateNodeRefList[0].nodeList != null
&& depentNode.updateNodeRefList[0].nodeList.Count > 0)
{
SymbolNode dependentSymbol = depentNode.updateNodeRefList[0].nodeList[0].symbol;
int symbolBlockId = dependentSymbol.codeBlockId;
if (symbolBlockId != Constants.kInvalidIndex)
if (core.CompleteCodeBlockDict.TryGetValue(symbolBlockId, out CodeBlock symbolBlock) &&
!symbolBlock.IsMyAncestorBlock(codeBlock.codeBlockId))
{
CodeBlock symbolBlock = core.CompleteCodeBlockList[symbolBlockId];
if (!symbolBlock.IsMyAncestorBlock(codeBlock.codeBlockId))
{
propogateUpdateGraphNode.PushDependent(depentNode);
}
propogateUpdateGraphNode.PushDependent(depentNode);
}
}
}
Expand Down
4 changes: 3 additions & 1 deletion src/Engine/ProtoScript/Runners/LiveRunner.cs
Original file line number Diff line number Diff line change
Expand Up @@ -322,8 +322,10 @@ private void ClearModifiedNestedBlocks(List<AssociativeNode> astNodes)

// Remove from the global codeblocks
core.CodeBlockList.RemoveAll(x => x.guid == bnode.guid);// && x.AstID == bnode.OriginalAstID);

// Remove from the runtime codeblocks
core.CompleteCodeBlockList.RemoveAll(x => x.guid == bnode.guid);// && x.AstID == bnode.OriginalAstID);
var keysToRemove = core.CompleteCodeBlockDict.Where(x => x.Value.guid == bnode.guid).Select(x => x.Key).ToList();
keysToRemove.ForEach(key => core.CompleteCodeBlockDict.Remove(key));
}
}
}
Expand Down
Loading