diff --git a/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/internal/pdom/tests/PDOMNameTests.java b/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/internal/pdom/tests/PDOMNameTests.java index a01b54d87c9..9dcde135f75 100644 --- a/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/internal/pdom/tests/PDOMNameTests.java +++ b/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/internal/pdom/tests/PDOMNameTests.java @@ -69,14 +69,12 @@ public class PDOMNameTests extends BaseTestCase { assertTrue(bindings[0] instanceof PDOMBinding); PDOMBinding binding_c = (PDOMBinding) bindings[0]; - PDOMName name_c = binding_c.getFirstReference(); - assertNotNull(name_c); - assertSame(binding_c.getLinkage(), name_c.getLinkage()); + PDOMName name_c1 = binding_c.getFirstReference(); + assertNotNull(name_c1); + assertSame(binding_c.getLinkage(), name_c1.getLinkage()); // Check that the external references list is currently empty. - IPDOMIterator extNames = binding_cpp.getExternalReferences(); - assertNotNull(extNames); - assertFalse(extNames.hasNext()); + assertEquals(0, countExternalReferences(binding_cpp)); // Make sure the C++ binding and the C name are in different linkages, then add the name // as an external reference of the binding. The case we're setting up is: @@ -86,13 +84,13 @@ public class PDOMNameTests extends BaseTestCase { // We can then test the following (see reference numbers below): // 1) Getting the C name as an external reference of the C++ binding // 2) Loading the C++ binding from the C name - assertNotSame(binding_cpp.getLinkage(), name_c.getLinkage()); - name_c.setBinding(binding_cpp); - binding_cpp.addReference(name_c); + assertNotSame(binding_cpp.getLinkage(), name_c1.getLinkage()); + name_c1.setBinding(binding_cpp); + binding_cpp.addReference(name_c1); // Make sure there is an external reference, then retrieve it. Then make sure there // aren't anymore external references. - extNames = binding_cpp.getExternalReferences(); + IPDOMIterator extNames = binding_cpp.getExternalReferences(); assertNotNull(extNames); assertTrue(extNames.hasNext()); PDOMName extRef = extNames.next(); @@ -102,8 +100,8 @@ public class PDOMNameTests extends BaseTestCase { // 1) Check that the external reference is the same as the C name that was added, that the // external reference does not have the same linkage as the binding, and that it does // have the same linkage as the initial name. - assertSame(name_c.getLinkage(), extRef.getLinkage()); - assertEquals(name_c.getRecord(), extRef.getRecord()); + assertSame(name_c1.getLinkage(), extRef.getLinkage()); + assertEquals(name_c1.getRecord(), extRef.getRecord()); assertNotSame(binding_cpp.getLinkage(), extRef.getLinkage()); assertSame(binding_c.getLinkage(), extRef.getLinkage()); @@ -113,8 +111,65 @@ public class PDOMNameTests extends BaseTestCase { assertEquals(binding_cpp.getRecord(), extBinding.getRecord()); assertEquals(binding_cpp.getNodeType(), extBinding.getNodeType()); assertTrue(binding_cpp.getClass() == extBinding.getClass()); + + // Bug 426238: When names are deleted they need to be removed from the external references + // list. This test puts 3 names into the list and then removes the 2nd, last, + // and then first. + + // Add more external references and then check removals. + PDOMName name_c2 = name_c1.getNextInFile(); + assertNotNull(name_c2); + PDOMName name_c3 = name_c2.getNextInFile(); + name_c2.setBinding(binding_cpp); + binding_cpp.addReference(name_c2); + name_c3.setBinding(binding_cpp); + binding_cpp.addReference(name_c3); + + // Collect the 3 names from the external reference list. + extNames = binding_cpp.getExternalReferences(); + assertNotNull(extNames); + assertTrue(extNames.hasNext()); + PDOMName extRef_1 = extNames.next(); + assertNotNull(extRef); + assertTrue(extNames.hasNext()); + PDOMName extRef_2 = extNames.next(); + assertTrue(extNames.hasNext()); + PDOMName extRef_3 = extNames.next(); + assertFalse(extNames.hasNext()); + + // Check that the list is ordered as expected (reversed during insertion). + assertEquals(name_c3.getRecord(), extRef_1.getRecord()); + assertEquals(name_c2.getRecord(), extRef_2.getRecord()); + assertEquals(name_c1.getRecord(), extRef_3.getRecord()); + + // 3) Check deleting of middle entry. + extRef_2.delete(); + assertEquals(2, countExternalReferences(binding_cpp)); + + // 4) Make sure extref head is updated when there is a next name. + extRef_1.delete(); + assertEquals(1, countExternalReferences(binding_cpp)); + extNames = binding_cpp.getExternalReferences(); + assertNotNull(extNames); + assertTrue(extNames.hasNext()); + assertEquals(extRef_3.getRecord(), extNames.next().getRecord()); + assertFalse(extNames.hasNext()); + + // 5) Check deleting of first entry. + extRef_3.delete(); + assertEquals(0, countExternalReferences(binding_cpp)); + } finally { pdom.releaseWriteLock(); } } + + private static int countExternalReferences(PDOMBinding binding) throws Exception { + IPDOMIterator extRefs = binding.getExternalReferences(); + assertNotNull(extRefs); + int count = 0; + for( ; extRefs.hasNext(); extRefs.next()) + ++count; + return count; + } } 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 4954161c346..eb73d630605 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 @@ -456,7 +456,7 @@ public class Database { if (blocksize < 0) { // Already freed. throw new CoreException(new Status(IStatus.ERROR, CCorePlugin.PLUGIN_ID, 0, - "Already freed", new Exception())); //$NON-NLS-1$ + "Already freed record " + offset, new Exception())); //$NON-NLS-1$ } addBlock(chunk, blocksize, block); freed += blocksize; diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/db/PDOMExternalReferencesList.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/db/PDOMExternalReferencesList.java index c0d08a0a9e3..891fe19f6cd 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/db/PDOMExternalReferencesList.java +++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/db/PDOMExternalReferencesList.java @@ -18,25 +18,42 @@ import org.eclipse.core.runtime.CoreException; /** * A utility class for storing a list of external references. An external reference is - * a PDOMName that references a PDOMBinding in a different linkage. - *

- * External references are stored in a singly linked list with one node for each linkage. - * Each node stores a list of PDOMNames. The PDOMName.BindingList is used to store the - * list. - *

- * Each node has three fields: - * { - * INT_SIZE linkageId; - * PTR_SIZE nextNode; - * PTR_SIZE nameListHead; - * } - *

- * An iterator is provided to examine all names in the list. An iterator is needed (instead - * of a simple list) so that we have a chance to move to the next linkage's node when we get - * to the end of a name list. + * a PDOMName that references a PDOMBinding in a different linkage. This list can be + * examined using {@link #getIterator()}. */ public class PDOMExternalReferencesList { + /* + * This data structure is a list of lists. The inner lists are names that should that + * should be created in the same linkage. The outer list collects the heads of the + * inner lists. + * + * This example shows this storage structure for three names in two linkages. + * + * NameA2 + * ^ + * | + * NameA1 NameB + * ^ ^ + * | | + * record --> LinkageA --> LinkageB + * + * NameA1 and NameA2 should both be created in LinkageA, NameB should be created in LinkageB. + * + * The interface to this class flattens this storage structure so it appears as a simple + * list that can be iterated over. + * + * Continuing with the same example, the iterator behaves as though the list were: + * + * { NameA1, NameA2, NameB } + * + * This class mostly doesn't care about the inner lists. They are implemented using the + * #getNextInBinding attribute of PDOMName. + * + * This class implements the outer list as a singly linked list of "nodes". Each node stores + * the linkageId, the record of the first PDOMName in the list, and the record of the next node. + */ + private final PDOM pdom; private final long record; @@ -71,7 +88,7 @@ public class PDOMExternalReferencesList { // needed. long lastAddr = record; long nodeRec = 0; - while((nodeRec = pdom.getDB().getRecPtr(lastAddr)) != 0) { + while ((nodeRec = pdom.getDB().getRecPtr(lastAddr)) != 0) { // Each node looks like { int linkageID; recPtr nextNode; recPtr headOfList; } int linkageID = pdom.getDB().getInt(nodeRec); if (linkageID == nameLinkageID) @@ -106,6 +123,35 @@ public class PDOMExternalReferencesList { pdom.getDB().putRecPtr(nodeRec + Database.INT_SIZE + Database.PTR_SIZE, name.getRecord()); } + /** + * This function must be used during PDOMName deletion when the list head is being deleted. + */ + public void setFirstReference(PDOMLinkage linkage, PDOMName name) throws CoreException { + // Find the head of this linkage's list. + PDOMName head = null; + Iterator iterator = new Iterator(record); + while (head == null) { + if (iterator.next == null) + break; + + if (!linkage.equals(iterator.next.getLinkage())) { + iterator.next = iterator.advance(); + } else { + head = iterator.next; + } + } + + // If there isn't already a node for this name's linkage then the new name can be inserted + // in the normal way. + if (head == null) { + add(name); + } else { + // Otherwise update the node's head field to point to the given name. + long nextNameRec = iterator.node + Database.INT_SIZE + Database.PTR_SIZE; + pdom.getDB().putRecPtr(nextNameRec, name == null ? 0 : name.getRecord()); + } + } + private class Iterator implements IPDOMIterator { private long nodeAddr; private long node; diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/dom/PDOMBinding.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/dom/PDOMBinding.java index cb5198df1dc..167eb221ede 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/dom/PDOMBinding.java +++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/dom/PDOMBinding.java @@ -184,7 +184,19 @@ public abstract class PDOMBinding extends PDOMNamedNode implements IPDOMBinding return new PDOMExternalReferencesList(getPDOM(), record + FIRST_EXTREF_OFFSET).getIterator(); } - public void setFirstReference(PDOMName name) throws CoreException { + /** + * In most cases the linkage can be found from the linkage of the name. However, when the + * list is being cleared (there is no next), the linkage must be passed in. + */ + public void setFirstReference(PDOMLinkage linkage, PDOMName name) throws CoreException { + if (linkage.equals(getLinkage())) { + setFirstReference(name); + } else { + new PDOMExternalReferencesList(getPDOM(), record + FIRST_EXTREF_OFFSET).setFirstReference(linkage, name); + } + } + + private void setFirstReference(PDOMName name) throws CoreException { // This needs to filter between the local and external lists because it can be used in // contexts that don't know which type of list they are iterating over. E.g., this is // used when deleting names from a PDOMFile. diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/dom/PDOMName.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/dom/PDOMName.java index 8fbb35c83cc..0e63b910929 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/dom/PDOMName.java +++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/dom/PDOMName.java @@ -387,7 +387,7 @@ public final class PDOMName implements IIndexFragmentName, IASTFileLocation { getBinding().setFirstDefinition(nextName); break; case IS_REFERENCE: - getBinding().setFirstReference(nextName); + getBinding().setFirstReference(getLinkage(), nextName); break; } }