Debugger: Adjust VariablesView for new expression API.

- Simplify handling of expression nodes. For primitive results, we now
  construct a Variable object that represents the expression result, and
  then add that as we would any other local variable. This simplifies handling,
  and also allows saving/restoration of their view state to be handled the
  same as other nodes. Complex expression results aren't yet handled properly,
  pending some further work in progress on the evaluator.

src/apps/debugger/user_interface/gui/team_window/VariablesView.cpp

@@ -26,7 +26,6 @@
 #include "ActionMenuItem.h"
 #include "Architecture.h"
 #include "ExpressionInfo.h"
-#include "ExpressionValueNode.h"
 #include "ExpressionValues.h"
 #include "FileSourceCode.h"
 #include "Function.h"
@@ -41,6 +40,8 @@
 #include "StackTrace.h"
 #include "StackFrame.h"
 #include "StackFrameValues.h"
+#include "StringUtils.h"
+#include "StringValue.h"
 #include "SyntheticPrimitiveType.h"
 #include "TableCellValueRenderer.h"
 #include "Team.h"
@@ -205,6 +206,47 @@ private:
 };
 
 
+// #pragma mark - ExpressionVariableID
+
+
+class VariablesView::ExpressionVariableID : public ObjectID {
+public:
+	ExpressionVariableID(ExpressionInfo* info)
+		:
+		fInfo(info)
+	{
+		fInfo->AcquireReference();
+	}
+
+	virtual ~ExpressionVariableID()
+	{
+		fInfo->ReleaseReference();
+	}
+
+	virtual	bool operator==(const ObjectID& other) const
+	{
+		const ExpressionVariableID* otherID
+			= dynamic_cast<const ExpressionVariableID*>(&other);
+		if (otherID == NULL)
+			return false;
+
+		return fInfo == otherID->fInfo;
+	}
+
+protected:
+	virtual	uint32 ComputeHashValue() const
+	{
+		uint32 hash = *(uint32*)(&fInfo);
+		hash = hash * 19 + StringUtils::HashValue(fInfo->Expression());
+
+		return hash;
+	}
+
+private:
+	ExpressionInfo*	fInfo;
+};
+
+
 // #pragma mark - ModelNode
 
 
@@ -593,7 +635,8 @@ public:
 									const TreeTablePath& path,
 									int32 columnIndex, BToolTip** _tip);
 
-			status_t			AddSyntheticNode(ModelNode* node);
+			status_t			AddSyntheticNode(Variable* variable,
+									ValueNodeChild*& _child);
 			void				RemoveSyntheticNode(ModelNode* node);
 
 private:
@@ -1467,24 +1510,50 @@ VariablesView::VariableTableModel::GetToolTipForTablePath(
 
 
 status_t
-VariablesView::VariableTableModel::AddSyntheticNode(ModelNode* node)
+VariablesView::VariableTableModel::AddSyntheticNode(Variable* variable,
+	ValueNodeChild*& _child)
 {
-	status_t error = node->Init();
+	ValueNodeContainer* container = fNodeManager->GetContainer();
+	AutoLocker<ValueNodeContainer> containerLocker(container);
+
+	_child = new(std::nothrow) VariableValueNodeChild(variable);
+	if (_child == NULL)
+		return B_NO_MEMORY;
+
+	BReference<ValueNodeChild> childReference(_child, true);
+	ValueNode* valueNode;
+	status_t error;
+	if (_child->IsInternal())
+		error = _child->CreateInternalNode(valueNode);
+	else {
+		error = TypeHandlerRoster::Default()->CreateValueNode(_child,
+			_child->GetType(), valueNode);
+	}
+
 	if (error != B_OK)
 		return error;
 
-	int32 index = fNodes.CountItems();
+	_child->SetNode(valueNode);
+	valueNode->ReleaseReference();
+	container->AddChild(_child);
 
-	if (!fNodes.AddItem(node)) {
+	error = _AddNode(variable, NULL, _child);
-		return B_NO_MEMORY;
+	if (error != B_OK) {
-		// NB: we take over the caller's reference
+		container->RemoveChild(_child);
+		return error;
 	}
 
-	fNodeTable.Insert(node);
+	// since we're injecting these nodes synthetically,
+	// we have to manually ask the node manager to create any
+	// applicable children; this would normally be done implicitly
+	// for top level nodes, as they're added from the parameters/locals,
+	// but not here.
+	fNodeManager->AddChildNodes(_child);
 
-	node->NodeChild()->Node()->SetContainer(fNodeManager->GetContainer());
+	ModelNode* childNode = fNodeTable.Lookup(_child);
-
+	if (childNode != NULL)
-	NotifyNodesAdded(TreeTablePath(), index, 1);
+		fContainerListener->ModelNodeValueRequested(childNode);
+	ValueNodeChildrenCreated(_child->Node());
 
 	return B_OK;
 }
@@ -1615,6 +1684,7 @@ VariablesView::VariablesView(Listener* listener)
 	fPreviousViewState(NULL),
 	fViewStateHistory(NULL),
 	fExpressions(NULL),
+	fExpressionChildren(10, false),
 	fTableCellContextMenuTracker(NULL),
 	fFrameClearPending(false),
 	fListener(listener)
@@ -1676,6 +1746,10 @@ VariablesView::SetStackFrame(Thread* thread, StackFrame* stackFrame)
 
 	_FinishContextMenu(true);
 
+	for (int32 i = 0; i < fExpressionChildren.CountItems(); i++)
+		fExpressionChildren.ItemAt(i)->ReleaseReference();
+	fExpressionChildren.MakeEmpty();
+
 	if (fThread != NULL)
 		fThread->ReleaseReference();
 	if (fStackFrame != NULL)
@@ -1975,7 +2049,7 @@ VariablesView::MessageReceived(BMessage* message)
 				valueReference.SetTo(value, true);
 			}
 
-			_SetExpressionNodeValue(info, result, value);
+			_AddExpressionNode(info, result, value);
 			break;
 		}
 		case MSG_VALUE_NODE_CHANGED:
@@ -2410,20 +2484,18 @@ VariablesView::_GetContextActionsForNode(ModelNode* node,
 	BPrivate::ObjectDeleter<ContextActionList> postActionListDeleter(
 		_postActions);
 
-#if 0
 	result = _AddContextAction("Add watch expression" B_UTF8_ELLIPSIS,
 		MSG_ADD_WATCH_EXPRESSION, _postActions, message);
 	if (result != B_OK)
 		return result;
 
-	if (dynamic_cast<ExpressionValueNodeChild*>(node->NodeChild()) != NULL) {
+	if (fExpressionChildren.HasItem(node->NodeChild())) {
 		result = _AddContextAction("Remove watch expression",
 			MSG_REMOVE_WATCH_EXPRESSION, _postActions, message);
 		if (result != B_OK)
 			return result;
 		message->AddPointer("node", node);
 	}
-#endif
 
 	preActionListDeleter.Detach();
 	postActionListDeleter.Detach();
@@ -2599,47 +2671,28 @@ VariablesView::_AddViewStateDescendentNodeInfos(VariablesViewState* viewState,
 
 		Value* value = node->GetValue();
 		Variable* variable = node->GetVariable();
-		if (variable != NULL) {
+		TypeComponentPath* componentPath = node->GetPath();
-			TypeComponentPath* componentPath = node->GetPath();
+		ObjectID* id = variable->ID();
-			ObjectID* id = variable->ID();
 
-			status_t error = viewState->SetNodeInfo(id, componentPath, nodeInfo);
+		status_t error = viewState->SetNodeInfo(id, componentPath, nodeInfo);
+		if (error != B_OK)
+			return error;
+
+		if (value != NULL && updateValues) {
+			BVariant variableValueData;
+			if (value->ToVariant(variableValueData))
+				error = viewState->Values()->SetValue(id, componentPath,
+					variableValueData);
 			if (error != B_OK)
 				return error;
-
-			if (value != NULL && updateValues) {
-				BVariant variableValueData;
-				if (value->ToVariant(variableValueData))
-					error = viewState->Values()->SetValue(id, componentPath,
-						variableValueData);
-				if (error != B_OK)
-					return error;
-			}
-
-			// recurse
-			error = _AddViewStateDescendentNodeInfos(viewState, node, path,
-				updateValues);
-			if (error != B_OK)
-				return error;
-		} else {
-			ExpressionValueNodeChild* child
-				= dynamic_cast<ExpressionValueNodeChild*>(node->NodeChild());
-			if (child != NULL && value != NULL && updateValues) {
-				BVariant variableValueData;
-				if (value->ToVariant(variableValueData)) {
-					FunctionID* id = fStackFrame->Function()->GetFunctionID();
-					if (id == NULL)
-						return B_NO_MEMORY;
-					BReference<FunctionID> functionReference(id, true);
-					status_t error = viewState->GetExpressionValues()
-						->SetValue(id, fThread, child->GetExpression(),
-						variableValueData);
-					if (error != B_OK)
-						return error;
-				}
-			}
 		}
 
+		// recurse
+		error = _AddViewStateDescendentNodeInfos(viewState, node, path,
+			updateValues);
+		if (error != B_OK)
+			return error;
+
 		path.RemoveLastComponent();
 	}
 
@@ -2658,54 +2711,37 @@ VariablesView::_ApplyViewStateDescendentNodeInfos(VariablesViewState* viewState,
 			return B_NO_MEMORY;
 
 		// apply the node's info, if any
-		Variable* variable = node->GetVariable();
+		ObjectID* objectID = node->GetVariable()->ID();
-		if (variable != NULL) {
+		TypeComponentPath* componentPath = node->GetPath();
-			ObjectID* objectID = node->GetVariable()->ID();
+		const VariablesViewNodeInfo* nodeInfo = viewState->GetNodeInfo(
-			TypeComponentPath* componentPath = node->GetPath();
+			objectID, componentPath);
-			const VariablesViewNodeInfo* nodeInfo = viewState->GetNodeInfo(
+		if (nodeInfo != NULL) {
-				objectID, componentPath);
+			// NB: if the node info indicates that the node in question
-			if (nodeInfo != NULL) {
+			// was being cast to a different type, this *must* be applied
-				// NB: if the node info indicates that the node in question
+			// before any other view state restoration, since it
-				// was being cast to a different type, this *must* be applied
+			// potentially changes the child hierarchy under that node.
-				// before any other view state restoration, since it
+			Type* type = nodeInfo->GetCastedType();
-				// potentially changes the child hierarchy under that node.
+			if (type != NULL) {
-				Type* type = nodeInfo->GetCastedType();
+				ValueNode* valueNode = NULL;
-				if (type != NULL) {
+				if (TypeHandlerRoster::Default()->CreateValueNode(
-					ValueNode* valueNode = NULL;
+					node->NodeChild(), type, valueNode) == B_OK) {
-					if (TypeHandlerRoster::Default()->CreateValueNode(
+					node->NodeChild()->SetNode(valueNode);
-						node->NodeChild(), type, valueNode) == B_OK) {
+					node->SetCastedType(type);
-						node->NodeChild()->SetNode(valueNode);
-						node->SetCastedType(type);
-					}
-				}
-
-				// we don't have a renderer yet so we can't apply the settings
-				// at this stage. Store them on the model node so we can lazily
-				// apply them once the value is retrieved.
-				node->SetLastRendererSettings(nodeInfo->GetRendererSettings());
-
-				fVariableTable->SetNodeExpanded(path,
-					nodeInfo->IsNodeExpanded());
-
-				BVariant previousValue;
-				if (viewState->Values()->GetValue(objectID, componentPath,
-					previousValue)) {
-					node->SetPreviousValue(previousValue);
 				}
 			}
-		} else {
+
-			ExpressionValueNodeChild* child
+			// we don't have a renderer yet so we can't apply the settings
-				= dynamic_cast<ExpressionValueNodeChild*>(node->NodeChild());
+			// at this stage. Store them on the model node so we can lazily
-			if (child != NULL) {
+			// apply them once the value is retrieved.
-				BVariant previousValue;
+			node->SetLastRendererSettings(nodeInfo->GetRendererSettings());
-				FunctionID* id = fStackFrame->Function()->GetFunctionID();
+
-				if (id == NULL)
+			fVariableTable->SetNodeExpanded(path,
-					return B_NO_MEMORY;
+				nodeInfo->IsNodeExpanded());
-				BReference<FunctionID> idReference(id, true);
+
-				if (viewState->GetExpressionValues()->GetValue(id, fThread,
+			BVariant previousValue;
-					child->GetExpression(), previousValue)) {
+			if (viewState->Values()->GetValue(objectID, componentPath,
-					node->SetPreviousValue(previousValue);
+				previousValue)) {
-				}
+				node->SetPreviousValue(previousValue);
 			}
 		}
 
@@ -2776,10 +2812,6 @@ VariablesView::_AddExpression(const char* expression, ExpressionInfo*& _info)
 
 	BReference<ExpressionInfo> infoReference(info, true);
 
-	status_t error = _AddExpressionNode(info);
-	if (error != B_OK)
-		return error;
-
 	if (!entry->AddItem(info))
 		return B_NO_MEMORY;
 
@@ -2793,9 +2825,7 @@ VariablesView::_AddExpression(const char* expression, ExpressionInfo*& _info)
 void
 VariablesView::_RemoveExpression(ModelNode* node)
 {
-	ExpressionValueNodeChild* child
+	if (!fExpressionChildren.HasItem(node->NodeChild()))
-		= dynamic_cast<ExpressionValueNodeChild*>(node->NodeChild());
-	if (child == NULL)
 		return;
 
 	FunctionID* id = fStackFrame->Function()->GetFunctionID();
@@ -2807,7 +2837,7 @@ VariablesView::_RemoveExpression(ModelNode* node)
 
 	for (int32 i = 0; i < entry->CountItems(); i++) {
 		ExpressionInfo* info = entry->ItemAt(i);
-		if (info->Expression() == child->GetExpression()) {
+		if (info->Expression() == node->Name()) {
 			entry->RemoveItemAt(i);
 			info->RemoveListener(this);
 			info->ReleaseReference();
@@ -2819,46 +2849,6 @@ VariablesView::_RemoveExpression(ModelNode* node)
 }
 
 
-status_t
-VariablesView::_AddExpressionNode(ExpressionInfo* info)
-{
-#if 0
-	ExpressionValueNodeChild* child
-		= new(std::nothrow) ExpressionValueNodeChild(info->Expression(), type);
-	if (child == NULL)
-		return B_NO_MEMORY;
-
-	BReference<ValueNodeChild> childReference(child, true);
-
-	ExpressionValueNode* expressionNode
-		= new(std::nothrow) ExpressionValueNode(child, type);
-	if (expressionNode == NULL)
-		return B_NO_MEMORY;
-
-	BReference<ValueNode> expressionNodeReference(expressionNode, true);
-
-	child->SetNode(expressionNode);
-
-	ModelNode* modelNode = new(std::nothrow) ModelNode(NULL, NULL,
-		child, true);
-	if (modelNode == NULL)
-		return B_NO_MEMORY;
-
-	BReference<ModelNode> modelNodeReference(modelNode, true);
-
-	status_t error = fVariableTableModel->AddSyntheticNode(modelNode);
-	if (error != B_OK)
-		return error;
-
-	expressionNodeReference.Detach();
-	modelNodeReference.Detach();
-
-	return B_OK;
-#endif
-	return B_NOT_SUPPORTED;
-}
-
-
 void
 VariablesView::_RestoreExpressionNodes()
 {
@@ -2878,45 +2868,90 @@ VariablesView::_RestoreExpressionNodes()
 
 	for (int32 i = 0; i < entry->CountItems(); i++) {
 		ExpressionInfo* info = entry->ItemAt(i);
-		_AddExpressionNode(info);
 		fListener->ExpressionEvaluationRequested(info, fStackFrame, fThread);
 	}
 }
 
 
 void
-VariablesView::_SetExpressionNodeValue(ExpressionInfo* info, status_t result,
+VariablesView::_AddExpressionNode(ExpressionInfo* info, status_t result,
 	ExpressionResult* value)
 {
-	FunctionInstance* instance = fStackFrame->Function();
+	Variable* variable = NULL;
-	if (instance == NULL)
+	BReference<Variable> variableReference;
-		return;
+	BVariant valueData;
 
-	FunctionID* id = instance->GetFunctionID();
+	Value* primitive = value->PrimitiveValue();
+	if (primitive != NULL) {
+		if (!primitive->ToVariant(valueData))
+			return;
+	} else
+		valueData.SetTo("Unsupported expression result type.");
+
+	ExpressionVariableID* id
+		= new(std::nothrow) ExpressionVariableID(info);
 	if (id == NULL)
 		return;
+	BReference<ObjectID> idReference(id, true);
 
-	BReference<FunctionID> idReference(id, true);
+	Type* type = NULL;
+	if (_GetTypeForTypeCode(valueData.Type(), type) != B_OK)
+		return;
+	BReference<Type> typeReference(type, true);
 
-	ExpressionInfoEntry* entry = fExpressions->Lookup(FunctionKey(id));
+	ValueLocation* location = new(std::nothrow) ValueLocation();
-	if (entry == NULL)
+	if (location == NULL)
+		return;
+	BReference<ValueLocation> locationReference(location, true);
+	if (valueData.IsNumber()) {
+
+		ValuePieceLocation piece;
+		if (!piece.SetToValue(valueData.Bytes(), valueData.Size())
+			|| !location->AddPiece(piece)) {
+			return;
+		}
+	}
+
+	variable = new(std::nothrow) Variable(id,
+		info->Expression(), type, location);
+	if (variable == NULL)
+		return;
+	variableReference.SetTo(variable, true);
+
+	ValueNodeChild* child = NULL;
+	status_t error = fVariableTableModel->AddSyntheticNode(variable, child);
+	if (error != B_OK)
 		return;
 
-	void* rootNode = fVariableTableModel->Root();
+	// In the case of either an evaluation error, or an unsupported result
-	for (int32 i = 0; i < fVariableTableModel->CountChildren(rootNode); i++) {
+	// type, set an explanatory string for the result directly.
-		ModelNode* node = (ModelNode*)fVariableTableModel->ChildAt(rootNode,
+	if (result != B_OK || valueData.Type() == B_STRING_TYPE) {
-			i);
+		StringValue* explicitValue = new(std::nothrow) StringValue(
-		ExpressionValueNodeChild* child
+			valueData.ToString());
-			= dynamic_cast<ExpressionValueNodeChild*>(node->NodeChild());
+		if (explicitValue == NULL)
-		if (child == NULL)
+			return;
-			continue;
 
-		if (child->GetExpression() != info->Expression())
+		child->Node()->SetLocationAndValue(NULL, explicitValue, B_OK);
-			continue;
+	}
 
-		child->Node()->SetLocationAndValue(NULL, value->PrimitiveValue(),
+	if (fExpressionChildren.AddItem(child)) {
-			result);
+		child->AcquireReference();
-		return;
+
+		// attempt to restore our newly added node's view state,
+		// if applicable.
+		FunctionID* functionID = fStackFrame->Function()
+			->GetFunctionID();
+		if (functionID == NULL)
+			return;
+		BReference<FunctionID> functionIDReference(functionID,
+			true);
+		VariablesViewState* viewState = fViewStateHistory
+			->GetState(fThread->ID(), functionID);
+		if (viewState != NULL) {
+			TreeTablePath path;
+			_ApplyViewStateDescendentNodeInfos(viewState,
+				fVariableTableModel->Root(), path);
+		}
 	}
 }
 
@@ -2924,7 +2959,7 @@ VariablesView::_SetExpressionNodeValue(ExpressionInfo* info, status_t result,
 status_t
 VariablesView::_GetTypeForTypeCode(int32 type, Type*& _resultType) const
 {
-	if (BVariant::TypeIsNumber(type)) {
+	if (BVariant::TypeIsNumber(type) || type == B_STRING_TYPE) {
 		_resultType = new(std::nothrow) SyntheticPrimitiveType(type);
 		if (_resultType == NULL)
 			return B_NO_MEMORY;

src/apps/debugger/user_interface/gui/team_window/VariablesView.h

@@ -23,6 +23,7 @@ class Thread;
 class Type;
 class TypeComponentPath;
 class ValueNode;
+class ValueNodeChild;
 class ValueNodeContainer;
 class Value;
 class Variable;
@@ -70,6 +71,7 @@ private:
 
 private:
 			class ContainerListener;
+			class ExpressionVariableID;
 			class ModelNode;
 			class VariableValueColumn;
 			class VariableTableModel;
@@ -77,6 +79,7 @@ private:
 			class TableCellContextMenuTracker;
 			typedef BObjectList<ActionMenuItem> ContextActionList;
 			typedef BObjectList<ExpressionInfo> ExpressionInfoList;
+			typedef BObjectList<ValueNodeChild> ExpressionChildList;
 
 			struct FunctionKey;
 			struct ExpressionInfoEntry;
@@ -113,10 +116,9 @@ private:
 									ExpressionInfo*& _info);
 			void				_RemoveExpression(ModelNode* node);
 
-			status_t			_AddExpressionNode(ExpressionInfo* info);
 			void				_RestoreExpressionNodes();
 
-			void				_SetExpressionNodeValue(ExpressionInfo* info,
+			void				_AddExpressionNode(ExpressionInfo* info,
 									status_t result, ExpressionResult* value);
 
 			status_t			_GetTypeForTypeCode(int32 typeCode,
@@ -131,6 +133,7 @@ private:
 			VariablesViewState*	fPreviousViewState;
 			VariablesViewStateHistory* fViewStateHistory;
 			ExpressionInfoTable* fExpressions;
+			ExpressionChildList	fExpressionChildren;
 			TableCellContextMenuTracker* fTableCellContextMenuTracker;
 			bool				fFrameClearPending;
 			Listener*			fListener;

src/apps/debugger/user_interface/util/UiUtils.cpp

@@ -479,6 +479,8 @@ UiUtils::TypeCodeToString(type_code type)
 			return "float";
 		case B_DOUBLE_TYPE:
 			return "double";
+		case B_STRING_TYPE:
+			return "string";
 		default:
 			return "unknown";
 	}