crash when undoing deletion of function definition code blocks (#11220)

* account for deletion of nodes

Co-authored-by: pinzart <[email protected]>

src/Engine/ProtoCore/CodeGen.cs

@@ -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
@@ -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;

src/Engine/ProtoCore/Core.cs

@@ -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
@@ -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)
                     {
@@ -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;
@@ -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;
@@ -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)

src/Engine/ProtoCore/DSASM/Executable.cs

@@ -1,5 +1,6 @@
 using System;
 using System.Collections.Generic;
+using System.Linq;
 using ProtoCore.AssociativeGraph;
 using ProtoCore.Lang;
 using ProtoFFI;
@@ -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; }
@@ -94,7 +101,7 @@ public void Reset()
             instrStreamList = null;
             iStreamCanvas = null;
             CodeBlocks = null;
-            CompleteCodeBlocks = null;
+            CompleteCodeBlockDict = null;
             ContextDataMngr = null;
             CodeToLocation = null;
             CurrentDSFileName = string.Empty;
@@ -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;
 

src/Engine/ProtoCore/DSASM/Executive.cs

@@ -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;
                             }
@@ -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)
                     {
@@ -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);
@@ -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];
@@ -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++;

src/Engine/ProtoCore/DSASM/Mirror/ExecutionMirror.cs

@@ -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
                 //
@@ -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);
@@ -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);
                 }
             }

src/Engine/ProtoCore/FunctionPointerTable.cs

@@ -1,4 +1,5 @@
-using System;
+using ProtoCore.Utils;
+using System;
 using System.Collections.Generic;
 
 namespace ProtoCore.DSASM
@@ -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;

src/Engine/ProtoCore/Properties/AssemblyInfo.cs

@@ -14,4 +14,5 @@
 [assembly:InternalsVisibleTo("DynamoCore")]
 [assembly: InternalsVisibleTo("DynamoCoreTests")]
 [assembly: InternalsVisibleTo("DynamoCoreWpfTests")]
-
+[assembly: InternalsVisibleTo("ProtoImperative")]
+[assembly: InternalsVisibleTo("ProtoScript")]

src/Engine/ProtoCore/Utils/ClassUtils.cs

@@ -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)
         {
             hasThisSymbol = false;
             addressType = ProtoCore.DSASM.AddressType.Invalid;
@@ -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>();
@@ -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)
         {
-            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)
             {

src/Engine/ProtoCore/Utils/CoreUtils.cs

@@ -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)

src/Engine/ProtoImperative/CodeGen.cs

@@ -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);
                                 }
                             }
                         }

src/Engine/ProtoScript/Runners/LiveRunner.cs

@@ -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));
                 }
             }
         }