Bug 399494 - Consider all variable objects as not complex

There are cases where we consider some variables as complex when they
are not.  In particular, if a pointer is declared using a typedef, is
will be considered complex with the current code.  This is because it
has a child (the pointed value), but CDT doesn't know it's a pointer.

One of the consequence is that we assume the value is not modifiable.
Therefore, we won't update its value when it changes, and we won't let
the user edit it.

Initially I thought it would be safe to assume that variables with two
or more children are complex, but pointers to structures have as many
children as the structure has fields.  Therefore, a pointer to a
structure, declared as a typedef, will still be wrongfully considered as
complex.  Since there's no easy way to know for sure whether a variable
is complex, just assume everything is simple.

I added a test to verify that the value of a pointer declared using a
typedef will update correctly in CDT as it changes in the program.
There are two distinct scenarios, pointers that are variables and
pointers that are fields of structures.  Instead of adding content to
testUpdateOfPointer, I decided to make a similar test method,
testUpdateOfPointerTypedef.  The original test method was getting too
long and was difficult to follow.  I think it's good to keep them short
and focused.

Another test verifies that the same kind of pointer can properly be
written/modified by the user.

Change-Id: If43b3b6e49cd4a20ea929c2a096745a32de14cd0
Signed-off-by: Simon Marchi <simon.marchi@polymtl.ca>

3 files changed, 93 insertions, 6 deletions

diff --git a/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/MIVariableManager.java b/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/MIVariableManager.java
index 79d9ea4d189..f64265fe90d 100644
--- a/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/MIVariableManager.java
+++ b/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/MIVariableManager.java
@@ -532,10 +532,11 @@ public class MIVariableManager implements ICommandControl {
 		// children taking into account RTTI ("set print object on")).
 		// Note that the numChildrenHint can be trusted when asking if the number of children is 0 or not
 		public boolean isComplex() {
-			return (getGDBType() == null) ? false
-					: getGDBType().getType() != GDBType.POINTER && getGDBType().getType() != GDBType.REFERENCE
-							&& (getNumChildrenHint() > 0
-									|| hasMore() || getDisplayHint().isCollectionHint());
+			// Since we can't reliably determine whether a variable is complex
+			// (see bug 399494), we err on the safe side and assume they are
+			// not. In the end we'll end up asking GDB a bit more things, but we
+			// should at least get correct results.
+			return false;
 		}
 		
 		/**
diff --git a/dsf-gdb/org.eclipse.cdt.tests.dsf.gdb/data/launch/src/ExpressionTestApp.cc b/dsf-gdb/org.eclipse.cdt.tests.dsf.gdb/data/launch/src/ExpressionTestApp.cc
index aa4ef2089dc..fe113029acd 100644
--- a/dsf-gdb/org.eclipse.cdt.tests.dsf.gdb/data/launch/src/ExpressionTestApp.cc
+++ b/dsf-gdb/org.eclipse.cdt.tests.dsf.gdb/data/launch/src/ExpressionTestApp.cc
@@ -132,7 +132,10 @@ int testChildren() {
 }
 
 int testWrite() {
+	typedef int *intPtr;
+
 	int a[2] = {3, 456};
+	intPtr ptr = 0;
 	
 	return 0;
 }
@@ -302,6 +305,31 @@ int testUpdateOfPointer() {
 	return 0;
 }
 
+int testUpdateOfPointerTypedef() {
+	typedef int *intPtr;
+
+	int int1 = 1;
+	int int2 = 2;
+	int int3 = 3;
+	int int4 = 4;
+
+	struct {
+		intPtr ptr;
+	} s;
+
+	intPtr ptr = &int1;
+	s.ptr = &int2;
+
+	/* testUpdateOfPointerTypedef_1 */
+
+	ptr = &int3;
+	s.ptr = &int4;
+
+	/* testUpdateOfPointerTypedef_2 */
+
+	return 0;
+}
+
 int testCanWrite() {
 	int a = 1;
 	int* b = &a;
@@ -428,6 +456,7 @@ int main() {
     testConcurrentReadAndUpdateChild();
     testConcurrentUpdateOutOfScopeChildThenParent();
     testUpdateOfPointer();
+    testUpdateOfPointerTypedef();
     testCanWrite();
     testArrays();
     testRTTI();
diff --git a/dsf-gdb/org.eclipse.cdt.tests.dsf.gdb/src/org/eclipse/cdt/tests/dsf/gdb/tests/MIExpressionsTest.java b/dsf-gdb/org.eclipse.cdt.tests.dsf.gdb/src/org/eclipse/cdt/tests/dsf/gdb/tests/MIExpressionsTest.java
index 8f8691ccfcb..251ee2df9d6 100644
--- a/dsf-gdb/org.eclipse.cdt.tests.dsf.gdb/src/org/eclipse/cdt/tests/dsf/gdb/tests/MIExpressionsTest.java
+++ b/dsf-gdb/org.eclipse.cdt.tests.dsf.gdb/src/org/eclipse/cdt/tests/dsf/gdb/tests/MIExpressionsTest.java
@@ -96,6 +96,8 @@ public class MIExpressionsTest extends BaseParametrizedTestCase {
 	private static final String[] LINE_TAGS = new String[] {
 		"testUpdateOfPointer_1",
 		"testUpdateOfPointer_2",
+		"testUpdateOfPointerTypedef_1",
+		"testUpdateOfPointerTypedef_2",
 	};
 
     @Override
@@ -510,10 +512,10 @@ public class MIExpressionsTest extends BaseParametrizedTestCase {
 	@Test
 	public void testWriteVariable() throws Throwable {
 		SyncUtil.runToLocation("testWrite");
-		MIStoppedEvent stoppedEvent = SyncUtil.step(1, StepType.STEP_OVER);
+		MIStoppedEvent stoppedEvent = SyncUtil.step(2, StepType.STEP_OVER);
 
 		final IFrameDMContext frameDmc = SyncUtil.getStackFrame(stoppedEvent.getDMContext(), 0);
-		final IExpressionDMContext exprDmc = SyncUtil.createExpression(frameDmc, "a[1]");
+		IExpressionDMContext exprDmc = SyncUtil.createExpression(frameDmc, "a[1]");
 
 		writeAndCheck(exprDmc, "987", IFormattedValues.DECIMAL_FORMAT, "987");
 		writeAndCheck(exprDmc, "16", IFormattedValues.HEX_FORMAT, "22");
@@ -524,6 +526,9 @@ public class MIExpressionsTest extends BaseParametrizedTestCase {
 		writeAndCheck(exprDmc, "0b1001", IFormattedValues.BINARY_FORMAT, "9");
 		writeAndCheck(exprDmc, "456", IFormattedValues.NATURAL_FORMAT, "456");
 
+		exprDmc = SyncUtil.createExpression(frameDmc, "ptr");
+
+		writeAndCheck(exprDmc, "0x10", IFormattedValues.HEX_FORMAT, "16");
 	}
 
 	/*
@@ -2633,6 +2638,58 @@ public class MIExpressionsTest extends BaseParametrizedTestCase {
     }
 
     /**
+	 * This test is similar to {@link #testUpdateOfPointer()
+	 * testUpdateOfPointer}, but uses a pointer declared using a typedef. We
+	 * test both a pointer that is a root varobj (a variable) and a pointer that
+	 * is a child varobj (field in a structure).
+	 */
+    @Test
+    public void testUpdateOfPointerTypedef() throws Throwable {
+		/* Places we're going to run to. */
+		String tag1 = String.format("%s:%d", SOURCE_NAME, getLineForTag("testUpdateOfPointerTypedef_1"));
+		String tag2 = String.format("%s:%d", SOURCE_NAME, getLineForTag("testUpdateOfPointerTypedef_2"));
+
+		MIStoppedEvent stoppedEvent = SyncUtil.runToLocation(tag1);
+		IFrameDMContext frameDmc = SyncUtil.getStackFrame(stoppedEvent.getDMContext(), 0);
+
+		/* Create expression for the structure. */
+		IExpressionDMContext structDmc = SyncUtil.createExpression(frameDmc, "s");
+
+		/* Create expression for the pointer variable and its target. */
+		IExpressionDMContext pointerVarDmc = SyncUtil.createExpression(frameDmc, "ptr");
+		IExpressionDMContext pointerVarTargetDmc = SyncUtil.getSubExpression(pointerVarDmc);
+
+		/* Create expression for the pointer field and its target. */
+		IExpressionDMContext pointerFieldDmc = SyncUtil.getSubExpression(structDmc);
+		IExpressionDMContext pointerFieldTargetDmc = SyncUtil.getSubExpression(pointerFieldDmc);
+
+		/* Get the values of the pointers. */
+		String pointerVarValue1 = SyncUtil.getExpressionValue(pointerVarDmc, IFormattedValues.NATURAL_FORMAT);
+		String pointerFieldValue1 = SyncUtil.getExpressionValue(pointerFieldDmc, IFormattedValues.NATURAL_FORMAT);
+
+		/* Verify the pointed values. */
+		String pointerVarTargetValue = SyncUtil.getExpressionValue(pointerVarTargetDmc, IFormattedValues.NATURAL_FORMAT);
+		String pointerFieldTargetValue = SyncUtil.getExpressionValue(pointerFieldTargetDmc, IFormattedValues.NATURAL_FORMAT);
+		assertThat(pointerVarTargetValue, is("1"));
+		assertThat(pointerFieldTargetValue, is("2"));
+
+		/* Run to the second tag. */
+		SyncUtil.runToLocation(tag2);
+
+		/* Get the new values of the pointers and make sure they have changed. */
+		String pointerVarValue2 = SyncUtil.getExpressionValue(pointerVarDmc, IFormattedValues.NATURAL_FORMAT);
+		String pointerFieldValue2 = SyncUtil.getExpressionValue(pointerFieldDmc, IFormattedValues.NATURAL_FORMAT);
+		assertThat(pointerVarValue2, is(not(equalTo(pointerVarValue1))));
+		assertThat(pointerFieldValue2, is(not(equalTo(pointerFieldValue1))));
+
+		/* Verify the new pointed values. */
+		pointerVarTargetValue = SyncUtil.getExpressionValue(pointerVarTargetDmc, IFormattedValues.NATURAL_FORMAT);
+		pointerFieldTargetValue = SyncUtil.getExpressionValue(pointerFieldTargetDmc, IFormattedValues.NATURAL_FORMAT);
+		assertThat(pointerVarTargetValue, is("3"));
+		assertThat(pointerFieldTargetValue, is("4"));
+    }
+
+    /**
      * This test verifies that we properly return if we can write to different expressions
      */
     @Test