From ebee62c7cacf9de35fd70543fa3dbfa3b6c8e305 Mon Sep 17 00:00:00 2001 From: Sergey Prigogin Date: Mon, 12 Apr 2010 01:44:49 +0000 Subject: [PATCH] Navigation to implicitly called constructors. Bug 248855. --- .../org/eclipse/cdt/core/dom/IName.java | 5 + .../CPPSelectionTestsAnyIndexer.java | 75 +++++++++--- .../selection/CPPSelectionTestsNoIndexer.java | 29 +++-- .../search/actions/OpenDeclarationsJob.java | 107 ++++++++++-------- 4 files changed, 136 insertions(+), 80 deletions(-) diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/core/dom/IName.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/core/dom/IName.java index e7cc11938c1..35369900320 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/core/dom/IName.java +++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/core/dom/IName.java @@ -21,6 +21,11 @@ import org.eclipse.cdt.core.dom.ast.IASTNode; * @noextend This interface is not intended to be extended by clients. */ public interface IName { + /** + * @since 5.2 + */ + IName[] EMPTY_ARRAY= {}; + /** * Returns the name without qualification and without template arguments. * @since 5.1 diff --git a/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/text/selection/CPPSelectionTestsAnyIndexer.java b/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/text/selection/CPPSelectionTestsAnyIndexer.java index a88e5843b0b..e244d6542e5 100644 --- a/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/text/selection/CPPSelectionTestsAnyIndexer.java +++ b/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/text/selection/CPPSelectionTestsAnyIndexer.java @@ -7,6 +7,7 @@ * * Contributors: * Markus Schorn - initial API and implementation + * Sergey Prigogin (Google) *******************************************************************************/ package org.eclipse.cdt.ui.tests.text.selection; @@ -398,14 +399,14 @@ public abstract class CPPSelectionTestsAnyIndexer extends BaseSelectionTestsInde // #include "testCPPSpecDeclsDefs.h" // int a; // defines // int X::y = 1; // defines - // X anX; // defines + // X anX; // defines variable, implicitly calls ctor // extern const int c; // declares // int f(int x) {return x+a;} // defines // struct S; // declares // typedef int Int; // declares // using N::d; // declares // S s; - // Int lhs= s.a+s.b+up+down+anX+0; + // Int lhs= s.a+s.b+up+down+anX+0; public void testCPPSpecDeclsDefs() throws Exception { StringBuffer[] buffers= getContents(2); String hcode= buffers[0].toString(); @@ -474,7 +475,7 @@ public abstract class CPPSelectionTestsAnyIndexer extends BaseSelectionTestsInde offset0= hcode.indexOf("X"); offset1= scode.indexOf("X"); - offset2= scode.indexOf("X", offset1+1); + offset2= scode.indexOf("X", offset1 + 1); decl= testF3(hfile, offset0); assertNode("X", offset0, decl); decl= testF3(file, offset1); @@ -483,7 +484,7 @@ public abstract class CPPSelectionTestsAnyIndexer extends BaseSelectionTestsInde assertNode("X", offset0, decl); offset0= hcode.indexOf("x;"); - offset1= hcode.indexOf("x", offset0+1); + offset1= hcode.indexOf("x", offset0 + 1); decl= testF3(hfile, offset0); assertNode("x", offset0, decl); decl= testF3(hfile, offset1); @@ -515,7 +516,7 @@ public abstract class CPPSelectionTestsAnyIndexer extends BaseSelectionTestsInde assertNode("down", offset0, decl); offset0= hcode.indexOf("N"); - offset1= hcode.indexOf("N;", offset0+1); + offset1= hcode.indexOf("N;", offset0 + 1); offset2= scode.indexOf("N"); decl= testF3(hfile, offset0); assertNode("N", offset0, decl); @@ -537,14 +538,14 @@ public abstract class CPPSelectionTestsAnyIndexer extends BaseSelectionTestsInde assertNode("N1", offset0, decl); offset0= scode.indexOf("anX"); - offset1= scode.indexOf("anX", offset0+1); + offset1= scode.indexOf("anX", offset0 + 1); decl= testF3(file, offset0); - assertNode("anX", offset0, decl); + assertNode("X", hcode.indexOf("X()"), decl); decl= testF3(file, offset1); assertNode("anX", offset0, decl); offset0= scode.indexOf("Int"); - offset1= scode.indexOf("Int", offset0+1); + offset1= scode.indexOf("Int", offset0 + 1); decl= testF3(file, offset0); assertNode("Int", offset0, decl); decl= testF3(file, offset1); @@ -625,7 +626,7 @@ public abstract class CPPSelectionTestsAnyIndexer extends BaseSelectionTestsInde assertNode("x", offset0, decl); } - // struct A { }; // implicitlydeclared A::operator= + // struct A { }; // implicitly declared A::operator= // struct B : A { // B& operator=(const B &); // }; @@ -646,7 +647,7 @@ public abstract class CPPSelectionTestsAnyIndexer extends BaseSelectionTestsInde int offset0, offset1; offset0= scode.indexOf("s)"); - offset1= scode.indexOf("s);", offset0+1); + offset1= scode.indexOf("s);", offset0 + 1); decl = testF3(file, offset0); assertNode("s", offset0, decl); decl = testF3(file, offset1); @@ -1023,7 +1024,7 @@ public abstract class CPPSelectionTestsAnyIndexer extends BaseSelectionTestsInde int offset1, offset2; offset1 = scode.indexOf("cxcpp"); - offset2 = scode.indexOf("cxcpp", offset1+1); + offset2 = scode.indexOf("cxcpp", offset1 + 1); testF3(cppfile, offset1); IEditorPart part = PlatformUI.getWorkbench().getActiveWorkbenchWindow().getActivePage().getActiveEditor(); IEditorInput input = part.getEditorInput(); @@ -1061,7 +1062,7 @@ public abstract class CPPSelectionTestsAnyIndexer extends BaseSelectionTestsInde int offset1, offset2; offset1 = ccode.indexOf("cxcpp"); - offset2 = ccode.indexOf("cxcpp", offset1+1); + offset2 = ccode.indexOf("cxcpp", offset1 + 1); testF3(cfile, offset1); IEditorPart part = PlatformUI.getWorkbench().getActiveWorkbenchWindow().getActivePage().getActiveEditor(); IEditorInput input = part.getEditorInput(); @@ -1094,12 +1095,12 @@ public abstract class CPPSelectionTestsAnyIndexer extends BaseSelectionTestsInde int offset1, offset2; offset1 = code.indexOf("ADD_TEXT"); - offset2 = code.indexOf("ADD_TEXT", offset1+1); + offset2 = code.indexOf("ADD_TEXT", offset1 + 1); decl= testF3(file, offset2); assertNode("ADD_TEXT", offset1, decl); - offset1 = code.indexOf("ADD", offset1+1); - offset2 = code.indexOf("ADD", offset2+1); + offset1 = code.indexOf("ADD", offset1 + 1); + offset2 = code.indexOf("ADD", offset2 + 1); decl= testF3(file, offset2); assertNode("ADD", offset1, decl); } @@ -1131,7 +1132,7 @@ public abstract class CPPSelectionTestsAnyIndexer extends BaseSelectionTestsInde offset1 = code.indexOf("operator []"); offset2 = code.indexOf("[6];"); - decl = testF3(file, offset2+1); + decl = testF3(file, offset2 + 1); assertNode("operator []", offset1, decl); offset2 = code.indexOf("];"); decl = testF3(file, offset2); @@ -1186,4 +1187,46 @@ public abstract class CPPSelectionTestsAnyIndexer extends BaseSelectionTestsInde IASTNode def = testF3(file, offset + 1); assertTrue(def instanceof IASTName); } + + // struct A { + // A(); + // A(int x); + // }; + + // #include "testImplicitConstructorCall_248855.h" + // void func() { + // A a1; + // A a2(5); + // } + // struct B { + // B() : a3(1) {} + // A a3; + // }; + public void testImplicitConstructorCall_248855() throws Exception { + StringBuffer[] buffers= getContents(2); + String hcode= buffers[0].toString(); + String scode= buffers[1].toString(); + IFile hfile = importFile("testImplicitConstructorCall_248855.h", hcode); + IFile file = importFile("testImplicitConstructorCall_248855.cpp", scode); + waitUntilFileIsIndexed(index, file, MAX_WAIT_TIME); + + IASTNode target = testF3(file, scode.indexOf("a1")); + assertTrue(target instanceof IASTName); + assertEquals("A", ((IASTName) target).toString()); + assertEquals(hcode.indexOf("A()"), target.getFileLocation().getNodeOffset()); + assertEquals("A".length(), ((ASTNode) target).getLength()); + + target = testF3(file, scode.indexOf("a2")); + assertTrue(target instanceof IASTName); + assertEquals("A", ((IASTName) target).toString()); + assertEquals(hcode.indexOf("A(int x)"), target.getFileLocation().getNodeOffset()); + assertEquals("A".length(), ((ASTNode) target).getLength()); + + try { + target = testF3(file, scode.indexOf("a3")); + fail("Didn't expect navigation to succeed due to multiple choices: B::a3, A::A(int x)."); + } catch (RuntimeException e) { + assertEquals("ambiguous input: 2", e.getMessage()); + } + } } diff --git a/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/text/selection/CPPSelectionTestsNoIndexer.java b/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/text/selection/CPPSelectionTestsNoIndexer.java index a64a729bcc6..139cc68ea0a 100644 --- a/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/text/selection/CPPSelectionTestsNoIndexer.java +++ b/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/text/selection/CPPSelectionTestsNoIndexer.java @@ -524,7 +524,7 @@ public class CPPSelectionTestsNoIndexer extends BaseUITestCase { enum { up, down }; // defines up and down namespace N { int d; } // defines N and N::d namespace N1 = N; // defines N1 - X anX; // defines anX + X anX; // defines anX, implicitly calls X() // whereas these are just declarations: extern int a; // declares a extern const int c; // declares c @@ -732,9 +732,9 @@ public class CPPSelectionTestsNoIndexer extends BaseUITestCase { offset = code.indexOf("anX; // defines anX"); //$NON-NLS-1$ decl = testF3(file, offset); assertTrue(decl instanceof IASTName); - assertEquals("anX", ((IASTName) decl).toString()); //$NON-NLS-1$ - assertEquals(481, ((ASTNode) decl).getOffset()); - assertEquals(3, ((ASTNode) decl).getLength()); + assertEquals("X", ((IASTName) decl).toString()); //$NON-NLS-1$ + assertEquals(code.indexOf("X()"), ((ASTNode) decl).getOffset()); + assertEquals("X".length(), ((ASTNode) decl).getLength()); offset = code.indexOf("a; // declares a"); //$NON-NLS-1$ def = testF3(file, offset); @@ -1000,7 +1000,7 @@ public class CPPSelectionTestsNoIndexer extends BaseUITestCase { for (int i= 0; i < 2; i++) { IFile file = importFile(filenames[i], code); int od1 = code.indexOf("functionPointer"); - int or1 = code.indexOf("functionPointer", od1+1); + int or1 = code.indexOf("functionPointer", od1 + 1); IASTNode decl = testF3(file, or1); assertTrue(decl instanceof IASTName); @@ -1013,7 +1013,7 @@ public class CPPSelectionTestsNoIndexer extends BaseUITestCase { IDocument doc= ((ITextEditor) editor).getDocumentProvider().getDocument(editor.getEditorInput()); doc.replace(doc.getLength(), 0, appendCode); int od2 = appendCode.indexOf("functionPointerArray"); - int or2 = appendCode.indexOf("functionPointerArray", od2+1); + int or2 = appendCode.indexOf("functionPointerArray", od2 + 1); decl = testF3(file, code.length() + or2); assertTrue(decl instanceof IASTName); @@ -1030,7 +1030,7 @@ public class CPPSelectionTestsNoIndexer extends BaseUITestCase { for (int i= 0; i < 2; i++) { IFile file = importFile(filenames[i], code); int od1 = code.indexOf("EMPTY"); - int or1 = code.indexOf("EMPTY", od1+1); + int or1 = code.indexOf("EMPTY", od1 + 1); IASTNode decl = testF3(file, or1); assertTrue(decl instanceof IASTName); @@ -1104,11 +1104,10 @@ public class CPPSelectionTestsNoIndexer extends BaseUITestCase { int offset= code.indexOf("func();"); try { IASTNode node= testF3(file, offset); + fail("Didn't expect navigation to succeed due to multiple choices."); } catch (RuntimeException e) { assertEquals("ambiguous input: 3", e.getMessage()); - return; } - fail("Expected exception not caught"); } // namespace nm { @@ -1126,13 +1125,13 @@ public class CPPSelectionTestsNoIndexer extends BaseUITestCase { assertContents(code, node.getFileLocation().getNodeOffset(), "func(T a, T b)"); } - // template void func(T a){} - // template void func(T a, T b){} + // template void func(T a){} + // template void func(T a, T b){} // - // template void testFunc() { - // Tmp val; - // func(val, val); // F3 could know that 'func(T a)' cannot be a correct match. - // } + // template void testFunc() { + // Tmp val; + // func(val, val); // F3 could know that 'func(T a)' cannot be a correct match. + // } public void testDependentNameTwoChoices_281736() throws Exception { String code= getContentsForTest(1)[0].toString(); IFile file = importFile("testDependentNameTwoChoices_281736.cpp", code); diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/search/actions/OpenDeclarationsJob.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/search/actions/OpenDeclarationsJob.java index d816931b99b..ae472106002 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/search/actions/OpenDeclarationsJob.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/search/actions/OpenDeclarationsJob.java @@ -7,9 +7,13 @@ * * Contributors: * Markus Schorn - initial API and implementation - *******************************************************************************/ + * Sergey Prigogin (Google) +******************************************************************************/ package org.eclipse.cdt.internal.ui.search.actions; +import static java.lang.Math.max; +import static java.lang.Math.min; + import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -152,24 +156,11 @@ class OpenDeclarationsJob extends Job implements ASTRunnable { final IASTNodeSelector nodeSelector = ast.getNodeSelector(null); IASTName sourceName= nodeSelector.findEnclosingName(selectionStart, selectionLength); + IName[] implicitTargets = findImplicitTargets(ast, nodeSelector, selectionStart, selectionLength); if (sourceName == null) { - IASTName implicit = nodeSelector.findEnclosingImplicitName(selectionStart, selectionLength); - if (implicit != null) { - IASTImplicitNameOwner owner = (IASTImplicitNameOwner) implicit.getParent(); - IASTImplicitName[] implicits = owner.getImplicitNames(); - // There may be more than one name in the same spot. - if (implicits.length > 0) { - List allNames = new ArrayList(); - for (IASTImplicitName name : implicits) { - if (((ASTNode) name).getOffset() == ((ASTNode) implicit).getOffset()) { - IBinding binding = name.resolveBinding(); // Guaranteed to resolve. - IName[] declNames = findDeclNames(ast, NameKind.REFERENCE, binding); - allNames.addAll(Arrays.asList(declNames)); - } - } - if (navigateViaCElements(fTranslationUnit.getCProject(), fIndex, allNames.toArray(new IName[0]))) - return Status.OK_STATUS; - } + if (implicitTargets.length > 0) { + if (navigateViaCElements(fTranslationUnit.getCProject(), fIndex, implicitTargets)) + return Status.OK_STATUS; } } else { boolean found= false; @@ -192,7 +183,7 @@ class OpenDeclarationsJob extends Job implements ASTRunnable { navigateToName(sourceName); return Status.OK_STATUS; } - List nameList= new ArrayList(); + IName[] targets = IName.EMPTY_ARRAY; String filename = ast.getFilePath(); for (IBinding binding : bindings) { if (binding != null && !(binding instanceof IProblemBinding)) { @@ -201,29 +192,29 @@ class OpenDeclarationsJob extends Job implements ASTRunnable { if (name instanceof IIndexName && filename.equals(((IIndexName) name).getFileLocation().getFileName())) { // Exclude index names from the current file. - } else if (isSameName(name, sourceName)) { + } else if (areOverlappingNames(name, sourceName)) { // Exclude the current location. } else if (binding instanceof IParameter) { if (isInSameFunction(sourceName, name)) { - nameList.add(name); + targets = ArrayUtil.append(targets, name); } } else if (binding instanceof ICPPTemplateParameter) { if (isInSameTemplate(sourceName, name)) { - nameList.add(name); + targets = ArrayUtil.append(targets, name); } } else if (name != null) { - nameList.add(name); + targets = ArrayUtil.append(targets, name); } } } } - IName[] declNames = nameList.toArray(new IName[nameList.size()]); - if (navigateViaCElements(fTranslationUnit.getCProject(), fIndex, declNames)) { + targets = ArrayUtil.trim(ArrayUtil.addAll(targets, implicitTargets)); + if (navigateViaCElements(fTranslationUnit.getCProject(), fIndex, targets)) { found= true; } else { // Leave old method as fallback for local variables, parameters and // everything else not covered by ICElementHandle. - found = navigateOneLocation(declNames); + found = navigateOneLocation(targets); } if (!found && !navigationFallBack(ast, sourceName, kind)) { fAction.reportSymbolLookupFailure(new String(sourceName.toCharArray())); @@ -255,25 +246,24 @@ class OpenDeclarationsJob extends Job implements ASTRunnable { private IName[] findDeclNames(IASTTranslationUnit ast, NameKind kind, IBinding binding) throws CoreException { IName[] declNames = findNames(fIndex, ast, kind, binding); - if (declNames.length == 0) { - if (binding instanceof ICPPSpecialization) { - // Bug 207320, handle template instances. - IBinding specialized= ((ICPPSpecialization) binding).getSpecializedBinding(); - if (specialized != null && !(specialized instanceof IProblemBinding)) { - declNames = findNames(fIndex, ast, NameKind.DEFINITION, specialized); - } - } else if (binding instanceof ICPPMethod) { - // Bug 86829, handle implicit methods. - ICPPMethod method= (ICPPMethod) binding; - if (method.isImplicit()) { - try { - IBinding clsBinding= method.getClassOwner(); - if (clsBinding != null && !(clsBinding instanceof IProblemBinding)) { - declNames= findNames(fIndex, ast, NameKind.REFERENCE, clsBinding); - } - } catch (DOMException e) { - // Don't log problem bindings. + // Bug 207320, handle template instances. + while (declNames.length == 0 && binding instanceof ICPPSpecialization) { + binding = ((ICPPSpecialization) binding).getSpecializedBinding(); + if (binding != null && !(binding instanceof IProblemBinding)) { + declNames = findNames(fIndex, ast, NameKind.DEFINITION, binding); + } + } + if (declNames.length == 0 && binding instanceof ICPPMethod) { + // Bug 86829, handle implicit methods. + ICPPMethod method= (ICPPMethod) binding; + if (method.isImplicit()) { + try { + IBinding clsBinding= method.getClassOwner(); + if (clsBinding != null && !(clsBinding instanceof IProblemBinding)) { + declNames= findNames(fIndex, ast, NameKind.REFERENCE, clsBinding); } + } catch (DOMException e) { + // Don't log problem bindings. } } } @@ -329,8 +319,7 @@ class OpenDeclarationsJob extends Job implements ASTRunnable { return index.findNames(binding, IIndex.FIND_DEFINITIONS | IIndex.SEARCH_ACROSS_LANGUAGE_BOUNDARIES); } - private IName[] findDeclarations(IIndex index, IASTTranslationUnit ast, - IBinding binding) throws CoreException { + private IName[] findDeclarations(IIndex index, IASTTranslationUnit ast, IBinding binding) throws CoreException { IName[] declNames= ast.getDeclarationsInAST(binding); for (int i = 0; i < declNames.length; i++) { IName name = declNames[i]; @@ -344,6 +333,26 @@ class OpenDeclarationsJob extends Job implements ASTRunnable { return declNames; } + /** + * Returns definitions of bindings referenced by implicit name at the given location. + */ + private IName[] findImplicitTargets(IASTTranslationUnit ast, IASTNodeSelector nodeSelector, + int offset, int length) throws CoreException { + IName[] definitions = IName.EMPTY_ARRAY; + IASTName firstName = nodeSelector.findEnclosingImplicitName(offset, length); + if (firstName != null) { + IASTImplicitNameOwner owner = (IASTImplicitNameOwner) firstName.getParent(); + for (IASTImplicitName name : owner.getImplicitNames()) { + if (((ASTNode) name).getOffset() == ((ASTNode) firstName).getOffset()) { + IBinding binding = name.resolveBinding(); // Guaranteed to resolve. + IName[] declNames = findDeclNames(ast, NameKind.REFERENCE, binding); + definitions = ArrayUtil.addAll(definitions, declNames); + } + } + } + return ArrayUtil.trim(definitions); + } + private static NameKind getNameKind(IName name) { if (name.isDefinition()) { if (getBinding(name) instanceof ICPPUsingDeclaration) { @@ -370,7 +379,7 @@ class OpenDeclarationsJob extends Job implements ASTRunnable { return null; } - private boolean isSameName(IName n1, IName n2) { + private boolean areOverlappingNames(IName n1, IName n2) { if (n1 == n2) return true; @@ -379,8 +388,8 @@ class OpenDeclarationsJob extends Job implements ASTRunnable { if (loc1 == null || loc2 == null) return false; return loc1.getFileName().equals(loc2.getFileName()) && - loc1.getNodeOffset() == loc2.getNodeOffset() && - loc1.getNodeLength() == loc2.getNodeLength(); + max(loc1.getNodeOffset(), loc2.getNodeOffset()) < + min(loc1.getNodeOffset() + loc1.getNodeLength(), loc2.getNodeOffset() + loc2.getNodeLength()); } private static boolean isInSameFunction(IASTName name1, IName name2) {