BUg 508254 - Move the recursion protection set in CPPVariable.getInitialValue() to EvalUtil.getVariableValue()

This catches recursion along paths in EvalUtil.getVariableValue() that don't
go through CPPVariabble.getInitialValue().

The patch also improves caching in EvalInitList.

Change-Id: I343bbf1bb7493b2c83771de3659209e5d512fd80

core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/internal/index/tests/IndexCPPBindingResolutionTest.java

@@ -1970,7 +1970,27 @@ public class IndexCPPBindingResolutionTest extends IndexBindingResolutionTestBas
 	//	constexpr S waldo = { nullptr, waldo.a };
 	
 	//  // empty file
-	public void testVariableInitializerThatReferencesVariable_508254() throws Exception {
+	public void testVariableInitializerThatReferencesVariable_508254a() throws Exception {
+		checkBindings();
+	}
+	
+	//	struct S {
+	//	    int* a;
+	//	    int* b;
+	//	};
+	//
+	//	constexpr S waldo = { nullptr, waldo.a };
+	//
+	//	struct T {
+	//	    int *pBlock;
+	//	};
+	//
+	//	static const constexpr T greebo[] = {
+	//		{ waldo.a },
+	//	};
+	
+	//	// empty file
+	public void testVariableInitializerThatReferencesVariable_508254b() throws Exception {
 		checkBindings();
 	}
 

core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/CPPVariable.java

@@ -17,9 +17,6 @@ import static org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.SemanticUti
 import static org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.SemanticUtil.REF;
 import static org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.SemanticUtil.TDEF;
 
-import java.util.HashSet;
-import java.util.Set;
-
 import org.eclipse.cdt.core.dom.ILinkage;
 import org.eclipse.cdt.core.dom.ast.DOMException;
 import org.eclipse.cdt.core.dom.ast.IASTDeclSpecifier;
@@ -59,17 +56,6 @@ public class CPPVariable extends PlatformObject implements ICPPInternalVariable
 	private IType fType;
 	private boolean fAllResolved;
 
-	/**
-	 * The set of CPPVariable objects for which initial value computation is in progress on each thread.
-	 * This is used to guard against recursion during initial value computation.
-	 */
-	private static final ThreadLocal<Set<CPPVariable>> fInitialValueInProgress = new ThreadLocal<Set<CPPVariable>>() {
-		@Override
-		protected Set<CPPVariable> initialValue() {
-			return new HashSet<>();
-		}
-	};
-
 	public CPPVariable(IASTName name) {
 	    boolean isDef = name != null && name.isDefinition();
 	    if (name instanceof ICPPASTQualifiedName) {
@@ -237,11 +223,6 @@ public class CPPVariable extends PlatformObject implements ICPPInternalVariable
 
 	@Override
 	public IValue getInitialValue() {
-		Set<CPPVariable> recursionProtectionSet = fInitialValueInProgress.get();
-		if (!recursionProtectionSet.add(this)) {
-			return IntegralValue.UNKNOWN;
-		}
-		try {
 		IValue initialValue = null;
 		final IType nestedType = SemanticUtil.getNestedType(getType(), TDEF | REF | CVTYPE);
 		if (nestedType instanceof ICPPClassType || (initialValue = VariableHelpers.getInitialValue(fDefinition, fDeclarations, getType())) == IntegralValue.UNKNOWN) {
@@ -255,9 +236,6 @@ public class CPPVariable extends PlatformObject implements ICPPInternalVariable
 			return DependentValue.create(initEval);
 		}
 		return initialValue;
-		} finally {
-			recursionProtectionSet.remove(this);
-		}
 	}
 
 	private IASTDeclarator findDeclarator() {

core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/semantics/EvalInitList.java

@@ -33,6 +33,8 @@ import org.eclipse.core.runtime.CoreException;
  */
 public class EvalInitList extends CPPDependentEvaluation {
 	private final ICPPEvaluation[] fClauses;
+	private boolean fCheckedIsValueDependent;
+	private boolean fIsValueDependent;
 	private boolean fCheckedIsConstantExpression;
 	private boolean fIsConstantExpression;
 
@@ -66,7 +68,11 @@ public class EvalInitList extends CPPDependentEvaluation {
 
 	@Override
 	public boolean isValueDependent() {
-		return containsDependentValue(fClauses);
+		if (!fCheckedIsValueDependent) {
+			fCheckedIsValueDependent = true;
+			fIsValueDependent = containsDependentValue(fClauses);
+		}
+		return fIsValueDependent;
 	}
 
 	@Override
@@ -147,8 +153,10 @@ public class EvalInitList extends CPPDependentEvaluation {
 				clauses[i] = clause;
 			}
 		}
-		EvalInitList evalInit = new EvalInitList(clauses, this.getTemplateDefinition());
+		if (clauses == fClauses) {
-		return evalInit;
+			return this;
+		}
+		return new EvalInitList(clauses, this.getTemplateDefinition());
 	}
 
 	@Override

core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/semantics/EvalUtil.java

@@ -12,6 +12,9 @@ import static org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.SemanticUti
 import static org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.SemanticUtil.REF;
 import static org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.SemanticUtil.TDEF;
 
+import java.util.HashSet;
+import java.util.Set;
+
 import org.eclipse.cdt.core.dom.ast.IASTExpression.ValueCategory;
 import org.eclipse.cdt.core.dom.ast.IASTStatement;
 import org.eclipse.cdt.core.dom.ast.IBinding;
@@ -29,6 +32,17 @@ import org.eclipse.cdt.internal.core.dom.parser.cpp.ICPPExecution;
 import org.eclipse.cdt.internal.core.dom.parser.cpp.ICPPExecutionOwner;
 
 public class EvalUtil {
+	/**
+	 * The set of ICPPVariable objects for which initial value computation is in progress on each thread.
+	 * This is used to guard against recursion during initial value computation.
+	 */
+	private static final ThreadLocal<Set<ICPPVariable>> fInitialValueInProgress = new ThreadLocal<Set<ICPPVariable>>() {
+		@Override
+		protected Set<ICPPVariable> initialValue() {
+			return new HashSet<>();
+		}
+	};
+
 	public static IValue getConditionExprValue(ICPPEvaluation conditionExprEval, ActivationRecord record, ConstexprEvaluationContext context) {
 		return conditionExprEval.computeForFunctionCall(record, context.recordStep()).getValue(context.getPoint());
 	}
@@ -137,6 +151,11 @@ public class EvalUtil {
 	 * the given activation record.
 	 */
 	public static ICPPEvaluation getVariableValue(ICPPVariable variable, ActivationRecord record) {
+		Set<ICPPVariable> recursionProtectionSet = fInitialValueInProgress.get();
+		if (!recursionProtectionSet.add(variable)) {
+			return EvalFixed.INCOMPLETE;
+		}
+		try {
 			IType type = variable.getType();
 			IType nestedType = SemanticUtil.getNestedType(type, TDEF | REF | CVTYPE);
 			IValue initialValue = variable.getInitialValue();
@@ -163,5 +182,8 @@ public class EvalUtil {
 				return null;
 			}
 			return valueEval;
+		} finally {
+			recursionProtectionSet.remove(variable);
+		}
 	}
 }