summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAndrew Eidsness2013-12-05 08:47:07 (EST)
committer Doug Schaefer2013-12-05 13:39:03 (EST)
commit4dab99404c4652336af7c74662304a1a40a6f29b (patch)
tree0dacb03a7639c65ab0680c45d7e78cc7c8200cb0
parent85cbac3dd0d8923e52095362b9ae22dc9423e05f (diff)
downloadorg.eclipse.cdt-4dab99404c4652336af7c74662304a1a40a6f29b.zip
org.eclipse.cdt-4dab99404c4652336af7c74662304a1a40a6f29b.tar.gz
org.eclipse.cdt-4dab99404c4652336af7c74662304a1a40a6f29b.tar.bz2
Bug 422681: Load the correct binding for cross-linkage referencesrefs/changes/77/19377/3
The test case in the previous commit was invalid. The test case did not modify the binding that was referenced by the name and it was checking for the wrong binding after reading the reference from the Database. Further, the test case had a hole where the type of binding that was being used happens to have same node type in both the C and C++ linkages. I've fixed the test case as follows: 1) The test type is now an enum which has a different nodeType in the C and C++ linkages. The test case will now be able to catch cases where the wrong linkage is used when reading the binding. 2) The test case now changes the name to reference a binding in a different linkage. This means the later code (in the test case) is expected to actually load a binding from a different linkage. 3) The test case now checks that the loaded binding really did come from the other linkage. The previous implementation of the test case was using the wrong binding for the checks. I've also updated the implementation so that the updated test case passes. As an aside, during this update we (Doug and I) noticed that PDOMNames unnecessarily store a PDOMLinkage. Bindings come from languages, and therefore need a PDOMLinkage (so that the proper type can be loaded). Names reference bindings and therefore should not have a linkage. The motivating example is that there should be one binding for the printf function, but it could be referenced using names that are created by different parsers. This can be fixed in a later change. The major change to implementation is that PDOMNode now stores an identifier for the factory to use when loading the node. PDOMNode was already using an int to store the nodeType. I've split this to store the factoryId as a short and the nodeType as a short. The highest nodeType currently in use is 58 -- a short should provide ample room for expansion. Since PDOMNode is now able to pick the proper factory for loading, we no longer need the PDOMLinkage.getNode(long) method and I've marked it deprecated. Instead there is a new method PDOMNode.load(PDOM, long). Nodes read their factoryId from the database, so the PDOMLinkage is not needed. Later commits should cleanup the following: a) Remove PDOMLinkage from PDOMNode (and related) b) Change return type of PDOMNode.getNodeType to short c) Replace deprecated calls to PDOMLinkage.getNode Among these changes, (a) should allow removal of the external references list. If names can be loaded without a linkage, then there would be no reason to store the linkage when storing the name. Change-Id: Ife2b21cb21ed1ac6d6c361d0ffb8c7434832c79c Signed-off-by: Andrew Eidsness <eclipse@jfront.com> Reviewed-on: https://git.eclipse.org/r/19377 Tested-by: Hudson CI Reviewed-by: Doug Schaefer <dschaefer@qnx.com> IP-Clean: Doug Schaefer <dschaefer@qnx.com>
-rw-r--r--core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/internal/pdom/tests/PDOMNameTests.java31
-rw-r--r--core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/index/IIndexBindingConstants.java1
-rw-r--r--core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/PDOM.java9
-rw-r--r--core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/dom/PDOMLinkage.java33
-rw-r--r--core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/dom/PDOMName.java4
-rw-r--r--core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/dom/PDOMNode.java79
-rw-r--r--core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/dom/c/PDOMCLinkage.java3
-rw-r--r--core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/dom/cpp/PDOMCPPLinkage.java3
8 files changed, 126 insertions, 37 deletions
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 d7e9c2c..a01b54d 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
@@ -44,8 +44,9 @@ public class PDOMNameTests extends BaseTestCase {
public void testExternalReferences() throws Exception {
IProject project = cproject.getProject();
- TestSourceReader.createFile(project, "file.cpp", "extern void func_cpp() { func_cpp(); }");
- TestSourceReader.createFile(project, "file.c", "extern void func_c() { func_c(); }");
+ // Use enum because this uses a different NodeType in C++ and C.
+ TestSourceReader.createFile(project, "file.cpp", "enum E_cpp { e_cpp }; extern E_cpp func_cpp() { func_cpp(); return e_cpp; }");
+ TestSourceReader.createFile(project, "file.c", "enum E_c { e_c }; extern enum E_c func_c() { func_c(); return e_c; }");
IndexerPreferences.set(project, IndexerPreferences.KEY_INDEXER_ID, IPDOMManager.ID_FAST_INDEXER);
CCorePlugin.getIndexManager().reindex(cproject);
@@ -54,7 +55,7 @@ public class PDOMNameTests extends BaseTestCase {
PDOM pdom = (PDOM) CCoreInternals.getPDOMManager().getPDOM(cproject);
pdom.acquireWriteLock();
try {
- IIndexBinding[] bindings = pdom.findBindings(new char[][]{"func_cpp".toCharArray()}, IndexFilter.ALL, npm());
+ IIndexBinding[] bindings = pdom.findBindings(new char[][]{"E_cpp".toCharArray()}, IndexFilter.ALL, npm());
assertEquals(1, bindings.length);
assertTrue(bindings[0] instanceof PDOMBinding);
@@ -63,7 +64,7 @@ public class PDOMNameTests extends BaseTestCase {
assertNotNull(name_cpp);
assertSame(binding_cpp.getLinkage(), name_cpp.getLinkage());
- bindings = pdom.findBindings(new char[][]{"func_c".toCharArray()}, IndexFilter.ALL, npm());
+ bindings = pdom.findBindings(new char[][]{"E_c".toCharArray()}, IndexFilter.ALL, npm());
assertEquals(1, bindings.length);
assertTrue(bindings[0] instanceof PDOMBinding);
@@ -78,8 +79,15 @@ public class PDOMNameTests extends BaseTestCase {
assertFalse(extNames.hasNext());
// Make sure the C++ binding and the C name are in different linkages, then add the name
- // as an external reference of the binding.
+ // as an external reference of the binding. The case we're setting up is:
+ //
+ // C++_Binding is referenced-by a C_Name which has a C++_Binding
+ //
+ // 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);
// Make sure there is an external reference, then retrieve it. Then make sure there
@@ -91,13 +99,20 @@ public class PDOMNameTests extends BaseTestCase {
assertNotNull(extRef);
assertFalse(extNames.hasNext());
- // 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.
+ // 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());
assertNotSame(binding_cpp.getLinkage(), extRef.getLinkage());
assertSame(binding_c.getLinkage(), extRef.getLinkage());
+
+ // 2) Make sure that the C name was able to properly load the C++ binding.
+ PDOMBinding extBinding = extRef.getBinding();
+ assertNotNull(extBinding);
+ assertEquals(binding_cpp.getRecord(), extBinding.getRecord());
+ assertEquals(binding_cpp.getNodeType(), extBinding.getNodeType());
+ assertTrue(binding_cpp.getClass() == extBinding.getClass());
} finally {
pdom.releaseWriteLock();
}
diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/index/IIndexBindingConstants.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/index/IIndexBindingConstants.java
index 07ea94e..9b62038 100644
--- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/index/IIndexBindingConstants.java
+++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/index/IIndexBindingConstants.java
@@ -14,6 +14,7 @@ package org.eclipse.cdt.internal.core.index;
* Constants used by IIndexFragment implementations for identifying persisted binding types
*/
public interface IIndexBindingConstants {
+ int LINKAGE= 0;
int ENUMERATOR= 3;
int MACRO_DEFINITION = 4;
int MACRO_CONTAINER = 5;
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 e98dd08..09a799d 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
@@ -243,11 +243,12 @@ public class PDOM extends PlatformObject implements IPDOM {
*
* CDT 8.3 development (versions not supported on the 8.2.x branch)
* 160.0 - Store specialized template parameters of class/function template specializations, bug 407497.
- * 161.0 - Allow reference to PDOMBinding from other PDOMLinkages, bug xyz.
+ * 161.0 - Allow reference to PDOMBinding from other PDOMLinkages, bug 422681.
+ * 162.0 - PDOMNode now stores the factoryId for loading, bug 422681.
*/
- private static final int MIN_SUPPORTED_VERSION= version(161, 0);
- private static final int MAX_SUPPORTED_VERSION= version(161, Short.MAX_VALUE);
- private static final int DEFAULT_VERSION = version(161, 0);
+ private static final int MIN_SUPPORTED_VERSION= version(162, 0);
+ private static final int MAX_SUPPORTED_VERSION= version(162, Short.MAX_VALUE);
+ private static final int DEFAULT_VERSION = version(162, 0);
private static int version(int major, int minor) {
return (major << 16) + minor;
diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/dom/PDOMLinkage.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/dom/PDOMLinkage.java
index c5d8997..f89d338 100644
--- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/dom/PDOMLinkage.java
+++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/dom/PDOMLinkage.java
@@ -75,9 +75,6 @@ public abstract class PDOMLinkage extends PDOMNamedNode implements IIndexLinkage
protected static final int RECORD_SIZE = PDOMNamedNode.RECORD_SIZE + 20;
protected static final long[] FILE_LOCAL_REC_DUMMY = new long[]{0};
- // Node types
- protected static final int LINKAGE= 0; // Special one for myself
-
private BTree fMacroIndex= null; // No need for volatile, all fields of BTree are final.
private final PDOM fPDOM;
private final Database fDatabase;
@@ -120,7 +117,7 @@ public abstract class PDOMLinkage extends PDOMNamedNode implements IIndexLinkage
@Override
public int getNodeType() {
- return LINKAGE;
+ return IIndexBindingConstants.LINKAGE;
}
public static IString getLinkageID(PDOM pdom, long record) throws CoreException {
@@ -185,16 +182,12 @@ public abstract class PDOMLinkage extends PDOMNamedNode implements IIndexLinkage
return null;
}
+ /**
+ * @deprecated Use {@link PDOMNode#load(PDOM, long)} instead.
+ */
+ @Deprecated
public final PDOMNode getNode(long record) throws CoreException {
- if (record == 0) {
- return null;
- }
- final int nodeType= PDOMNode.getNodeType(fDatabase, record);
- switch (nodeType) {
- case LINKAGE:
- return null;
- }
- return getNode(record, nodeType);
+ return PDOMNode.load(getPDOM(), record);
}
abstract public PDOMNode getNode(long record, int nodeType) throws CoreException;
@@ -279,9 +272,23 @@ public abstract class PDOMLinkage extends PDOMNamedNode implements IIndexLinkage
return null;
}
+ /**
+ * Return an identifier that uniquely identifies the given binding within this linkage. The
+ * value cannot be used for global comparison because it does not include enough information
+ * to globally identify the binding (across all linkages).
+ */
public abstract int getBindingType(IBinding binding);
/**
+ * Return an identifier that would globally identifies the given binding if it were to be
+ * added to this linkage. This value can be used for comparison with the result of
+ * {@link PDOMNode#getNodeId(Database, long)}.
+ */
+ public int getBindingId(IBinding binding) {
+ return PDOMNode.getNodeId(getLinkageID(), getBindingType(binding));
+ }
+
+ /**
* Call-back informing the linkage that a name has been added. This is
* used to do additional processing, like establishing inheritance relationships.
* @param file the file that has triggered the creation of the name
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 d6da229..8fbb35c 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
@@ -212,9 +212,7 @@ public final class PDOMName implements IIndexFragmentName, IASTFileLocation {
@Override
public char[] getSimpleID() {
try {
- Database db = linkage.getDB();
- long bindingRec = db.getRecPtr(record + BINDING_REC_OFFSET);
- IPDOMBinding binding = linkage.getBinding(bindingRec);
+ PDOMBinding binding = getBinding();
return binding != null ? binding.getNameCharArray() : null;
} catch (CoreException e) {
CCorePlugin.log(e);
diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/dom/PDOMNode.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/dom/PDOMNode.java
index 7f50ad5..5044d2e 100644
--- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/dom/PDOMNode.java
+++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/dom/PDOMNode.java
@@ -13,7 +13,9 @@
package org.eclipse.cdt.internal.core.pdom.dom;
import org.eclipse.cdt.core.CCorePlugin;
+import org.eclipse.cdt.core.dom.ILinkage;
import org.eclipse.cdt.core.dom.IPDOMVisitor;
+import org.eclipse.cdt.internal.core.index.IIndexBindingConstants;
import org.eclipse.cdt.internal.core.pdom.PDOM;
import org.eclipse.cdt.internal.core.pdom.db.Database;
import org.eclipse.core.runtime.CoreException;
@@ -24,7 +26,8 @@ import org.eclipse.core.runtime.CoreException;
* This class managed the parent pointer.
*/
public abstract class PDOMNode implements IInternalPDOMNode {
- private static final int TYPE = 0;
+ private static final int FACTORY_ID = 0;
+ private static final int NODE_TYPE = 2;
private static final int PARENT = 4;
protected static final int RECORD_SIZE = 8;
@@ -33,7 +36,36 @@ public abstract class PDOMNode implements IInternalPDOMNode {
protected final long record;
private volatile long cachedParentRecord;
-
+
+ /**
+ * Load a node from the specified record in the given database. Return null if a node cannot
+ * be loaded.
+ *
+ * @param pdom The PDOM from which to load the node.
+ * @param record The record of the node in the given PDOM.
+ * @return The PDOMNode at the specified location or null if a node cannot be loaded.
+ * @throws CoreException When there is a problem reading the given pdom's Database
+ */
+ public static PDOMNode load(PDOM pdom, long record) throws CoreException {
+ if (record == 0) {
+ return null;
+ }
+
+ Database db = pdom.getDB();
+
+ // Decode the factory id from the serialized type. If it is a valid PDOMLinkage then
+ // use that linkage to build the node. Otherwise fall back to using this linkage.
+ short factoryId = db.getShort(record + FACTORY_ID);
+ short nodeType = db.getShort(record + NODE_TYPE);
+
+ // For an unknown reason linkages cannot be loaded with this method.
+ if (nodeType == IIndexBindingConstants.LINKAGE)
+ return null;
+
+ PDOMLinkage factory = pdom.getLinkage(factoryId);
+ return factory == null ? null : factory.getNode(record, nodeType);
+ }
+
protected PDOMNode(PDOMLinkage linkage, long record) {
fLinkage = linkage;
this.record = record;
@@ -54,8 +86,11 @@ public abstract class PDOMNode implements IInternalPDOMNode {
this.fLinkage = linkage;
record = db.malloc(getRecordSize());
- db.putInt(record + TYPE, getNodeType());
-
+
+ short factoryId = (short) (linkage == null ? ILinkage.NO_LINKAGE_ID : linkage.getLinkageID());
+ db.putShort(record + FACTORY_ID, factoryId);
+ db.putShort(record + NODE_TYPE, (short) getNodeType());
+
cachedParentRecord = parentRec;
db.putRecPtr(record + PARENT, parentRec);
}
@@ -74,6 +109,13 @@ public abstract class PDOMNode implements IInternalPDOMNode {
protected abstract int getRecordSize();
+ /**
+ * Return a value to uniquely identify the node within the factory that is responsible for loading
+ * instances of this node from the PDOM.
+ * <b>
+ * NOTE: For historical reasons the return value is an int. However, implementations must ensure that
+ * the result fits in a short. The upper two bytes will be ignored.
+ */
public abstract int getNodeType();
@Override
@@ -119,11 +161,34 @@ public abstract class PDOMNode implements IInternalPDOMNode {
public void accept(IPDOMVisitor visitor) throws CoreException {
// No children here.
}
-
+
+ /**
+ * Uniquely identifies the type of a node using the factoryId and the nodeType. This
+ * should only be used for comparison with the result of calls to this method on other
+ * nodes.
+ */
+ public static int getNodeId(Database db, long record) throws CoreException {
+ return getNodeId(db.getShort(record + FACTORY_ID), db.getShort(record + NODE_TYPE));
+ }
+
+ /**
+ * Return an value to globally identify the given node within the given linkage. This value
+ * can be used for comparison with other PDOMNodes.
+ */
+ public static int getNodeId(int linkageID, int nodeType) {
+ return (linkageID << 16) | (nodeType & 0xffff);
+ }
+
+ /**
+ * Return a value that identifies the node within a linkage. This value cannot be
+ * used for global comparison because it does not contain enough information to identify
+ * the linkage within which this id is unique.
+ * @see #getNodeId(Database, long)
+ */
public static int getNodeType(Database db, long record) throws CoreException {
- return db.getInt(record + TYPE);
+ return (db.getShort(record + NODE_TYPE));
}
-
+
public long getParentNodeRec() throws CoreException {
if (cachedParentRecord != 0) {
return cachedParentRecord;
diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/dom/c/PDOMCLinkage.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/dom/c/PDOMCLinkage.java
index e298266..e5695a6 100644
--- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/dom/c/PDOMCLinkage.java
+++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/dom/c/PDOMCLinkage.java
@@ -34,6 +34,7 @@ import org.eclipse.cdt.internal.core.dom.parser.c.CBasicType;
import org.eclipse.cdt.internal.core.dom.parser.c.CFunctionType;
import org.eclipse.cdt.internal.core.dom.parser.c.CPointerType;
import org.eclipse.cdt.internal.core.dom.parser.c.CQualifierType;
+import org.eclipse.cdt.internal.core.index.IIndexBindingConstants;
import org.eclipse.cdt.internal.core.index.IIndexCBindingConstants;
import org.eclipse.cdt.internal.core.index.composite.CompositeIndexBinding;
import org.eclipse.cdt.internal.core.pdom.PDOM;
@@ -61,7 +62,7 @@ class PDOMCLinkage extends PDOMLinkage implements IIndexCBindingConstants {
@Override
public int getNodeType() {
- return LINKAGE;
+ return IIndexBindingConstants.LINKAGE;
}
@Override
diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/dom/cpp/PDOMCPPLinkage.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/dom/cpp/PDOMCPPLinkage.java
index 670467a..266ba2d 100644
--- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/dom/cpp/PDOMCPPLinkage.java
+++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/dom/cpp/PDOMCPPLinkage.java
@@ -118,6 +118,7 @@ import org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.EvalTypeId;
import org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.EvalUnary;
import org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.EvalUnaryTypeID;
import org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.TypeOfDependentExpression;
+import org.eclipse.cdt.internal.core.index.IIndexBindingConstants;
import org.eclipse.cdt.internal.core.index.IIndexCPPBindingConstants;
import org.eclipse.cdt.internal.core.index.composite.CompositeIndexBinding;
import org.eclipse.cdt.internal.core.pdom.PDOM;
@@ -177,7 +178,7 @@ class PDOMCPPLinkage extends PDOMLinkage implements IIndexCPPBindingConstants {
@Override
public int getNodeType() {
- return LINKAGE;
+ return IIndexBindingConstants.LINKAGE;
}
// Binding types