From e7c64e785beed108d2126d065176f76a2737119d Mon Sep 17 00:00:00 2001 From: Nathan Ridge Date: Sun, 24 Sep 2017 16:17:38 -0400 Subject: [PATCH] Bug 515417 - Improve handling of friend classes in PDOM According to the standard, if a friend declaration is the only declaration of a class type, then that class type is only visible to argument-dependent lookup until another declaration is encountered. Bug 508338 attempted to implement this rule for PDOM class types by not storing the class type in the index at all when that first declaration is encountered. However, this meant not recording the friend relationship either, which regressed CompletionTests.testTypes_FriendClass (which was then disabled). Bug 512932 implemented this rule for AST class types in a different way, by keeping track of which names are visible to ADL only. This change reverts the fix for bug 508338, and extends the approach from bug 512932 to PDOM class types. It also re- enabled CompletionTests.testTypes_FriendClass which now passes. Change-Id: I9a845fdba514339d86c0c3761a85cf34a17a5613 --- .../core/pdom/dom/BindingCollector.java | 8 +++++++ .../core/pdom/dom/cpp/IPDOMCPPClassType.java | 7 +++++++ .../pdom/dom/cpp/PDOMCPPClassTemplate.java | 5 +++-- ...CPPClassTemplatePartialSpecialization.java | 2 +- .../core/pdom/dom/cpp/PDOMCPPClassType.java | 21 +++++++++++++++++-- .../core/pdom/dom/cpp/PDOMCPPLinkage.java | 13 ++++++------ .../text/contentassist2/CompletionTests.java | 2 +- 7 files changed, 45 insertions(+), 13 deletions(-) diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/dom/BindingCollector.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/dom/BindingCollector.java index 08dabb7ad36..4b4b130acd0 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/dom/BindingCollector.java +++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/dom/BindingCollector.java @@ -19,6 +19,7 @@ import org.eclipse.cdt.core.dom.ast.IBinding; import org.eclipse.cdt.core.dom.ast.IEnumerator; import org.eclipse.cdt.core.dom.ast.cpp.ICPPEnumeration; import org.eclipse.cdt.core.index.IndexFilter; +import org.eclipse.cdt.internal.core.pdom.dom.cpp.IPDOMCPPClassType; import org.eclipse.core.runtime.CoreException; /** @@ -70,6 +71,13 @@ public final class BindingCollector extends NamedNodeCollector { } } } + if (tBinding instanceof IPDOMCPPClassType) { + // Skip bindings that are visible to ADL only. + // TODO(nathanridge): Check if we're doing ADL, and accept it in that case. + if (((IPDOMCPPClassType) tBinding).isVisibleToAdlOnly()) { + return true; + } + } if (filter == null || filter.acceptBinding((IBinding) tBinding)) { return super.addNode(tBinding); } diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/dom/cpp/IPDOMCPPClassType.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/dom/cpp/IPDOMCPPClassType.java index 996e4d62572..9efb5691307 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/dom/cpp/IPDOMCPPClassType.java +++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/dom/cpp/IPDOMCPPClassType.java @@ -40,4 +40,11 @@ public interface IPDOMCPPClassType extends ICPPClassType, IPDOMBinding, IIndexTy * @param visibility The visibility of the member. */ void addMember(PDOMNode member, int visibility) throws CoreException; + + /** + * Returns true if this class type is visible to ADL only. + * A class type is visible to ADL only if it's only declaration so far + * is a friend declaration inside another class. + */ + default boolean isVisibleToAdlOnly() { return false; } } diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/dom/cpp/PDOMCPPClassTemplate.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/dom/cpp/PDOMCPPClassTemplate.java index 2789171a391..56886cbda20 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/dom/cpp/PDOMCPPClassTemplate.java +++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/dom/cpp/PDOMCPPClassTemplate.java @@ -60,9 +60,10 @@ public class PDOMCPPClassTemplate extends PDOMCPPClassType private volatile ICPPTemplateParameter[] params; // Cached template parameters. - public PDOMCPPClassTemplate(PDOMCPPLinkage linkage, PDOMNode parent, ICPPClassTemplate template) + public PDOMCPPClassTemplate(PDOMCPPLinkage linkage, PDOMNode parent, ICPPClassTemplate template, + boolean visibleToAdlOnly) throws CoreException, DOMException { - super(linkage, parent, template); + super(linkage, parent, template, visibleToAdlOnly); final Database db = getDB(); ICPPTemplateParameter[] origParams= template.getTemplateParameters(); diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/dom/cpp/PDOMCPPClassTemplatePartialSpecialization.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/dom/cpp/PDOMCPPClassTemplatePartialSpecialization.java index 4c7d96a7052..27084e9ee4d 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/dom/cpp/PDOMCPPClassTemplatePartialSpecialization.java +++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/dom/cpp/PDOMCPPClassTemplatePartialSpecialization.java @@ -49,7 +49,7 @@ class PDOMCPPClassTemplatePartialSpecialization extends PDOMCPPClassTemplate public PDOMCPPClassTemplatePartialSpecialization(PDOMCPPLinkage linkage, PDOMNode parent, ICPPClassTemplatePartialSpecialization partial, PDOMCPPClassTemplate primary) throws CoreException, DOMException { - super(linkage, parent, partial); + super(linkage, parent, partial, false); getDB().putRecPtr(record + PRIMARY, primary.getRecord()); primary.addPartial(this); diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/dom/cpp/PDOMCPPClassType.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/dom/cpp/PDOMCPPClassType.java index 1cb07ac3874..a8f4cad182d 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/dom/cpp/PDOMCPPClassType.java +++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/dom/cpp/PDOMCPPClassType.java @@ -56,17 +56,20 @@ class PDOMCPPClassType extends PDOMCPPBinding implements IPDOMCPPClassType, IPDO private static final int KEY = FIRSTFRIEND + 4; // byte private static final int ANONYMOUS = KEY + 1; // byte private static final int FINAL = ANONYMOUS + 1; // byte + private static final int VISIBLE_TO_ADL_ONLY = FINAL + 1; // byte @SuppressWarnings("hiding") - protected static final int RECORD_SIZE = FINAL + 1; + protected static final int RECORD_SIZE = VISIBLE_TO_ADL_ONLY + 1; private PDOMCPPClassScope fScope; // No need for volatile, all fields of PDOMCPPClassScope are final. - public PDOMCPPClassType(PDOMLinkage linkage, PDOMNode parent, ICPPClassType classType) throws CoreException { + public PDOMCPPClassType(PDOMLinkage linkage, PDOMNode parent, ICPPClassType classType, + boolean visibleToAdlOnly) throws CoreException { super(linkage, parent, classType.getNameCharArray()); setKind(classType); setAnonymous(classType); setFinal(classType); + setVisibleToAdlOnly(visibleToAdlOnly); // Linked list is initialized by storage being zero'd by malloc. } @@ -106,6 +109,10 @@ class PDOMCPPClassType extends PDOMCPPBinding implements IPDOMCPPClassType, IPDO private void setFinal(ICPPClassType ct) throws CoreException { getDB().putByte(record + FINAL, (byte) (ct.isFinal() ? 1 : 0)); } + + private void setVisibleToAdlOnly(boolean visibleToAdlOnly) throws CoreException { + getDB().putByte(record + VISIBLE_TO_ADL_ONLY, (byte) (visibleToAdlOnly ? 1 : 0)); + } @Override public boolean mayHaveChildren() { @@ -268,6 +275,16 @@ class PDOMCPPClassType extends PDOMCPPBinding implements IPDOMCPPClassType, IPDO return false; } } + + @Override + public boolean isVisibleToAdlOnly() { + try { + return getDB().getByte(record + VISIBLE_TO_ADL_ONLY) != 0; + } catch (CoreException e) { + CCorePlugin.log(e); + return false; + } + } @Override public boolean isSameType(IType type) { 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 a79dd854fcd..6612330bf08 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 @@ -796,13 +796,12 @@ class PDOMCPPLinkage extends PDOMLinkage implements IIndexCPPBindingConstants { // name is not found by name lookup until a matching declaration is provided in the innermost // enclosing nonclass scope. // See http://bugs.eclipse.org/508338 - if (!(binding instanceof ICPPInternalBinding) - || ASTInternal.hasNonFriendDeclaration((ICPPInternalBinding) binding)) { - if (binding instanceof ICPPClassTemplate) { - pdomBinding= new PDOMCPPClassTemplate(this, parent, (ICPPClassTemplate) binding); - } else { - pdomBinding= new PDOMCPPClassType(this, parent, (ICPPClassType) binding); - } + boolean visibleToAdlOnly = (binding instanceof ICPPInternalBinding) + && !ASTInternal.hasNonFriendDeclaration((ICPPInternalBinding) binding); + if (binding instanceof ICPPClassTemplate) { + pdomBinding= new PDOMCPPClassTemplate(this, parent, (ICPPClassTemplate) binding, visibleToAdlOnly); + } else { + pdomBinding= new PDOMCPPClassType(this, parent, (ICPPClassType) binding, visibleToAdlOnly); } } else if (binding instanceof ICPPVariableTemplate) { pdomBinding = new PDOMCPPVariableTemplate(this, parent, (ICPPVariableTemplate) binding); diff --git a/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/text/contentassist2/CompletionTests.java b/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/text/contentassist2/CompletionTests.java index 2ceb03dddd6..ebe55719a8f 100644 --- a/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/text/contentassist2/CompletionTests.java +++ b/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/text/contentassist2/CompletionTests.java @@ -220,7 +220,7 @@ public class CompletionTests extends CompletionTestBase { } //class _friend_class { C3* x; void m() {x->m/*cursor*/ - public void DISABLED_Bug_515417_testTypes_FriendClass() throws Exception { + public void testTypes_FriendClass() throws Exception { final String[] expected= { "m123(void)", "m12(void)", "m13(void)", "m23(void)", "m1protected(void)", "m2protected(void)", "m2private(void)"