mirrors/cdt
1
0
Fork 0
mirror of https://github.com/eclipse-cdt/cdt synced 2025-06-06 01:06:01 +02:00
Code Issues Wiki Activity

Bug 422681: Load the correct binding for cross-linkage references

Browse source 
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>
This commit is contained in:
Andrew Eidsness 2013-12-05 08:47:07 -05:00 • committed by Doug Schaefer
parent 85cbac3dd0
commit 4dab99404c
8 changed files with 127 additions and 38 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

31
core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/internal/pdom/tests/PDOMNameTests.java
View file

@ -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();
}

1
core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/index/IIndexBindingConstants.java
View file

@ -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;

9
core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/PDOM.java
View file

@ -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;

33
core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/dom/PDOMLinkage.java
View file

@ -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,8 +272,22 @@ 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.

4
core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/dom/PDOMName.java
View file

@ -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);

81
core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/dom/PDOMNode.java
View file

@ -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.
}
public static int getNodeType(Database db, long record) throws CoreException {
return db.getInt(record + TYPE);
/**
* 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.getShort(record + NODE_TYPE));
}
public long getParentNodeRec() throws CoreException {
if (cachedParentRecord != 0) {
return cachedParentRecord;

3
core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/dom/c/PDOMCLinkage.java
View file

@ -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

3
core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/dom/cpp/PDOMCPPLinkage.java
View file

@ -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