Bug 450888. Fixed the problem decribed in comment #0

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

@@ -63,7 +63,6 @@ import org.eclipse.cdt.core.parser.util.ObjectMap;
 import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPTemplateTypeArgument;
 import org.eclipse.cdt.internal.core.dom.parser.cpp.ClassTypeHelper;
 import org.eclipse.cdt.internal.core.dom.parser.cpp.ICPPInstanceCache;
-import org.eclipse.core.runtime.CoreException;
 
 /**
  * For testing PDOM binding resolution
@@ -103,7 +102,7 @@ public class IndexCPPBindingResolutionBugs extends IndexBindingResolutionTestBas
 	// OBJ {}
 	// FUNC() {}
 	// FUNC2(1) {}
-	public void testBug208558() throws CoreException {
+	public void testBug208558() throws Exception {
 		IIndex index= getIndex();
 
 		IIndexMacro[] macrosA= index.findMacros("OBJ".toCharArray(), IndexFilter.ALL, npm());
@@ -1159,7 +1158,7 @@ public class IndexCPPBindingResolutionBugs extends IndexBindingResolutionTestBas
 	//      useBase(x.d);
 	//	}
 	public void testLateDefinitionOfInheritance_Bug292749() throws Exception {
-    	getBindingFromASTName("useBase(x.d", 7, ICPPFunction.class);
+    	getBindingFromFirstIdentifier("useBase(x.d)", ICPPFunction.class);
 	}
 
 	// namespace one {
@@ -1369,14 +1368,17 @@ public class IndexCPPBindingResolutionBugs extends IndexBindingResolutionTestBas
 	//	    waldo(new B<E>());
 	//	  }
 	//	};
-	public void _testTemplateArgumentResolution_450888() throws Exception {
+	public void testTemplateArgumentResolution_450888() throws Exception {
-		getProblemFromFirstIdentifier("waldo"); // waldo is not resolved because E doesn't extend C.
+		getProblemFromFirstIdentifier("waldo"); // waldo is unresolved because E doesn't extend C.
 		IASTTranslationUnit ast = strategy.getAst(0);
 		ITranslationUnit tu = ast.getOriginatingTranslationUnit();
 		IWorkingCopy workingCopy = tu.getWorkingCopy();
 		IBuffer buffer = workingCopy.getBuffer();
 		buffer.setContents(buffer.getContents().replace("E {", "E : public C<int> {"));
+		// Release and re-acquire the index lock to clear the caches. 
+		getIndex().releaseReadLock();
+		getIndex().acquireReadLock();
 		ast = workingCopy.getAST(strategy.getIndex(), ITranslationUnit.AST_SKIP_INDEXED_HEADERS);
-		checkBindings(ast); // E now extends C, there should be no unresolved symbols.
+		checkBindings(ast);
 	}
 }

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

@@ -813,8 +813,8 @@ public class CPPSemantics {
 		if (binding == null)
 			return null;
         IScope scope = binding.getScope();
-        if (scope instanceof IIndexScope && tu != null) {
+        if (tu != null) {
-        	scope= tu.mapToASTScope((IIndexScope) scope);
+        	scope= tu.mapToASTScope(scope);
         }
         while (scope != null && !(scope instanceof ICPPNamespaceScope)) {
             scope = getParentScope(scope, tu);
@@ -982,7 +982,7 @@ public class CPPSemantics {
 		}
 
 		while (nextScope != null || nextTmplScope != null) {
-			// when the non-template scope is no longer contained within the first template scope,
+			// When the non-template scope is no longer contained within the first template scope,
 			// we use the template scope for the next iteration.
 			boolean useTemplScope= false;
 			if (nextTmplScope != null) {
@@ -996,8 +996,8 @@ public class CPPSemantics {
 			}
 			ICPPScope scope= useTemplScope ? nextTmplScope : nextScope;
 			CPPASTTranslationUnit tu = data.getTranslationUnit();
-			if (scope instanceof IIndexScope && tu != null) {
+			if (tu != null) {
-				scope= (ICPPScope) tu.mapToASTScope(((IIndexScope) scope));
+				scope= (ICPPScope) tu.mapToASTScope((scope));
 			}
 
 			if (!data.usingDirectivesOnly && !(data.ignoreMembers && scope instanceof ICPPClassScope)) {
@@ -1355,13 +1355,13 @@ public class CPPSemantics {
 
 	static ICPPScope getParentScope(IScope scope, ICPPASTTranslationUnit unit) throws DOMException {
 		IScope parentScope= scope.getParent();
-		// the index cannot return the translation unit as parent scope
+		// The index cannot return the translation unit as parent scope.
 		if (unit instanceof CPPASTTranslationUnit) {
 			if (parentScope == null
 					&& (scope instanceof IIndexScope || scope instanceof ICPPClassSpecializationScope)) {
 				parentScope = unit.getScope();
-			} else if (parentScope instanceof IIndexScope) {
+			} else {
-				parentScope = ((CPPASTTranslationUnit) unit).mapToASTScope((IIndexScope) parentScope);
+				parentScope = ((CPPASTTranslationUnit) unit).mapToASTScope(parentScope);
 			}
 		}
 		return (ICPPScope) parentScope;
@@ -1377,8 +1377,8 @@ public class CPPSemantics {
 			ICPPUsingDirective directive, Set<ICPPNamespaceScope> handled) throws DOMException {
 		ICPPNamespaceScope nominated= directive.getNominatedScope();
 		CPPASTTranslationUnit tu= data.getTranslationUnit();
-		if (nominated instanceof IIndexScope && tu != null) {
+		if (tu != null) {
-			nominated= (ICPPNamespaceScope) tu.mapToASTScope((IIndexScope) nominated);
+			nominated= (ICPPNamespaceScope) tu.mapToASTScope(nominated);
 		}
 		if (nominated == null || data.visited.containsKey(nominated) || (handled != null && !handled.add(nominated))) {
 			return;

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

@@ -170,6 +170,7 @@ import org.eclipse.cdt.internal.core.dom.parser.cpp.ICPPUnknownMemberClassInstan
 import org.eclipse.cdt.internal.core.dom.parser.cpp.ICPPUnknownType;
 import org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.Conversions.Context;
 import org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.Conversions.UDCMode;
+import org.eclipse.cdt.internal.core.index.IIndexType;
 
 /**
  * Collection of static methods to perform template instantiation, member specialization and
@@ -420,6 +421,17 @@ public class CPPTemplates {
 			ICPPTemplateInstance result = ((ICPPInstanceCache) template).getInstance(args);
 			if (forDefinition && result instanceof IIndexBinding)
 				return null;
+			if (result != null) {
+				// Don't use the cached instance if its argument is an index type and the requested
+				// argument is an AST type. Despite identical signatures the types may be different.  
+				ICPPTemplateArgument[] instanceArgs = result.getTemplateArguments();
+				for (int i = 0; i < args.length; i++) {
+					if (!(args[i].getTypeValue() instanceof IIndexType) &&
+							(instanceArgs[i].getTypeValue() instanceof IIndexType)) {
+						return null; 
+					}
+				}
+			}
 			return result;
 		}
 		return null;

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

@@ -183,7 +183,6 @@ import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTFieldReference;
 import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTFunctionCallExpression;
 import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTIdExpression;
 import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTName;
-import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTTranslationUnit;
 import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTUnaryExpression;
 import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPAliasTemplate;
 import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPArrayType;
@@ -517,7 +516,7 @@ public class CPPVisitor extends ASTQueries {
 	    	binding = CPPSemantics.resolveBinding(elabType.getName());
 	    }
 	    if (binding instanceof IIndexBinding && binding instanceof ICPPClassType) {
-	    	binding= ((CPPASTTranslationUnit) elabType.getTranslationUnit()).mapToAST((ICPPClassType) binding, elabType);
+	    	binding= (ICPPClassType) SemanticUtil.mapToAST((ICPPClassType) binding, elabType);
 	    	ASTInternal.addDeclaration(binding, elabType);
 	    }
 
@@ -1270,9 +1269,7 @@ public class CPPVisitor extends ASTQueries {
 					boolean done= true;
 					IScope scope= null;
 					if (binding instanceof ICPPClassType) {
-						if (binding instanceof IIndexBinding && tu != null) {
+						binding= (ICPPClassType) SemanticUtil.mapToAST((ICPPClassType) binding, name);
-							binding= (((CPPASTTranslationUnit) tu)).mapToAST((ICPPClassType) binding, name);
-						}
 						scope= ((ICPPClassType) binding).getCompositeScope();
 					} else if (binding instanceof ICPPNamespace) {
 						scope= ((ICPPNamespace) binding).getNamespaceScope();
@@ -2556,7 +2553,7 @@ public class CPPVisitor extends ASTQueries {
 					break;
 				IBinding binding = segments[i].resolveBinding();
 				if (binding instanceof IIndexBinding && binding instanceof ICPPClassType) {
-					binding = ((CPPASTTranslationUnit) name.getTranslationUnit()).mapToAST((ICPPClassType) binding, name);
+					binding = (ICPPClassType) SemanticUtil.mapToAST((ICPPClassType) binding, name);
 				}
 				return bindingToOwner(binding);
 			}

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

@@ -44,6 +44,7 @@ import org.eclipse.cdt.core.dom.ast.IFunctionType;
 import org.eclipse.cdt.core.dom.ast.IPointerType;
 import org.eclipse.cdt.core.dom.ast.IProblemBinding;
 import org.eclipse.cdt.core.dom.ast.IQualifierType;
+import org.eclipse.cdt.core.dom.ast.IScope;
 import org.eclipse.cdt.core.dom.ast.ISemanticProblem;
 import org.eclipse.cdt.core.dom.ast.IType;
 import org.eclipse.cdt.core.dom.ast.ITypedef;
@@ -406,7 +407,7 @@ public class SemanticUtil {
 		if (newNestedType == null)
 			return type;
 
-		// Bug 249085 make sure not to add unnecessary qualifications
+		// Do not to add unnecessary qualifications (bug 24908). 
 		if (type instanceof IQualifierType) {
 			IQualifierType qt= (IQualifierType) type;
 			return addQualifiers(newNestedType, qt.isConst(), qt.isVolatile(), false);
@@ -489,38 +490,62 @@ public class SemanticUtil {
 		}
 	}
 
-	public static IType mapToAST(IType type, IASTNode node) {
+	public static IType mapToAST(IType type, IASTNode point) {
-		if (node == null)
+		if (point != null && type instanceof IIndexBinding && type instanceof ICPPClassType) {
+			IASTTranslationUnit ast = point.getTranslationUnit();
+			if (ast instanceof CPPASTTranslationUnit) {
+				return ((CPPASTTranslationUnit) ast).mapToAST((ICPPClassType) type, point);
+			}
+		}
 		return type;
+	}
 
-		if (type instanceof IFunctionType) {
+	public static ICPPTemplateArgument[] mapToAST(ICPPTemplateArgument[] args, IASTNode point) {
-			final ICPPFunctionType ft = (ICPPFunctionType) type;
+		if (point == null)
-			final IType r = ft.getReturnType();
+			return args;
-			final IType ret = mapToAST(r, node);
-			if (ret == r) {
-				return type;
-			}
-			return new CPPFunctionType(ret, ft.getParameterTypes(), ft.isConst(), ft.isVolatile(),
-					ft.hasRefQualifier(), ft.isRValueReference(), ft.takesVarArgs());
-		}
-		if (type instanceof ITypeContainer) {
-			final ITypeContainer tc = (ITypeContainer) type;
-			final IType nestedType= tc.getType();
-			if (nestedType == null)
-				return type;
 
-			IType newType= mapToAST(nestedType, node);
+		// Don't create a new array until it's really needed.
-			if (newType != nestedType) {
+		ICPPTemplateArgument[] result = args;
-				return replaceNestedType(tc, newType);
+		for (int i = 0; i < args.length; i++) {
+			final ICPPTemplateArgument arg = args[i];
+			ICPPTemplateArgument newArg = arg;
+			if (arg != null) {
+				newArg = mapToAST(arg, point);
+				if (result != args) {
+					result[i] = newArg;
+				} else if (arg != newArg) {
+					result = new ICPPTemplateArgument[args.length];
+					if (i > 0) {
+						System.arraycopy(args, 0, result, 0, i);
 					}
-			return type;
+					result[i] = newArg;
-		} else if (type instanceof ICPPClassType && type instanceof IIndexBinding) {
-			IASTTranslationUnit tu = node.getTranslationUnit();
-			if (tu instanceof CPPASTTranslationUnit) {
-				return ((CPPASTTranslationUnit) tu).mapToAST((ICPPClassType) type, node);
 				}
 			}
-		return type;
+		}
+		return result;
+	}
+
+	public static ICPPTemplateArgument mapToAST(ICPPTemplateArgument arg, IASTNode point) {
+		IType type = arg.getTypeValue();
+		if (type != null) {
+			IType mappedType = mapToAST(type, point);
+			IType originalType = arg.getOriginalTypeValue();
+			IType mappedOriginalType = originalType == type ? mappedType : mapToAST(originalType, point);
+			if (mappedType != type || mappedOriginalType != originalType) {
+				return new CPPTemplateTypeArgument(mappedType, mappedOriginalType);
+			}
+		}
+		return arg;
+	}
+
+	public static IScope mapToAST(IScope scope, IASTNode point) {
+		if (point != null) {
+			IASTTranslationUnit ast = point.getTranslationUnit();
+			if (ast instanceof CPPASTTranslationUnit) {
+				return ((CPPASTTranslationUnit) ast).mapToASTScope(scope);
+			}
+		}
+		return scope;
 	}
 
 	public static IType[] getSimplifiedTypes(IType[] types) {
@@ -713,12 +738,15 @@ public class SemanticUtil {
 				clazz= (ICPPClassType) ((ICPPDeferredClassInstance) clazz).getSpecializedBinding();
 			}
 
+			// The base classes may have changed since the definition of clazz was indexed.
+			clazz = (ICPPClassType) mapToAST(clazz, point);
+
 			for (ICPPBase cppBase : ClassTypeHelper.getBases(clazz, point)) {
 				IBinding base= cppBase.getBaseClass();
 				if (base instanceof IType && hashSet.add(base)) {
 					IType tbase= (IType) base;
 					if (tbase.isSameType(baseClass) ||
-							(baseClass instanceof ICPPSpecialization &&  // allow some flexibility with templates
+							(baseClass instanceof ICPPSpecialization && // Allow some flexibility with templates.  
 							((IType) ((ICPPSpecialization) baseClass).getSpecializedBinding()).isSameType(tbase))) {
 						return 1;
 					}

core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/PDOM.java

@@ -973,7 +973,6 @@ public class PDOM extends PlatformObject implements IPDOM {
 
 	@Override
 	public void releaseReadLock() {
-		boolean clearCache= false;
 		synchronized (mutex) {
 			assert lockCount > 0: "No lock to release"; //$NON-NLS-1$
 			if (sDEBUG_LOCKS) {
@@ -984,13 +983,16 @@ public class PDOM extends PlatformObject implements IPDOM {
 			if (lockCount > 0)
 				--lockCount;
 			mutex.notifyAll();
-			clearCache= lockCount == 0;
 			db.setLocked(lockCount != 0);
 		}
-		if (clearCache) {
+		// A lock release probably means that some AST is going away. The result cache has to be
+		// cleared since it may contain objects belonging to the AST that is going away. A failure
+		// to release an AST object would cause a memory leak since the whole AST would remain
+		// pinned to memory.
+		// TODO(sprigogin): It would be more efficient to replace the global result cache with
+		// separate caches for each AST.
 		clearResultCache();
 	}
-	}
 
 	/**
 	 * Acquire a write lock on this PDOM. Blocks until any existing read/write locks are released.