Bug 172008 - Improve Open Declaration by using the PDOM and finding definitions first before looking for plain declarations.

Bug 170542 - Moved the pdom locking back to per database instead of global.

6 files changed, 114 insertions, 106 deletions

diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/core/dom/ast/gnu/cpp/GPPLanguage.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/core/dom/ast/gnu/cpp/GPPLanguage.java
index b011274c143..de5fc4204bd 100644
--- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/core/dom/ast/gnu/cpp/GPPLanguage.java
+++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/core/dom/ast/gnu/cpp/GPPLanguage.java
@@ -13,6 +13,7 @@ package org.eclipse.cdt.core.dom.ast.gnu.cpp;
 
 import java.util.ArrayList;
 import java.util.List;
+import java.util.regex.Pattern;
 
 import org.eclipse.cdt.core.CCorePlugin;
 import org.eclipse.cdt.core.dom.CDOM;
@@ -22,6 +23,7 @@ import org.eclipse.cdt.core.dom.ast.IASTName;
 import org.eclipse.cdt.core.dom.ast.IASTNode;
 import org.eclipse.cdt.core.dom.ast.IASTTranslationUnit;
 import org.eclipse.cdt.core.dom.ast.cpp.CPPASTVisitor;
+import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTQualifiedName;
 import org.eclipse.cdt.core.model.ICProject;
 import org.eclipse.cdt.core.model.IContributedModelBuilder;
 import org.eclipse.cdt.core.model.ILanguage;
@@ -215,4 +217,16 @@ public class GPPLanguage extends PlatformObject implements ILanguage {
 		// Use the default CDT model builder
 		return null;
 	}
+	
+	public static Pattern[] createSearchPattern(IASTName name) {
+		if (name instanceof ICPPASTQualifiedName) {
+			IASTName[] names = ((ICPPASTQualifiedName)name).getNames();
+			List patterns = new ArrayList();
+			for (int i = 0; i < names.length; ++i)
+				patterns.add(Pattern.compile(new String(names[i].toCharArray())));
+			return (Pattern[])patterns.toArray(new Pattern[patterns.size()]);
+		} else {
+			return new Pattern[] { Pattern.compile(new String(name.toCharArray())) };
+		}
+	}
 }
diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/PDOM.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/PDOM.java
index 8b2d759b96f..e7051131fb9 100644
--- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/PDOM.java
+++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/PDOM.java
@@ -382,20 +382,20 @@ public class PDOM extends PlatformObject
 	}
 
 	public void acquireReadLock() throws InterruptedException {
-		Database.acquireLock();
+		db.acquireLock();
 	}
 	
 	public void releaseReadLock() {
-		Database.releaseLock();
+		db.releaseLock();
 	}
 	
 	public void acquireWriteLock() throws InterruptedException {
-		Database.acquireLock();
+		db.acquireLock();
 	}
 	
 	public void releaseWriteLock() {
 		Database.saveAll();
-		Database.releaseLock();
+		db.releaseLock();
 		fireChange();
 	}
 	
diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/db/Chunk.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/db/Chunk.java
index 5284f610d77..cbc083e55bd 100644
--- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/db/Chunk.java
+++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/db/Chunk.java
@@ -118,7 +118,7 @@ public class Chunk {
 	
 	void free() throws CoreException {
 		save();
-		db.toc[index] = null;
+		db.freeChunk(index);
 	}
 	
 }
diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/db/Database.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/db/Database.java
index edec622fe8b..b90395c433e 100644
--- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/db/Database.java
+++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/db/Database.java
@@ -26,7 +26,7 @@ public class Database {
 
 	// Access directly by the Chunks
 	final RandomAccessFile file;
-	Chunk[] toc;
+	private Chunk[] toc;
 	
 	private long malloced;
 	private long freed;
@@ -79,7 +79,7 @@ public class Database {
 	 * Empty the contents of the Database, make it ready to start again
 	 * @throws CoreException
 	 */
-	public void clear(int version) throws CoreException {
+	public synchronized void clear(int version) throws CoreException {
 		// Clear out the data area and reset the version
 		Chunk chunk = getChunk(0);
 		chunk.putInt(0, version);
@@ -99,17 +99,30 @@ public class Database {
 	 * @return
 	 */
 	public Chunk getChunk(int offset) throws CoreException {
+//		if (lockCount == 0)
+			// Accessing database without locking
+//			CCorePlugin.log(new Status(IStatus.ERROR, CCorePlugin.PLUGIN_ID, 0, "Index not locked", new Exception())); //$NON-NLS-1$
+		
 		int index = offset / CHUNK_SIZE;
-		Chunk chunk = toc[index];
-		boolean isNew = false;
-		if (chunk == null) {
-			chunk = toc[index] = new Chunk(this, index);
-			isNew = true;
+		Chunk chunk;
+		boolean isNew;
+		synchronized (this) {
+			chunk = toc[index];
+			isNew = false;
+			if (chunk == null) {
+				chunk = toc[index] = new Chunk(this, index);
+				isNew = true;
+			}
 		}
 		Database.lruPutFirst(chunk, isNew);
 		return chunk;
 	}
 
+	// Called by the chunk to set itself free
+	synchronized void freeChunk(int index) {
+		toc[index] = null;
+	}
+	
 	/**
 	 * Allocate a block out of the database.
 	 * 
@@ -140,10 +153,22 @@ public class Database {
 		Chunk chunk;
 		if (freeblock == 0) {
 			// Out of memory, allocate a new chunk
-			int i = createChunk();
-			chunk = toc[i];
-			freeblock = i * CHUNK_SIZE;
-			blocksize = CHUNK_SIZE;
+			synchronized (this) {
+				Chunk[] oldtoc = toc;
+				int n = oldtoc.length;
+				freeblock = n * CHUNK_SIZE;
+				blocksize = CHUNK_SIZE;
+				try {
+					file.seek(freeblock);
+					file.write(new byte[CHUNK_SIZE]);
+				} catch (IOException e) {
+					throw new CoreException(new DBStatus(e));
+				}
+				toc = new Chunk[n + 1];
+				System.arraycopy(oldtoc, 0, toc, 0, n);
+				toc[n] = chunk = new Chunk(this, n);
+			}
+			Database.lruPutFirst(chunk, true);
 		} else {
 			chunk = getChunk(freeblock);
 			removeBlock(chunk, blocksize, freeblock);
@@ -164,23 +189,6 @@ public class Database {
 		return freeblock + 4;
 	}
 	
-	private int createChunk() throws CoreException {
-		try {
-			Chunk[] oldtoc = toc;
-			int n = oldtoc.length;
-			int offset = n * CHUNK_SIZE;
-			file.seek(offset);
-			file.write(new byte[CHUNK_SIZE]);
-			toc = new Chunk[n + 1];
-			System.arraycopy(oldtoc, 0, toc, 0, n);
-			toc[n] = new Chunk(this, n);
-			Database.lruPutFirst(toc[n], true);
-			return n;
-		} catch (IOException e) {
-			throw new CoreException(new DBStatus(e));
-		}
-	}
-	
 	private int getFirstBlock(int blocksize) throws CoreException {
 		return getChunk(0).getInt((blocksize / MIN_SIZE) * INT_SIZE);
 	}
@@ -283,28 +291,6 @@ public class Database {
 			return new ShortString(this, offset);
 	}
 	
-	public int getNumChunks() {
-		return toc.length;
-	}
-
-	public void reportFreeBlocks() throws CoreException {
-		System.out.println("Allocated size: " + toc.length * CHUNK_SIZE);
-		System.out.println("malloc'ed: " + malloced);
-		System.out.println("free'd: " + freed);
-		System.out.println("wasted: " + (toc.length * CHUNK_SIZE - (malloced - freed)));
-		System.out.println("Free blocks");
-		for (int bs = MIN_SIZE; bs <= CHUNK_SIZE; bs += MIN_SIZE) {
-			int count = 0;
-			int block = getFirstBlock(bs);
-			while (block != 0) {
-				++count;
-				block = getInt(block + NEXT_OFFSET);
-			}
-			if (count != 0)
-				System.out.println("Block size: " + bs + "=" + count);
-		}
-	}
-	
 	// Chunk management
 	static private Chunk lruFirst;
 	static private Chunk lruLast;
@@ -329,15 +315,11 @@ public class Database {
 		}
 	}
 	
-	static public final void lruPutFirst(Chunk chunk, boolean isNew) throws CoreException {
+	static private final void lruPutFirst(Chunk chunk, boolean isNew) throws CoreException {
 		// If this chunk is already first, we're good
 		if (chunk == lruFirst)
 			return;
 		
-		if (lockCount == 0)
-			// Accessing database without locking
-			CCorePlugin.log(new Status(IStatus.ERROR, CCorePlugin.PLUGIN_ID, 0, "Index not locked", new Exception())); //$NON-NLS-1$
-
 		if (!isNew) {
 			// Remove from current position in cache
 			if (chunk.lruPrev != null) {
@@ -373,13 +355,11 @@ public class Database {
 		}
 	}
 	
-	// Global locks on the databases
-	// Allow only one access at a time
-	private static Object mutex = new Object();
-	private static Thread lockOwner;
-	private static int lockCount;
+	private Object mutex = new Object();
+	private Thread lockOwner;
+	private int lockCount;
 	
-	public static void acquireLock() throws InterruptedException {
+	public void acquireLock() throws InterruptedException {
 		synchronized (mutex) {
 			if (lockOwner != Thread.currentThread())
 				while (lockCount > 0)
@@ -389,7 +369,7 @@ public class Database {
 		}
 	}
 	
-	public static void releaseLock() {
+	public void releaseLock() {
 		synchronized (mutex) {
 			--lockCount;
 			if (lockCount == 0) {
diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/search/PDOMSearchTextSelectionQuery.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/search/PDOMSearchTextSelectionQuery.java
index ffd03819202..8ed89e5b4b4 100644
--- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/search/PDOMSearchTextSelectionQuery.java
+++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/search/PDOMSearchTextSelectionQuery.java
@@ -16,9 +16,11 @@ import org.eclipse.cdt.core.dom.IPDOM;
 import org.eclipse.cdt.core.dom.ast.IASTName;
 import org.eclipse.cdt.core.dom.ast.IASTTranslationUnit;
 import org.eclipse.cdt.core.dom.ast.IBinding;
+import org.eclipse.cdt.core.dom.ast.gnu.cpp.GPPLanguage;
 import org.eclipse.cdt.core.model.ICElement;
 import org.eclipse.cdt.core.model.ILanguage;
 import org.eclipse.cdt.core.model.ITranslationUnit;
+import org.eclipse.cdt.internal.core.dom.parser.ProblemBinding;
 import org.eclipse.core.runtime.CoreException;
 import org.eclipse.core.runtime.IProgressMonitor;
 import org.eclipse.core.runtime.IStatus;
@@ -52,8 +54,13 @@ public class PDOMSearchTextSelectionQuery extends PDOMSearchQuery {
 				
 				for (int i = 0; i < names.length; ++i) {
 					IBinding binding = names[i].resolveBinding();
-					if (binding != null)
+					if (binding != null && !(binding instanceof ProblemBinding))
 						createMatches(language, binding);
+					else { // try pdom directly
+						IBinding[] bindings = pdom.findBindings(GPPLanguage.createSearchPattern(names[i]), monitor);
+						for (int j = 0; j < bindings.length; ++j)
+							createMatches(language, bindings[j]);
+					}
 				}
 			} catch (InterruptedException e) {
 			} finally {
diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/search/actions/OpenDeclarationsAction.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/search/actions/OpenDeclarationsAction.java
index 34461080628..1205568e754 100644
--- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/search/actions/OpenDeclarationsAction.java
+++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/search/actions/OpenDeclarationsAction.java
@@ -11,13 +11,17 @@
 
 package org.eclipse.cdt.internal.ui.search.actions;
 
+import org.eclipse.cdt.core.CCorePlugin;
+import org.eclipse.cdt.core.dom.IPDOM;
 import org.eclipse.cdt.core.dom.ast.IASTFileLocation;
 import org.eclipse.cdt.core.dom.ast.IASTName;
 import org.eclipse.cdt.core.dom.ast.IASTTranslationUnit;
 import org.eclipse.cdt.core.dom.ast.IBinding;
-import org.eclipse.cdt.core.dom.ast.IProblemBinding;
+import org.eclipse.cdt.core.dom.ast.gnu.cpp.GPPLanguage;
+import org.eclipse.cdt.core.model.ILanguage;
 import org.eclipse.cdt.core.model.IWorkingCopy;
-import org.eclipse.cdt.internal.core.pdom.dom.PDOMBinding;
+import org.eclipse.cdt.internal.core.dom.parser.ProblemBinding;
+import org.eclipse.cdt.internal.core.pdom.PDOM;
 import org.eclipse.cdt.internal.ui.editor.CEditor;
 import org.eclipse.cdt.internal.ui.editor.CEditorMessages;
 import org.eclipse.cdt.ui.CUIPlugin;
@@ -59,25 +63,46 @@ public class OpenDeclarationsAction extends SelectionParseAction {
 				if (workingCopy == null)
 					return Status.CANCEL_STATUS;
 
-				int style = 0;
-//				IPDOM pdom = CCorePlugin.getPDOMManager().getPDOM(workingCopy.getCProject());
-//				if (!pdom.isEmpty())
-//					style |= ILanguage.AST_SKIP_ALL_HEADERS | ILanguage.AST_USE_INDEX;
-				IASTTranslationUnit ast = workingCopy.getLanguage().getASTTranslationUnit(workingCopy, style);
-				IASTName[] selectedNames = workingCopy.getLanguage().getSelectedNames(ast, selectionStart, selectionLength);
-					
-				if (selectedNames.length > 0 && selectedNames[0] != null) { // just right, only one name selected
-					IASTName searchName = selectedNames[0];
-		
-					IBinding binding = searchName.resolveBinding();
-					if (binding != null && !(binding instanceof IProblemBinding)) {
-						final IASTName[] declNames = ast.getDeclarations(binding);
-						if (declNames.length > 0) {
+				IPDOM pdom = CCorePlugin.getPDOMManager().getPDOM(workingCopy.getCProject());
+				pdom.acquireReadLock();
+				try {
+					IASTTranslationUnit ast = workingCopy.getLanguage().getASTTranslationUnit(workingCopy,
+							ILanguage.AST_SKIP_ALL_HEADERS | ILanguage.AST_USE_INDEX);
+					IASTName[] selectedNames = workingCopy.getLanguage().getSelectedNames(ast, selectionStart, selectionLength);
+
+					if (selectedNames.length > 0 && selectedNames[0] != null) { // got a name
+						IASTName searchName = selectedNames[0];
+						IBinding binding = searchName.resolveBinding();
+						IASTName[] declNames = null;
+						if (binding != null && !(binding instanceof ProblemBinding)) {
+							boolean isDefinition = searchName.isDefinition();
+							declNames = isDefinition ? ast.getDeclarations(binding) : ast.getDefinitions(binding);
+							if (declNames.length == 0) // try the other way
+								declNames = isDefinition ? ast.getDefinitions(binding) : ast.getDeclarations(binding);
+						}
+						
+						if (declNames == null || declNames.length == 0) { // try the pdom
+							IBinding[] bindings = pdom.findBindings(GPPLanguage.createSearchPattern(searchName), monitor);
+							for (int i = 0; i < bindings.length; ++i) {
+								declNames = ((PDOM)pdom).getDefinitions(bindings[i]);
+								if (declNames.length > 0)
+									break;
+							}
+							if (declNames == null || declNames.length == 0) // try the decls
+								for (int i = 0; i < bindings.length; ++i) {
+									declNames = ((PDOM)pdom).getDeclarations(bindings[i]);
+									if (declNames.length > 0)
+										break;
+								}
+						}
+
+						if (declNames != null && declNames.length > 0) { // got one
 					    	IASTFileLocation fileloc = declNames[0].getFileLocation();
 					    	if (fileloc != null) {
 								final IPath path = new Path(fileloc.getFileName());
 						    	final int offset = fileloc.getNodeOffset();
 						    	final int length = fileloc.getNodeLength();
+
 								Display.getDefault().asyncExec(new Runnable() {
 									public void run() {
 										try {
@@ -87,34 +112,16 @@ public class OpenDeclarationsAction extends SelectionParseAction {
 										}
 									};
 								});
-					    	}
-						} else if (binding instanceof PDOMBinding) {
-							PDOMBinding pdomBinding = (PDOMBinding)binding;
-							IASTName name = pdomBinding.getFirstDefinition();
-							if (name == null)
-								name = pdomBinding.getFirstDeclaration();
-							if (name != null) {
-						    	IASTFileLocation fileloc = name.getFileLocation();
-						    	if (fileloc != null) {
-									final IPath path = new Path(fileloc.getFileName());
-							    	final int offset = fileloc.getNodeOffset();
-							    	final int length = fileloc.getNodeLength();
-									Display.getDefault().asyncExec(new Runnable() {
-										public void run() {
-											try {
-												open(path, offset, length);
-											} catch (CoreException e) {
-												CUIPlugin.getDefault().log(e);
-											}
-										}
-									});
-						    	}
 							}
 						}
 					}
+				} finally {
+					pdom.releaseReadLock();
 				}
 					
 				return Status.OK_STATUS;
+			} catch (InterruptedException e) {
+				return Status.CANCEL_STATUS;
 			} catch (CoreException e) {
 				return e.getStatus();
 			}