From 61e9d699ba21775a06748d2fbf3183d064411c37 Mon Sep 17 00:00:00 2001 From: Sergey Prigogin Date: Wed, 8 Feb 2012 17:30:00 -0800 Subject: [PATCH] Bug 370887 - CPPVariableReadWriteFlags.getReadWriteFlags returns incorrect results. --- .../core/parser/tests/ast2/AST2BaseTest.java | 18 ++- .../ast2/VariableReadWriteFlagsTest.java | 123 +++++++++++------- .../dom/parser/VariableReadWriteFlags.java | 38 ++++-- .../core/dom/parser/cpp/ClassTypeHelper.java | 36 +++++ .../semantics/CPPVariableReadWriteFlags.java | 25 +++- .../ExtractFunctionRefactoringTest.java | 1 - .../cdt/ui/tests/text/MarkOccurrenceTest.java | 2 +- 7 files changed, 175 insertions(+), 68 deletions(-) diff --git a/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/ast2/AST2BaseTest.java b/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/ast2/AST2BaseTest.java index 51a2daceaa1..8305b215918 100644 --- a/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/ast2/AST2BaseTest.java +++ b/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/ast2/AST2BaseTest.java @@ -597,11 +597,27 @@ public class AST2BaseTest extends BaseTestCase { public IASTName findName(String section, int len) { final int offset = contents.indexOf(section); - assertTrue(offset >= 0); + assertTrue("Section \"" + section + "\" not found", offset >= 0); IASTNodeSelector selector = tu.getNodeSelector(null); return selector.findName(offset, len); } + public IASTName findName(String context, String name) { + if (context == null) { + context = contents; + } + int offset = contents.indexOf(context); + assertTrue("Context \"" + context + "\" not found", offset >= 0); + int nameOffset = context.indexOf(name); + assertTrue("Name \"" + name + "\" not found", nameOffset >= 0); + IASTNodeSelector selector = tu.getNodeSelector(null); + return selector.findName(offset + nameOffset, name.length()); + } + + public IASTName findName(String name) { + return findName(contents, name); + } + public IASTName findImplicitName(String section, int len) { final int offset = contents.indexOf(section); assertTrue(offset >= 0); diff --git a/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/ast2/VariableReadWriteFlagsTest.java b/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/ast2/VariableReadWriteFlagsTest.java index 43b4926929c..c0ef5db339e 100644 --- a/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/ast2/VariableReadWriteFlagsTest.java +++ b/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/ast2/VariableReadWriteFlagsTest.java @@ -32,21 +32,16 @@ public class VariableReadWriteFlagsTest extends AST2BaseTest { super(contents, isCPP); } - void assertReadWriteFlags(String section, int expectedFlags) throws Exception { - int len; - for (len = 0; len < section.length(); len++) { - if (!Character.isJavaIdentifierPart(section.charAt(len))) - break; - } - assertReadWriteFlags(section, len, expectedFlags); - } - - void assertReadWriteFlags(String section, int len, int expectedFlags) throws Exception { - IASTName variable = findName(section, len); + void assertReadWriteFlags(String context, String name, int expectedFlags) throws Exception { + IASTName variable = findName(context, name); assertNotNull(variable); assertEquals(flagsToString(expectedFlags), flagsToString(getReadWriteFlags(variable))); } + void assertReadWriteFlags(String name, int expectedFlags) throws Exception { + assertReadWriteFlags(null, name, expectedFlags); + } + int getReadWriteFlags(IASTName variable) { return isCPP ? CPPVariableReadWriteFlags.getReadWriteFlags(variable) : @@ -90,71 +85,101 @@ public class VariableReadWriteFlagsTest extends AST2BaseTest { return new AssertionHelper(code, true); } - // int test() { - // int a; + // int test(int a) { // a = 2; + // a *= 3; // return a + 1; // } public void testSimpleAccess() throws Exception { AssertionHelper a = getCPPAssertionHelper(); - a.assertReadWriteFlags("a = 2", WRITE); - a.assertReadWriteFlags("a +", READ); + a.assertReadWriteFlags("a = 2", "a", WRITE); + a.assertReadWriteFlags("a *= 3", "a", READ | WRITE); + a.assertReadWriteFlags("a + 1", "a", READ); } - // int a = 1; - public void _testEqualsInitializer() throws Exception { + // class C { + // public: + // C(int); + // }; + // + // class D { + // public: + // D(); + // }; + // + // int a; + // int b = 1; + // C c; + // D d; + // C e(1); + // template void foo(T p) { + // T f; + // } + public void testVariableDeclaration() throws Exception { AssertionHelper a = getCPPAssertionHelper(); - a.assertReadWriteFlags("a", WRITE); + a.assertReadWriteFlags("int a", "a", 0); + a.assertReadWriteFlags("int b = 1", "b", WRITE); + a.assertReadWriteFlags("C c", "c", 0); + a.assertReadWriteFlags("D d", "d", WRITE); + a.assertReadWriteFlags("C e(1)", "e", WRITE); + a.assertReadWriteFlags("T f", "f", WRITE); } // struct A { int x; }; // - // void test() { - // A a; + // void test(A a, A* ap) { // a.x = 1; + // (&a)->x = 1; + // ap->x = 1; // }; public void testFieldAccess() throws Exception { AssertionHelper a = getCPPAssertionHelper(); - a.assertReadWriteFlags("a.", WRITE); + a.assertReadWriteFlags("a.x", "a", WRITE); + a.assertReadWriteFlags("a.x", "x", WRITE); + a.assertReadWriteFlags("(&a)->x", "a", WRITE); + a.assertReadWriteFlags("(&a)->x", "x", WRITE); + a.assertReadWriteFlags("ap->x", "ap", READ); + a.assertReadWriteFlags("ap->x", "x", WRITE); } - // struct A { int x; }; + // void f(int* x, int& y); + // void g(const int* x, const int& y, int z); // - // void test(A* a) { - // a->x = 1; + // void test(int a, int b, int c) { + // f(&a, b); + // g(&a, b, c); // }; - public void testFieldAccessWithDereference() throws Exception { + public void testFunctionCall() throws Exception { AssertionHelper a = getCPPAssertionHelper(); - a.assertReadWriteFlags("a->", READ); - } - - // void f(int* x); - // void g(const int* x); - // - // void test() { - // int a, b; - // f(&a); - // g(&b); - // }; - public void testExplicitArgument() throws Exception { - AssertionHelper a = getCPPAssertionHelper(); - a.assertReadWriteFlags("a)", READ | WRITE); - a.assertReadWriteFlags("b)", READ); + a.assertReadWriteFlags("f(&a, b)", "a", READ | WRITE); + a.assertReadWriteFlags("f(&a, b)", "b", READ | WRITE); + a.assertReadWriteFlags("f(&a, b)", "f", READ); + a.assertReadWriteFlags("g(&a, b, c)", "a", READ); + a.assertReadWriteFlags("g(&a, b, c)", "b", READ); + a.assertReadWriteFlags("g(&a, b, c)", "c", READ); } // struct A { - // void m1(); - // void m2() const; + // void m(); + // void mc() const; // }; // - // void test() { - // A a; - // a.m1(); - // a.m2(); + // void test(A a, A* ap) { + // a.m(); + // a.mc(); + // (&a)->m(); + // (&a)->mc(); + // ap->m(); + // (*ap).m(); // }; - public void testImplicitArgument() throws Exception { + public void testMethodCall() throws Exception { AssertionHelper a = getCPPAssertionHelper(); - a.assertReadWriteFlags("a.m1", READ | WRITE); - a.assertReadWriteFlags("a.m2", READ); + a.assertReadWriteFlags("a.m()", "a", READ | WRITE); + a.assertReadWriteFlags("a.m()", "m", READ); + a.assertReadWriteFlags("a.mc()", "a", READ); + a.assertReadWriteFlags("(&a)->m()", "a", READ | WRITE); + a.assertReadWriteFlags("(&a)->m()", "m", READ); + a.assertReadWriteFlags("ap->m()", "ap", READ); + a.assertReadWriteFlags("(*ap).m()", "ap", READ); } } diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/VariableReadWriteFlags.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/VariableReadWriteFlags.java index 29e5bb8fdb7..881a069faf8 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/VariableReadWriteFlags.java +++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/VariableReadWriteFlags.java @@ -57,15 +57,14 @@ public abstract class VariableReadWriteFlags { protected static final int READ = PDOMName.READ_ACCESS; protected static final int WRITE = PDOMName.WRITE_ACCESS; - protected VariableReadWriteFlags() { - } - protected int rwAnyNode(IASTNode node, int indirection) { final IASTNode parent = node.getParent(); if (parent instanceof IASTExpression) { return rwInExpression((IASTExpression) parent, node, indirection); } else if (parent instanceof IASTStatement) { return rwInStatement((IASTStatement) parent, node, indirection); + } else if (parent instanceof IASTDeclarator) { + return rwInDeclarator((IASTDeclarator) parent, indirection); } else if (parent instanceof IASTEqualsInitializer) { return rwInEqualsInitializer((IASTEqualsInitializer) parent, indirection); } else if (parent instanceof IASTArrayModifier) { @@ -76,6 +75,12 @@ public abstract class VariableReadWriteFlags { return READ | WRITE; // fallback } + protected int rwInDeclarator(IASTDeclarator parent, int indirection) { + if (parent.getInitializer() != null) + return WRITE; + return 0; + } + protected int rwInEqualsInitializer(IASTEqualsInitializer parent, int indirection) { IASTNode grand= parent.getParent(); if (grand instanceof IASTDeclarator) { @@ -112,11 +117,7 @@ public abstract class VariableReadWriteFlags { return rwInBinaryExpression(node, (IASTBinaryExpression) expr, indirection); } if (expr instanceof IASTFieldReference) { - if (node.getPropertyInParent() != IASTFieldReference.FIELD_OWNER || - !((IASTFieldReference) expr).isPointerDereference()) { - return rwAnyNode(expr, indirection); - } - return READ; + return rwInFieldReference(node, (IASTFieldReference) expr, indirection); } if (expr instanceof IASTCastExpression) { // must be ahead of unary return rwAnyNode(expr, indirection); @@ -126,7 +127,7 @@ public abstract class VariableReadWriteFlags { } if (expr instanceof IASTArraySubscriptExpression) { if (indirection > 0 && node.getPropertyInParent() == IASTArraySubscriptExpression.ARRAY) { - return rwAnyNode(expr, indirection-1); + return rwAnyNode(expr, indirection - 1); } return READ; } @@ -146,7 +147,7 @@ public abstract class VariableReadWriteFlags { } if (expr instanceof IASTFunctionCallExpression) { if (node.getPropertyInParent() == IASTFunctionCallExpression.FUNCTION_NAME) { - return rwFunctionName((IASTExpression) node); + return rwInFunctionName((IASTExpression) node); } return rwArgumentForFunctionCall((IASTFunctionCallExpression) expr, node, indirection); } @@ -160,7 +161,20 @@ public abstract class VariableReadWriteFlags { return READ | WRITE; // fall back } - protected int rwFunctionName(IASTExpression node) { + protected int rwInFieldReference(IASTNode node, IASTFieldReference expr, int indirection) { + if (node.getPropertyInParent() == IASTFieldReference.FIELD_NAME) { + if (expr.getPropertyInParent() != IASTFunctionCallExpression.FUNCTION_NAME) + return rwAnyNode(expr, indirection); + } else { // IASTFieldReference.FIELD_OWNER + if (expr.isPointerDereference()) + --indirection; + if (indirection >= 0) + return rwAnyNode(expr, indirection); + } + return READ; + } + + protected int rwInFunctionName(IASTExpression node) { return READ; } @@ -206,7 +220,7 @@ public abstract class VariableReadWriteFlags { while (parent instanceof IASTCompoundStatement) { IASTCompoundStatement compound= (IASTCompoundStatement) parent; IASTStatement[] statements= compound.getStatements(); - if (statements[statements.length-1] != stmt) { + if (statements[statements.length - 1] != stmt) { return 0; } stmt= compound; diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/ClassTypeHelper.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/ClassTypeHelper.java index 9e864e324df..d2b13592b99 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/ClassTypeHelper.java +++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/ClassTypeHelper.java @@ -870,6 +870,42 @@ public class ClassTypeHelper { return null; } + /** + * Returns true if and only if the given class has a trivial default constructor. + * A default constructor is trivial if: + * + * Similar to std::tr1::has_trivial_default_constructor. + * + * @param classTarget the class to check + * @return true if the class has a trivial default constructor + */ + public static boolean hasTrivialDefaultConstructor(ICPPClassType classTarget) { + for (ICPPConstructor ctor : classTarget.getConstructors()) { + if (!ctor.isImplicit() && ctor.getParameters().length == 0) + return false; + } + for (ICPPClassType baseClass : getAllBases(classTarget)) { + if (!classTarget.isSameType(baseClass) && !hasTrivialDefaultConstructor(baseClass)) + return false; + } + for (ICPPField field : classTarget.getDeclaredFields()) { + if (!field.isStatic()) { + IType type = field.getType(); + type = SemanticUtil.getNestedType(type, TDEF | CVTYPE | ARRAY); + if (type instanceof ICPPClassType && !classTarget.isSameType(type) && + !hasTrivialDefaultConstructor((ICPPClassType) type)) { + return false; + } + } + } + return true; + } + /** * Returns true if and only if the given class has a trivial destructor. * A destructor is trivial if: diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/semantics/CPPVariableReadWriteFlags.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/semantics/CPPVariableReadWriteFlags.java index 40a7d9b50d9..cb3e0dad436 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/semantics/CPPVariableReadWriteFlags.java +++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/semantics/CPPVariableReadWriteFlags.java @@ -13,6 +13,7 @@ package org.eclipse.cdt.internal.core.dom.parser.cpp.semantics; import org.eclipse.cdt.core.dom.ast.IASTDeclarator; import org.eclipse.cdt.core.dom.ast.IASTExpression; +import org.eclipse.cdt.core.dom.ast.IASTIdExpression; import org.eclipse.cdt.core.dom.ast.IASTImplicitName; import org.eclipse.cdt.core.dom.ast.IASTImplicitNameOwner; import org.eclipse.cdt.core.dom.ast.IASTName; @@ -25,11 +26,14 @@ import org.eclipse.cdt.core.dom.ast.IType; import org.eclipse.cdt.core.dom.ast.IVariable; import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTConstructorInitializer; import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTUnaryExpression; +import org.eclipse.cdt.core.dom.ast.cpp.ICPPClassType; import org.eclipse.cdt.core.dom.ast.cpp.ICPPConstructor; import org.eclipse.cdt.core.dom.ast.cpp.ICPPFunctionType; import org.eclipse.cdt.core.dom.ast.cpp.ICPPReferenceType; import org.eclipse.cdt.internal.core.dom.parser.ITypeContainer; import org.eclipse.cdt.internal.core.dom.parser.VariableReadWriteFlags; +import org.eclipse.cdt.internal.core.dom.parser.cpp.ClassTypeHelper; +import org.eclipse.cdt.internal.core.dom.parser.cpp.ICPPUnknownType; /** * Helper class to determine whether a variable is accessed for reading and/or writing. @@ -52,6 +56,17 @@ public final class CPPVariableReadWriteFlags extends VariableReadWriteFlags { return super.rwAnyNode(node, indirection); } + @Override + protected int rwInDeclarator(IASTDeclarator parent, int indirection) { + IType type = CPPVisitor.createType(parent); + if (type instanceof ICPPUnknownType || + type instanceof ICPPClassType && + !ClassTypeHelper.hasTrivialDefaultConstructor((ICPPClassType) type)) { + return WRITE; + } + return super.rwInDeclarator(parent, indirection); + } + private int rwInCtorInitializer(IASTNode node, int indirection, ICPPASTConstructorInitializer parent) { IASTNode grand= parent.getParent(); if (grand instanceof IASTDeclarator) { @@ -94,10 +109,12 @@ public final class CPPVariableReadWriteFlags extends VariableReadWriteFlags { } @Override - protected int rwFunctionName(IASTExpression node) { - IType type= node.getExpressionType(); - if (type instanceof ICPPFunctionType && !((ICPPFunctionType) type).isConst()) - return READ | WRITE; + protected int rwInFunctionName(IASTExpression node) { + if (!(node instanceof IASTIdExpression)) { + IType type= node.getExpressionType(); + if (type instanceof ICPPFunctionType && !((ICPPFunctionType) type).isConst()) + return READ | WRITE; + } return READ; } diff --git a/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/extractfunction/ExtractFunctionRefactoringTest.java b/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/extractfunction/ExtractFunctionRefactoringTest.java index 847c1575454..d03f38e6e06 100644 --- a/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/extractfunction/ExtractFunctionRefactoringTest.java +++ b/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/extractfunction/ExtractFunctionRefactoringTest.java @@ -881,7 +881,6 @@ public class ExtractFunctionRefactoringTest extends RefactoringTestBase { // return extracted(b, &a) + a.const_method(); //} public void _testOutputParameterWithMethodCall() throws Exception { - // Currently fails due to http://bugs.eclipse.org/bugs/show_bug.cgi?id=370887 getPreferenceStore().setValue(PreferenceConstants.FUNCTION_PASS_OUTPUT_PARAMETERS_BY_POINTER, true); assertRefactoringSuccess(); } diff --git a/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/text/MarkOccurrenceTest.java b/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/text/MarkOccurrenceTest.java index fd6cd7e0b1d..1e96b57d50b 100644 --- a/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/text/MarkOccurrenceTest.java +++ b/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/text/MarkOccurrenceTest.java @@ -357,7 +357,7 @@ public class MarkOccurrenceTest extends BaseUITestCase { fEditor.selectAndReveal(fMatch.getOffset(), fMatch.getLength()); - assertOccurrences(2, 1); + assertOccurrences(2, 0); assertOccurrencesInWidget(); }