21030: Fixes potential bug when copying the map data from assocs -- t…

…he cycle check could be cleared. (#196)

Also other minor code cleanup.

src/Amalgam/evaluablenode/EvaluableNode.cpp

@@ -436,7 +436,7 @@ void EvaluableNode::CopyValueFrom(EvaluableNode *n)
 		if(n_mcn.empty())
 			ClearMappedChildNodes();
 		else
-			SetMappedChildNodes(n_mcn, true);
+			SetMappedChildNodes(n_mcn, true, n->GetNeedCycleCheck(), n->GetIsIdempotent());
 	}
 	else if(DoesEvaluableNodeTypeUseNumberData(cur_type))
 		GetNumberValueReference() = n->GetNumberValueReference();
@@ -1285,7 +1285,7 @@ EvaluableNode **EvaluableNode::GetOrCreateMappedChildNode(const StringInternPool
 	return &inserted_node->second;
 }
 
-void EvaluableNode::SetMappedChildNodes(AssocType &new_mcn, bool copy)
+void EvaluableNode::SetMappedChildNodes(AssocType &new_mcn, bool copy, bool need_cycle_check, bool is_idempotent)
 {
 	if(!IsAssociativeArray())
 		return;
@@ -1304,34 +1304,12 @@ void EvaluableNode::SetMappedChildNodes(AssocType &new_mcn, bool copy)
 	else
 		mcn.swap(new_mcn);
 
-	//if cycles, propagate upward
-	SetNeedCycleCheck(false);
-	for(auto &[_, cn] : mcn)
-	{
-		if(cn != nullptr && cn->GetNeedCycleCheck())
-		{
-			SetNeedCycleCheck(true);
-			break;
-		}
-	}
+	SetNeedCycleCheck(need_cycle_check);
 
-	//set idempotency
-	if(GetNumLabels() == 0)
-	{
-		//if potentially idempotent, then see if it is
-		if(IsEvaluableNodeTypePotentiallyIdempotent(type))
-		{
-			SetIsIdempotent(true);
-			for(auto &[_, cn] : mcn)
-			{
-				if(cn != nullptr && !cn->GetIsIdempotent())
-				{
-					SetIsIdempotent(false);
-					break;
-				}
-			}
-		}
-	}
+	if(is_idempotent && (GetNumLabels() > 0 || !IsEvaluableNodeTypePotentiallyIdempotent(type)))
+		SetIsIdempotent(false);
+	else
+		SetIsIdempotent(is_idempotent);
 }
 
 std::pair<bool, EvaluableNode **> EvaluableNode::SetMappedChildNode(const std::string &id, EvaluableNode *node, bool overwrite)

src/Amalgam/evaluablenode/EvaluableNode.h

@@ -752,7 +752,8 @@ class EvaluableNode
 	//returns a pointer to the pointer of the child node, creating it if necessary and populating it with a nullptr
 	EvaluableNode **GetOrCreateMappedChildNode(const StringInternPool::StringID sid);
 	// if copy is set to true, then it will copy the map, otherwise it will swap
-	void SetMappedChildNodes(AssocType &new_mcn, bool copy);
+	void SetMappedChildNodes(AssocType &new_mcn, bool copy,
+		bool need_cycle_check = true, bool is_idempotent = false);
 	//if overwrite is true, then it will overwrite the value, otherwise it will only set it if it does not exist
 	// will return true if it was successfully written (false if overwrite is set to false and the key already exists),
 	// as well as a pointer to where the pointer is stored

src/Amalgam/interpreter/Interpreter.h

@@ -633,8 +633,7 @@ class Interpreter
 			EvaluableNode *target_origin, EvaluableNode *target,
 			EvaluableNodeImmediateValueWithType current_index,
 			EvaluableNode *current_value,
-			EvaluableNodeRefType &result,
-			EvaluableNodeReference previous_result = EvaluableNodeReference::Null())
+			EvaluableNodeRefType &result)
 		{
 			//save a location in the stack now to store the result in later
 			resultsSaver.PushEvaluableNode(nullptr);
@@ -645,7 +644,7 @@ class Interpreter
 
 			Concurrency::threadPool.BatchEnqueueTask(
 				[this, interpreter, node_to_execute, target_origin, target, current_index,
-					current_value, &result, previous_result, results_saver_location]
+					current_value, &result, results_saver_location]
 				{
 					EvaluableNodeManager *enm = interpreter->evaluableNodeManager;
 					interpreter->memoryModificationLock = Concurrency::ReadLock(enm->memoryModificationMutex);
@@ -654,7 +653,7 @@ class Interpreter
 					EvaluableNode *construction_stack = enm->AllocListNode(parentInterpreter->constructionStackNodes);
 					std::vector<ConstructionStackIndexAndPreviousResultUniqueness> csiau(parentInterpreter->constructionStackIndicesAndUniqueness);
 					interpreter->PushNewConstructionContextToStack(construction_stack->GetOrderedChildNodes(),
-						csiau, target_origin, target, current_index, current_value, previous_result);
+						csiau, target_origin, target, current_index, current_value, EvaluableNodeReference::Null());
 
 					auto result_ref = interpreter->ExecuteNode(node_to_execute,
 						enm->AllocListNode(parentInterpreter->callStackNodes),

src/Amalgam/interpreter/InterpreterOpcodesListManipulation.cpp

@@ -609,8 +609,8 @@ EvaluableNodeReference Interpreter::InterpretNode_ENT_RANGE(EvaluableNode *en, b
 	auto node_stack = CreateInterpreterNodeStackStateSaver(function);
 
 	EvaluableNodeReference result(evaluableNodeManager->AllocNode(ENT_LIST), true);
-	auto &list_ocn = result->GetOrderedChildNodesReference();
-	list_ocn.resize(num_nodes);
+	auto &result_ocn = result->GetOrderedChildNodesReference();
+	result_ocn.resize(num_nodes);
 
 #ifdef MULTITHREAD_SUPPORT
 	if(en->GetConcurrency() && num_nodes > 1)
@@ -625,7 +625,7 @@ EvaluableNodeReference Interpreter::InterpretNode_ENT_RANGE(EvaluableNode *en, b
 			for(size_t node_index = 0; node_index < num_nodes; node_index++)
 				concurrency_manager.EnqueueTaskWithConstructionStack<EvaluableNode *>(function,
 					nullptr, result, EvaluableNodeImmediateValueWithType(node_index * range_step_size + range_start),
-					nullptr, list_ocn[node_index]);
+					nullptr, result_ocn[node_index]);
 
 			enqueue_task_lock.Unlock();
 			concurrency_manager.EndConcurrency();
@@ -638,7 +638,6 @@ EvaluableNodeReference Interpreter::InterpretNode_ENT_RANGE(EvaluableNode *en, b
 
 	PushNewConstructionContext(nullptr, result, EvaluableNodeImmediateValueWithType(0.0), nullptr);
 
-	auto &result_ocn = result->GetOrderedChildNodesReference();
 	for(size_t i = 0; i < num_nodes; i++)
 	{
 		//pass index of list to be mapped -- leave value at nullptr

src/Amalgam/interpreter/InterpreterOpcodesMath.cpp

@@ -813,7 +813,7 @@ EvaluableNodeReference Interpreter::InterpretNode_ENT_MIN(EvaluableNode *en, boo
 	}
 
 	if(value_found)
-		return AllocReturn(result_value, immediate_result);;
+		return AllocReturn(result_value, immediate_result);
 	return EvaluableNodeReference::Null();
 }