From 666369f3f90b990a8c2987e8e93e8563057e42b1 Mon Sep 17 00:00:00 2001 From: Emanuel Graf Date: Thu, 6 Jan 2011 10:22:23 +0000 Subject: [PATCH] Bug 260133: Extract function and extract local variable don't handle type promotion https://bugs.eclipse.org/bugs/show_bug.cgi?id=260133 --- .../refactoring/ExtractExpression.rts | 120 +++------ .../resources/refactoring/ExtractMethod.rts | 8 +- .../extractfunction/ExtractExpression.java | 236 ++++-------------- 3 files changed, 77 insertions(+), 287 deletions(-) diff --git a/core/org.eclipse.cdt.ui.tests/resources/refactoring/ExtractExpression.rts b/core/org.eclipse.cdt.ui.tests/resources/refactoring/ExtractExpression.rts index edc050277bf..b76925e4de8 100644 --- a/core/org.eclipse.cdt.ui.tests/resources/refactoring/ExtractExpression.rts +++ b/core/org.eclipse.cdt.ui.tests/resources/refactoring/ExtractExpression.rts @@ -446,7 +446,7 @@ class Test class Test { void test(); - wchar_t greeting(); + const char greeting(); }; @@ -461,7 +461,7 @@ void Test::test() //= #include "test.h" -wchar_t Test::greeting() +const char Test::greeting() { return "hello"; } @@ -541,7 +541,7 @@ class Test void Test::test() { - float f = /*$*/0.42/*$$*/; + float f = /*$*/0.42f/*$$*/; } //= @@ -549,7 +549,7 @@ void Test::test() float Test::certainty() { - return 0.42; + return 0.42f; } void Test::test() @@ -686,49 +686,6 @@ void Test::test() bool b = invalid(); } -//!Extract string constant -//#org.eclipse.cdt.ui.tests.refactoring.extractfunction.ExtractFunctionRefactoringTest - -//@.config -filename=test.cpp -methodname=greeting - -//@test.h -class Test -{ - void test(); -}; - - -//= -class Test -{ - void test(); - wchar_t greeting(); -}; - - -//@test.cpp -#include "test.h" - -void Test::test() -{ - char* hi = /*$*/"hello"/*$$*/; -} - -//= -#include "test.h" - -wchar_t Test::greeting() -{ - return "hello"; -} - -void Test::test() -{ - char* hi = greeting(); -} - //!Extract int constant //#org.eclipse.cdt.ui.tests.refactoring.extractfunction.ExtractFunctionRefactoringTest @@ -772,49 +729,6 @@ void Test::test() int i = size(); } -//!Extract float constant -//#org.eclipse.cdt.ui.tests.refactoring.extractfunction.ExtractFunctionRefactoringTest - -//@.config -filename=test.cpp -methodname=certainty - -//@test.h -class Test -{ - void test(); -}; - - -//= -class Test -{ - void test(); - float certainty(); -}; - - -//@test.cpp -#include "test.h" - -void Test::test() -{ - float f = /*$*/0.42/*$$*/; -} - -//= -#include "test.h" - -float Test::certainty() -{ - return 0.42; -} - -void Test::test() -{ - float f = certainty(); -} - //!Extract char constant //#org.eclipse.cdt.ui.tests.refactoring.extractfunction.ExtractFunctionRefactoringTest @@ -972,3 +886,29 @@ int test(foo s) { } +//!Bug 260133 Extract function and extract local variable don't handle type promotion +//#org.eclipse.cdt.ui.tests.refactoring.extractfunction.ExtractFunctionRefactoringTest + +//@.config +filename=test.cpp +methodname=bar + +//@test.cpp + +void foo(){ + int x = 3; + double y = /*$*/x + 2.5/*$$*/; +} + +//= + +double bar(int x) +{ + return x + 2.5; +} + +void foo(){ + int x = 3; + double y = bar(x); +} + diff --git a/core/org.eclipse.cdt.ui.tests/resources/refactoring/ExtractMethod.rts b/core/org.eclipse.cdt.ui.tests/resources/refactoring/ExtractMethod.rts index 637f90427e8..f0c4e27b714 100644 --- a/core/org.eclipse.cdt.ui.tests/resources/refactoring/ExtractMethod.rts +++ b/core/org.eclipse.cdt.ui.tests/resources/refactoring/ExtractMethod.rts @@ -2381,7 +2381,7 @@ int main() { //= #include "testString.h" -wchar_t endTag(test::string name) +const char endTag(test::string name) { return ""; } @@ -2437,7 +2437,7 @@ int main() { //= #include "testString.h" -wchar_t exp() +const char exp() { return ">" + ""; } @@ -2549,7 +2549,7 @@ int main() { //= #include "testString.h" -wchar_t exp() +const char exp() { return ">" + " typdefs = getTypdefs(extractedNode); + if (extractedNode instanceof IASTExpression) { + IASTExpression exp = (IASTExpression) extractedNode; + INodeFactory factory = extractedNode.getTranslationUnit().getASTNodeFactory(); + DeclarationGenerator generator = DeclarationGenerator.create(factory); + IType expressionType = exp.getExpressionType(); + for (ITypedef typedef : typdefs) { + if (typedef.getType().isSameType(expressionType)) { + return generator.createDeclSpecFromType(typedef); } } + return generator.createDeclSpecFromType(expressionType); + } else {// Fallback return createSimpleDeclSpecifier(Kind.eVoid); } - if(declSpecifier.isFrozen()) { - return declSpecifier.copy(); - }else { - return declSpecifier; - } } - private IASTDeclSpecifier handleLiteralExpression(IASTLiteralExpression extractedNode) { - switch(extractedNode.getKind()){ - case IASTLiteralExpression.lk_char_constant: - return createSimpleDeclSpecifier(Kind.eChar); - case IASTLiteralExpression.lk_float_constant: - return createSimpleDeclSpecifier(Kind.eFloat); - case IASTLiteralExpression.lk_integer_constant: - return createSimpleDeclSpecifier(Kind.eInt); - case IASTLiteralExpression.lk_string_literal: - return createSimpleDeclSpecifier(Kind.eWChar); - case IASTLiteralExpression.lk_false: - //Like lk_true a boolean type - case IASTLiteralExpression.lk_true: - return createSimpleDeclSpecifier(Kind.eBoolean); - default: - return null; - } - } - - private IASTDeclSpecifier handleNewExpression(ICPPASTNewExpression expression) { - return expression.getTypeId().getDeclSpecifier(); - } - - private IASTDeclSpecifier handleBinaryExpression(ICPPASTBinaryExpression node) { - - switch (node.getOperator()) { - case IASTBinaryExpression.op_equals: - case IASTBinaryExpression.op_notequals: - case IASTBinaryExpression.op_logicalOr: - case IASTBinaryExpression.op_logicalAnd: - case IASTBinaryExpression.op_greaterEqual: - case IASTBinaryExpression.op_greaterThan: - case IASTBinaryExpression.op_lessEqual: - case IASTBinaryExpression.op_lessThan: - - /* We assume that these operations evaluate to bool and don't - * consider overriden operators from custom types for now.*/ - return createSimpleDeclSpecifier(Kind.eBoolean); - - case IASTBinaryExpression.op_plus: - case IASTBinaryExpression.op_plusAssign: - case IASTBinaryExpression.op_minus: - case IASTBinaryExpression.op_minusAssign: - case IASTBinaryExpression.op_multiply: - case IASTBinaryExpression.op_multiplyAssign: - case IASTBinaryExpression.op_divide: - case IASTBinaryExpression.op_divideAssign: - case IASTBinaryExpression.op_assign: - - - return getTypeFromBinaryExp(node); - } - - return null /* not yet handled */; - } - - private IASTDeclSpecifier getTypeFromBinaryExp(ICPPASTBinaryExpression node) { - if(node.getOperand1() instanceof ICPPASTBinaryExpression) { - IASTDeclSpecifier ret = getTypeFromBinaryExp(((CPPASTBinaryExpression)node.getOperand1())); - if(ret != null) return ret; - }else { - if(node.getOperand1() instanceof CPPASTIdExpression) { - return getBinaryExpressionType((CPPASTIdExpression) node.getOperand1()); - } - } - - if(node.getOperand2() instanceof ICPPASTBinaryExpression) { - IASTDeclSpecifier ret = getTypeFromBinaryExp(((CPPASTBinaryExpression)node.getOperand2())); - if(ret != null) return ret; - }else { - if(node.getOperand2() instanceof CPPASTIdExpression) { - return getBinaryExpressionType((CPPASTIdExpression) node.getOperand2()); - } - } - return null; - } - - private IASTDeclSpecifier getBinaryExpressionType(CPPASTIdExpression expression) { - IType expressionType = ((IVariable) expression.getName().resolveBinding()).getType(); - if (expressionType instanceof ITypedef) { - ITypedef typdef = (ITypedef) expressionType; - IType expandedType = expression.getExpressionType(); - if(typdef.getType().isSameType(expandedType)) { - return getDeclSpecifierForType(typdef); - }else{ - return getDeclSpecifierForType(expandedType); + private List getTypdefs(IASTNode extractedNode) { + final ArrayList typeDefs = new ArrayList(); + extractedNode.accept(new ASTVisitor() { + { + shouldVisitExpressions = true; } - } - return getDeclSpecifierForType(expressionType); - - } - - private IASTDeclSpecifier getDeclSpecifierForType(IType expressionType) { - - if (expressionType instanceof CPPBasicType) { - - CPPBasicType basicType = (CPPBasicType) expressionType; - return createSimpleDeclSpecifier(basicType.getKind()); - - } else if (expressionType instanceof ITypedef) { - return getDeclSpecForType((ITypedef) expressionType); - } else if (expressionType instanceof ICPPClassType) { - return getDeclSpecForType((ICPPClassType)expressionType); - } - return null; - } - - private IASTDeclSpecifier getDeclSpecForType(ICPPClassType expressionType) { - IASTName name = null; - - char[][] qualifiedNameCharArray = CPPVisitor.getQualifiedNameCharArray(expressionType); - name = new CPPASTQualifiedName(); - for (char[] cs : qualifiedNameCharArray) { - ((ICPPASTQualifiedName) name).addName(new CPPASTName(cs)); - } - - return new CPPASTNamedTypeSpecifier(name); - } - - private CPPASTNamedTypeSpecifier getDeclSpecForType(ITypedef classType) { - - IASTName name = null; - char[][] qualifiedNameCharArray = CPPVisitor.getQualifiedNameCharArray(classType); - name = new CPPASTQualifiedName(); - for (char[] cs : qualifiedNameCharArray) { - ((ICPPASTQualifiedName) name).addName(new CPPASTName(cs)); - } - - return new CPPASTNamedTypeSpecifier(name); + @Override + public int visit(IASTExpression expression) { + if (expression instanceof IASTIdExpression) { + IASTIdExpression id = (IASTIdExpression) expression; + IBinding binding = id.getName().resolveBinding(); + IType expressionType = null; + if (binding instanceof IVariable) { + expressionType = ((IVariable) binding).getType(); + } + if (binding instanceof IType) { + expressionType = (IType) binding; + } + if (expressionType != null && expressionType instanceof ITypedef) { + ITypedef typdef = (ITypedef) expressionType; + typeDefs.add(typdef); + } + } + return PROCESS_CONTINUE; + } + }); + return typeDefs; } private static IASTDeclSpecifier createSimpleDeclSpecifier(IBasicType.Kind type) { @@ -278,34 +156,6 @@ public class ExtractExpression extends ExtractedFunctionConstructionHelper { } return functionName; } - - private static IASTDeclSpecifier handleFunctionCallExpression(IASTFunctionCallExpression callExpression) { - IASTName functionName = findCalledFunctionName(callExpression); - if(functionName != null) { - IBinding binding = functionName.resolveBinding(); - if (binding instanceof CPPFunction) { - CPPFunction function = (CPPFunction) binding; - if(function.getDefinition() != null) { - IASTNode parent = function.getDefinition().getParent(); - if(parent instanceof CPPASTFunctionDefinition) { - CPPASTFunctionDefinition definition = (CPPASTFunctionDefinition) parent; - return definition.getDeclSpecifier(); - } - } else if(hasDeclaration(function)) { - IASTNode parent = function.getDeclarations()[0].getParent(); - if (parent instanceof CPPASTSimpleDeclaration) { - CPPASTSimpleDeclaration declaration = (CPPASTSimpleDeclaration) parent; - return declaration.getDeclSpecifier(); - } - } - }else if(binding instanceof ITypedef) { - ITypedef typedef = (ITypedef) binding; - return new CPPASTNamedTypeSpecifier(new CPPASTName(typedef.getNameCharArray())); - } - - } - return null; - } @Override protected boolean isReturnTypeAPointer(IASTNode node) {