diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/core/parser/util/ArrayUtil.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/core/parser/util/ArrayUtil.java index 4d43ae174ea..8c560e483a1 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/core/parser/util/ArrayUtil.java +++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/core/parser/util/ArrayUtil.java @@ -421,13 +421,30 @@ public abstract class ArrayUtil { } /** - * Note that this should only be used when the placement of nulls within the array - * is unknown (due to performance efficiency). - * + * Moves all null elements to the end of the array. The order of non-null elements is preserved. + * @since 5.4 + */ + public static void compact(Object[] array) { + int j = 0; + for (int i = 0; i < array.length; i++) { + if (array[i] != null) { + if (j != i) { + array[j] = array[i]; + array[i] = null; + } + j++; + } + } + } + + /** * Removes all of the nulls from the array and returns a new array that contains all * of the non-null elements. * * If there are no nulls in the original array then the original array is returned. + + * Note that this method should only be used when the placement of nulls within the array + * is unknown (due to performance efficiency). */ @SuppressWarnings("unchecked") public static T[] removeNulls(Class c, T[] array) { @@ -455,13 +472,13 @@ public abstract class ArrayUtil { } /** - * Note that this should only be used when the placement of nulls within the array - * is unknown (due to performance efficiency). - * * Removes all of the nulls from the array and returns a new array that contains all * of the non-null elements. * * If there are no nulls in the original array then the original array is returned. + * + * Note that this method should only be used when the placement of nulls within the array + * is unknown (due to performance efficiency). * @since 5.2 */ @SuppressWarnings("unchecked") @@ -598,29 +615,9 @@ public abstract class ArrayUtil { } } - static public int[] setInt(int[] array, int idx, int val) { - if (array == null) { - array = new int[DEFAULT_LENGTH > idx + 1 ? DEFAULT_LENGTH : idx + 1]; - array[idx] = val; - return array; - } - - if (array.length <= idx) { - int newLen = array.length * 2; - while (newLen <= idx) - newLen *= 2; - int[] temp = new int[newLen]; - System.arraycopy(array, 0, temp, 0, array.length); - - array = temp; - } - array[idx] = val; - return array; - } - /** - * Stores the specified array contents in a new array of specified - * runtime type. + * Stores the specified array contents in a new array of specified runtime type. + * * @param target the runtime type of the new array * @param source the source array * @return the current array stored in a new array with the specified runtime type, @@ -682,4 +679,24 @@ public abstract class ArrayUtil { } return newArgs; } + + public static int[] setInt(int[] array, int idx, int val) { + if (array == null) { + array = new int[DEFAULT_LENGTH > idx + 1 ? DEFAULT_LENGTH : idx + 1]; + array[idx] = val; + return array; + } + + if (array.length <= idx) { + int newLen = array.length * 2; + while (newLen <= idx) + newLen *= 2; + int[] temp = new int[newLen]; + System.arraycopy(array, 0, temp, 0, array.length); + + array = temp; + } + array[idx] = val; + return array; + } } 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 e7d2716525f..471a174916f 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 @@ -1737,6 +1737,103 @@ public class ExtractFunctionRefactoringTest extends RefactoringTestBase { assertRefactoringSuccess(); } + //test.c + //void test() { + // int i = 0; + // while (i <= 10) { + // /*$*/i++;/*$$*/ + // } + //} + //==================== + //int extracted(int i) { + // i++; + // return i; + //} + // + //void test() { + // int i = 0; + // while (i <= 10) { + // i = extracted(i); + // } + //} + public void testOutputParametersDetectionInWhileLoop() throws Exception { + assertRefactoringSuccess(); + } + + //test.c + //void test() { + // int i = 0; + //loop: + // if (i > 10) return; + // /*$*/i++;/*$$*/ + // goto loop; + //} + //==================== + //int extracted(int i) { + // i++; + // return i; + //} + // + //void test() { + // int i = 0; + //loop: + // if (i > 10) return; + // i = extracted(i); + // goto loop; + //} + public void testOutputParametersDetectionWithGotoLoopSimple() throws Exception { + assertRefactoringSuccess(); + } + + //test.c + //void test() { + // int a = 0, b = 0, c = 0, d = 0; + //loop1: + // if (a > 1) return; + // goto loop1; + //loop2: + // if (b > 2) return; + //loop3: + // if (c > 3) return; + // goto loop2; + //loop4: + // if (d > 4) return; + // goto loop3; + // /*$*/a++; + // b++; + // c++; + // d++;/*$$*/ + // goto loop4; + //} + //==================== + //int extracted(int a, int b, int* c, int* d) { + // a++; + // b++; + // *c++; + // *d++; + // return b; + //} + // + //void test() { + // int a = 0, b = 0, c = 0, d = 0; + //loop1: + // if (a > 1) return; + // goto loop1; + //loop2: + // if (b > 2) return; + //loop3: + // if (c > 3) return; + // goto loop2; + //loop4: + // if (d > 4) return; + // goto loop3; + // b = extracted(a, b, &c, &d); + // goto loop4; + //} + public void testOutputParametersDetectionWithGotoLoopComplex() throws Exception { + assertRefactoringSuccess(); + } + //main.cpp //void method() { // /*$*/for (int var = 0; var < 100; ++var) { diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/corext/refactoring/code/flow/FlowAnalyzer.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/corext/refactoring/code/flow/FlowAnalyzer.java index 4fbf5a0dcb3..abb635e644b 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/corext/refactoring/code/flow/FlowAnalyzer.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/corext/refactoring/code/flow/FlowAnalyzer.java @@ -134,7 +134,7 @@ abstract class FlowAnalyzer extends ASTGenericVisitor { fFlowContext= context; } - protected abstract boolean createReturnFlowInfo(IASTReturnStatement node); + protected abstract boolean shouldCreateReturnFlowInfo(IASTReturnStatement node); protected abstract boolean traverseNode(IASTNode node); @@ -589,7 +589,7 @@ abstract class FlowAnalyzer extends ASTGenericVisitor { } public int leave(IASTGotoStatement node) { - // TODO(sprigogin): Implement goto support + setFlowInfo(node, createBranch(node.getName())); return PROCESS_SKIP; } @@ -602,9 +602,7 @@ abstract class FlowAnalyzer extends ASTGenericVisitor { } public int leave(IASTLabelStatement node) { - FlowInfo info= assignFlowInfo(node, node.getNestedStatement()); - if (info != null) - info.removeLabel(node.getName()); + assignFlowInfo(node, node.getNestedStatement()); return PROCESS_SKIP; } @@ -639,7 +637,7 @@ abstract class FlowAnalyzer extends ASTGenericVisitor { } public int leave(IASTReturnStatement node) { - if (createReturnFlowInfo(node)) { + if (shouldCreateReturnFlowInfo(node)) { ReturnFlowInfo info= createReturn(node); setFlowInfo(node, info); info.merge(getFlowInfo(node.getReturnArgument()), fFlowContext); diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/corext/refactoring/code/flow/InOutFlowAnalyzer.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/corext/refactoring/code/flow/InOutFlowAnalyzer.java index 42a353bf092..b72d6841cdf 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/corext/refactoring/code/flow/InOutFlowAnalyzer.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/corext/refactoring/code/flow/InOutFlowAnalyzer.java @@ -58,7 +58,7 @@ public class InOutFlowAnalyzer extends FlowAnalyzer { } @Override - protected boolean createReturnFlowInfo(IASTReturnStatement node) { + protected boolean shouldCreateReturnFlowInfo(IASTReturnStatement node) { // We are only traversing selected nodes. return true; } diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/corext/refactoring/code/flow/InputFlowAnalyzer.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/corext/refactoring/code/flow/InputFlowAnalyzer.java index 162a85aa819..f2726269bf9 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/corext/refactoring/code/flow/InputFlowAnalyzer.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/corext/refactoring/code/flow/InputFlowAnalyzer.java @@ -19,31 +19,38 @@ package org.eclipse.cdt.internal.corext.refactoring.code.flow; import org.eclipse.core.runtime.Assert; import org.eclipse.jface.text.IRegion; +import org.eclipse.cdt.core.dom.ast.ASTVisitor; import org.eclipse.cdt.core.dom.ast.IASTConditionalExpression; import org.eclipse.cdt.core.dom.ast.IASTDoStatement; import org.eclipse.cdt.core.dom.ast.IASTExpression; import org.eclipse.cdt.core.dom.ast.IASTForStatement; import org.eclipse.cdt.core.dom.ast.IASTFunctionDefinition; +import org.eclipse.cdt.core.dom.ast.IASTGotoStatement; import org.eclipse.cdt.core.dom.ast.IASTIfStatement; +import org.eclipse.cdt.core.dom.ast.IASTLabelStatement; +import org.eclipse.cdt.core.dom.ast.IASTName; import org.eclipse.cdt.core.dom.ast.IASTNode; import org.eclipse.cdt.core.dom.ast.IASTReturnStatement; import org.eclipse.cdt.core.dom.ast.IASTStatement; import org.eclipse.cdt.core.dom.ast.IASTSwitchStatement; import org.eclipse.cdt.core.dom.ast.IASTWhileStatement; +import org.eclipse.cdt.core.dom.ast.IBinding; import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTRangeBasedForStatement; +import org.eclipse.cdt.core.parser.util.ArrayUtil; +import org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.CPPSemantics; import org.eclipse.cdt.internal.corext.util.ASTNodes; public class InputFlowAnalyzer extends FlowAnalyzer { private static class LoopReentranceVisitor extends FlowAnalyzer { - private Selection fSelection; - private IASTNode fLoopNode; + private final Selection selection; + private final IASTNode loopNode; public LoopReentranceVisitor(FlowContext context, Selection selection, IASTNode loopNode) { super(context); - fSelection= selection; - fLoopNode= loopNode; + this.selection= selection; + this.loopNode= loopNode; } @Override @@ -52,13 +59,13 @@ public class InputFlowAnalyzer extends FlowAnalyzer { } @Override - protected boolean createReturnFlowInfo(IASTReturnStatement node) { + protected boolean shouldCreateReturnFlowInfo(IASTReturnStatement node) { // Make sure that the whole return statement is selected or located before the selection. - return ASTNodes.endOffset(node) <= fSelection.getEnd(); + return ASTNodes.endOffset(node) <= selection.getEnd(); } protected IASTNode getLoopNode() { - return fLoopNode; + return loopNode; } public void process(IASTNode node) { @@ -93,7 +100,7 @@ public class InputFlowAnalyzer extends FlowAnalyzer { setFlowInfo(node, forInfo); // If the for statement is the outermost loop then we only have to consider // the action. The parameter and expression are only evaluated once. - if (node == fLoopNode) { + if (node == loopNode) { forInfo.mergeAction(actionInfo, fFlowContext); } else { // Inner for loops are evaluated in the sequence expression, parameter, @@ -118,7 +125,7 @@ public class InputFlowAnalyzer extends FlowAnalyzer { setFlowInfo(node, forInfo); // The for statement is the outermost loop. In this case we only have // to consider the increment, condition and action. - if (node == fLoopNode) { + if (node == loopNode) { forInfo.mergeIncrement(incrementInfo, fFlowContext); forInfo.mergeCondition(conditionInfo, fFlowContext); forInfo.mergeAction(actionInfo, fFlowContext); @@ -141,9 +148,126 @@ public class InputFlowAnalyzer extends FlowAnalyzer { } } + private static class GotoLoopRegion implements IRegion { + final IASTLabelStatement firstLabelStatement; + IASTGotoStatement lastGotoStatement; + + GotoLoopRegion(IASTLabelStatement firstLabelStatement, IASTGotoStatement lastGotoStatement) { + this.firstLabelStatement = firstLabelStatement; + this.lastGotoStatement = lastGotoStatement; + } + + @Override + public int getOffset() { + return ASTNodes.offset(firstLabelStatement); + } + + @Override + public int getLength() { + return ASTNodes.endOffset(lastGotoStatement) - getOffset(); + } + } + + private static class GotoAnalyzer extends ASTVisitor { + private GotoLoopRegion[] gotoRegions = {}; + + public GotoAnalyzer() { + shouldVisitStatements = true; + } + + @Override + public int visit(IASTStatement node) { + if (!(node instanceof IASTLabelStatement)) + return PROCESS_CONTINUE; + + IASTLabelStatement labelStatement = (IASTLabelStatement) node; + IASTName labelName = ((IASTLabelStatement) node).getName(); + IBinding binding = labelName.resolveBinding(); + IASTName[] references = labelStatement.getTranslationUnit().getReferences(binding); + IASTGotoStatement lastGotoStatement = null; + for (IASTName name : references) { + if (name.getPropertyInParent() == IASTGotoStatement.NAME) { + IASTGotoStatement gotoStatement = (IASTGotoStatement) name.getParent(); + IASTStatement lastStatement = lastGotoStatement != null ? lastGotoStatement : labelStatement; + if (isBefore(lastStatement, gotoStatement)) { + lastGotoStatement = gotoStatement; + } + } + } + if (lastGotoStatement == null) + return PROCESS_CONTINUE; + + GotoLoopRegion newRegion = new GotoLoopRegion(labelStatement, lastGotoStatement); + boolean gaps = false; + for (int i = 0; i < gotoRegions.length; i++) { + GotoLoopRegion existing = gotoRegions[i]; + if (existing == null) + break; + if (isBefore(newRegion.firstLabelStatement, existing.lastGotoStatement)) { + if (isBefore(existing.lastGotoStatement, newRegion.lastGotoStatement)) { + existing.lastGotoStatement = newRegion.lastGotoStatement; + for (int j = i + 1; j < gotoRegions.length; j++) { + newRegion = gotoRegions[j]; + if (newRegion == null) + break; + if (isBefore(newRegion.firstLabelStatement, existing.lastGotoStatement)) { + if (isBefore(existing.lastGotoStatement, newRegion.lastGotoStatement)) { + existing.lastGotoStatement = newRegion.lastGotoStatement; + } + } + gotoRegions[j] = null; + gaps = true; + } + } + newRegion = null; + break; + } + } + if (gaps) { + ArrayUtil.compact(gotoRegions); + } else if (newRegion != null) { + gotoRegions = ArrayUtil.append(gotoRegions, newRegion); + } + return PROCESS_SKIP; + } + + public GotoLoopRegion[] getGotoRegions() { + return ArrayUtil.trim(gotoRegions); + } + } + + private static class GotoReentranceVisitor extends FlowAnalyzer { + private final Selection selection; + private final GotoLoopRegion loopRegion; + + public GotoReentranceVisitor(FlowContext context, Selection selection, GotoLoopRegion loopRegion) { + super(context); + this.selection= selection; + this.loopRegion = loopRegion; + } + + @Override + protected boolean traverseNode(IASTNode node) { + return !isBefore(node, loopRegion.firstLabelStatement) && + !isBefore(loopRegion.lastGotoStatement, node); + } + + @Override + protected boolean shouldCreateReturnFlowInfo(IASTReturnStatement node) { + // Make sure that the whole return statement is selected or located before the selection. + return ASTNodes.endOffset(node) <= selection.getEnd(); + } + + public FlowInfo process(IASTFunctionDefinition node) { + node.accept(this); + return getFlowInfo(node); + } + } + private Selection fSelection; private boolean fDoLoopReentrance; private LoopReentranceVisitor fLoopReentranceVisitor; + private GotoReentranceVisitor fGotoReentranceVisitor; public InputFlowAnalyzer(FlowContext context, Selection selection, boolean doLoopReentrance) { super(context); @@ -154,16 +278,34 @@ public class InputFlowAnalyzer extends FlowAnalyzer { public FlowInfo perform(IASTFunctionDefinition node) { node.accept(this); + if (fDoLoopReentrance) { + GotoAnalyzer gotoAnalyzer = new GotoAnalyzer(); + node.accept(gotoAnalyzer); + GotoLoopRegion[] gotoRegions = gotoAnalyzer.getGotoRegions(); + for (GotoLoopRegion loopRegion : gotoRegions) { + if (fSelection.coveredBy(loopRegion)) { + GenericSequentialFlowInfo info= createSequential(); + info.merge(getFlowInfo(node), fFlowContext); + fGotoReentranceVisitor = new GotoReentranceVisitor(fFlowContext, fSelection, loopRegion); + FlowInfo gotoInfo = fGotoReentranceVisitor.process(node); + info.merge(gotoInfo, fFlowContext); + setFlowInfo(node, info); + break; + } + } + } return getFlowInfo(node); } @Override protected boolean traverseNode(IASTNode node) { + if (node instanceof IASTLabelStatement) + return true; return ASTNodes.endOffset(node) > fSelection.getEnd(); } @Override - protected boolean createReturnFlowInfo(IASTReturnStatement node) { + protected boolean shouldCreateReturnFlowInfo(IASTReturnStatement node) { // Make sure that the whole return statement is located after the selection. // There can be cases like return i + [x + 10] * 10; In this case we must not create // a return info node. @@ -210,7 +352,7 @@ public class InputFlowAnalyzer extends FlowAnalyzer { (elsePart != null && fSelection.coveredBy(elsePart))) { GenericSequentialFlowInfo info= createSequential(); setFlowInfo(node, info); - endVisitConditional(info, node.getLogicalConditionExpression(), new IASTNode[] { thenPart, elsePart }); + leaveConditional(info, node.getLogicalConditionExpression(), new IASTNode[] { thenPart, elsePart }); return PROCESS_SKIP; } return super.leave(node); @@ -233,7 +375,7 @@ public class InputFlowAnalyzer extends FlowAnalyzer { (elsePart != null && fSelection.coveredBy(elsePart))) { GenericSequentialFlowInfo info= createSequential(); setFlowInfo(node, info); - endVisitConditional(info, node.getConditionExpression(), new IASTNode[] { thenPart, elsePart }); + leaveConditional(info, node.getConditionExpression(), new IASTNode[] { thenPart, elsePart }); return PROCESS_SKIP; } return super.leave(node); @@ -280,10 +422,9 @@ public class InputFlowAnalyzer extends FlowAnalyzer { return PROCESS_SKIP; } - private void endVisitConditional(GenericSequentialFlowInfo info, IASTNode condition, IASTNode[] branches) { + private void leaveConditional(GenericSequentialFlowInfo info, IASTNode condition, IASTNode[] branches) { info.merge(getFlowInfo(condition), fFlowContext); - for (int i= 0; i < branches.length; i++) { - IASTNode branch= branches[i]; + for (IASTNode branch : branches) { if (branch != null && fSelection.coveredBy(branch)) { info.merge(getFlowInfo(branch), fFlowContext); break; @@ -301,4 +442,8 @@ public class InputFlowAnalyzer extends FlowAnalyzer { info.merge(fLoopReentranceVisitor.getFlowInfo(node), fFlowContext); setFlowInfo(node, info); } + + private static boolean isBefore(IASTNode node1, IASTNode node2) { + return CPPSemantics.declaredBefore(node1, node2, false); + } }