mirrors/cdt
1
0
Fork 0
mirror of https://github.com/eclipse-cdt/cdt synced 2025-07-24 09:25:31 +02:00
Code Issues Wiki Activity

Bug 545704 - Added checker for float comparison

Browse source 
Change-Id: Id5529b9bd5ee38bac5f5b7e8adab741f0bce3f8e
Signed-off-by: Marco Stornelli <marco.stornelli@gmail.com>
This commit is contained in:
Marco Stornelli 2019-03-23 16:24:39 +01:00
parent 2f5f4d50a2
commit fa012eb7d4
8 changed files with 401 additions and 24 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
codan/org.eclipse.cdt.codan.checkers/OSGI-INF/l10n/bundle.properties
View file

@ -193,3 +193,7 @@ problem.name.MultipleDeclarationsProblem = Multiple variable declaration
problem.messagePattern.MultipleDeclarationsProblem = Variables should be declared one per line to improve readability
problem.description.MultipleDeclarationsProblem = This rule will flag multiple declarations on one line
checker.name.FloatCompareChecker = Float compare checker
problem.name.FloatCompareProblem = Direct float comparison
problem.messagePattern.FloatCompareProblem = Float numbers should be compared using an epsilon
problem.description.FloatCompareProblem = This rule will flag direct float or double comparison

15
codan/org.eclipse.cdt.codan.checkers/plugin.xml
View file

@ -507,6 +507,21 @@
messagePattern="%problem.messagePattern.CopyrightProblem"
name="%problem.name.CopyrightProblem">
</problem>
</checker>
<checker
class="org.eclipse.cdt.codan.internal.checkers.FloatCompareChecker"
id="org.eclipse.cdt.codan.internal.checkers.FloatCompareChecker"
name="%checker.name.FloatCompareChecker">
<problem
category="org.eclipse.cdt.codan.core.categories.ProgrammingProblems"
defaultEnabled="false"
defaultSeverity="Warning"
description="%problem.description.FloatCompareProblem"
id="org.eclipse.cdt.codan.internal.checkers.FloatCompareProblem"
markerType="org.eclipse.cdt.codan.core.codanProblem"
messagePattern="%problem.messagePattern.FloatCompareProblem"
name="%problem.name.FloatCompareProblem">
</problem>
</checker>
<checker
class="org.eclipse.cdt.codan.internal.checkers.SwitchCaseChecker"

24
codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/ClassMembersInitializationChecker.java
View file

@ -55,10 +55,10 @@ import org.eclipse.cdt.core.dom.ast.cpp.ICPPMethod;
import org.eclipse.cdt.core.dom.ast.cpp.ICPPReferenceType;
import org.eclipse.cdt.core.dom.ast.cpp.SemanticQueries;
import org.eclipse.cdt.core.index.IIndex;
import org.eclipse.cdt.core.index.IIndexBinding;
import org.eclipse.cdt.internal.core.dom.parser.cpp.ICPPDeferredClassInstance;
import org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.CPPSemantics;
import org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.CPPVariableReadWriteFlags;
import org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.CPPVisitor;
import org.eclipse.cdt.internal.core.pdom.dom.PDOMName;
/**
@ -224,7 +224,7 @@ public class ClassMembersInitializationChecker extends AbstractIndexAstChecker {
private IField getContainedEquivalentBinding(Iterable<IField> fields, IBinding binding, IIndex index) {
for (IField field : fields) {
if (areEquivalentBindings(binding, field, index)) {
if (CPPVisitor.areEquivalentBindings(binding, field, index)) {
return field;
}
}
@ -232,26 +232,6 @@ public class ClassMembersInitializationChecker extends AbstractIndexAstChecker {
return null;
}
private boolean areEquivalentBindings(IBinding binding1, IBinding binding2, IIndex index) {
if (binding1.equals(binding2)) {
return true;
}
if ((binding1 instanceof IIndexBinding) != (binding2 instanceof IIndexBinding) && index != null) {
if (binding1 instanceof IIndexBinding) {
binding2 = index.adaptBinding(binding2);
} else {
binding1 = index.adaptBinding(binding1);
}
if (binding1 == null || binding2 == null) {
return false;
}
if (binding1.equals(binding2)) {
return true;
}
}
return false;
}
/** Checks whether class member of the specified type should be initialized
*
* @param type Type to check

153
codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/FloatCompareChecker.java Normal file
View file

@ -0,0 +1,153 @@
/*******************************************************************************
* Copyright (c) 2019 Marco Stornelli
*
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
* which accompanies this distribution, and is available at
* https://www.eclipse.org/legal/epl-2.0/
*
* SPDX-License-Identifier: EPL-2.0
*******************************************************************************/
package org.eclipse.cdt.codan.internal.checkers;
import org.eclipse.cdt.codan.core.cxx.model.AbstractIndexAstChecker;
import org.eclipse.cdt.core.dom.ast.ASTVisitor;
import org.eclipse.cdt.core.dom.ast.IASTBinaryExpression;
import org.eclipse.cdt.core.dom.ast.IASTExpression;
import org.eclipse.cdt.core.dom.ast.IASTFieldReference;
import org.eclipse.cdt.core.dom.ast.IASTIdExpression;
import org.eclipse.cdt.core.dom.ast.IASTLiteralExpression;
import org.eclipse.cdt.core.dom.ast.IASTTranslationUnit;
import org.eclipse.cdt.core.dom.ast.IBasicType;
import org.eclipse.cdt.core.dom.ast.IBinding;
import org.eclipse.cdt.core.dom.ast.IType;
import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTExpression;
import org.eclipse.cdt.internal.core.dom.parser.ValueFactory;
import org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.CPPVisitor;
import org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.SemanticUtil;
@SuppressWarnings("restriction")
public class FloatCompareChecker extends AbstractIndexAstChecker {
public static final String ERR_ID = "org.eclipse.cdt.codan.internal.checkers.FloatCompareProblem"; //$NON-NLS-1$
@Override
public void processAst(IASTTranslationUnit ast) {
ast.accept(new ASTVisitor() {
{
shouldVisitExpressions = true;
}
private boolean equals(IASTExpression expr1, IASTExpression expr2) {
if (expr1 instanceof ICPPASTExpression && expr2 instanceof ICPPASTExpression) {
return ((ICPPASTExpression) expr1).getEvaluation()
.isEquivalentTo(((ICPPASTExpression) expr2).getEvaluation());
}
if (expr1 instanceof IASTIdExpression && expr2 instanceof IASTIdExpression) {
IBinding leftLeftBinding = ((IASTIdExpression) expr1).getName().resolveBinding();
IBinding rightLeftBinding = ((IASTIdExpression) expr2).getName().resolveBinding();
if (CPPVisitor.areEquivalentBindings(leftLeftBinding, rightLeftBinding,
expr1.getTranslationUnit().getIndex())) {
return true;
}
} else if (expr1 instanceof IASTLiteralExpression && expr2 instanceof IASTLiteralExpression) {
Number n1 = ValueFactory.getConstantNumericalValue(expr1);
Number n2 = ValueFactory.getConstantNumericalValue(expr2);
if (n1 != null && n1.equals(n2))
return true;
} else if (expr1 instanceof IASTFieldReference && expr2 instanceof IASTFieldReference) {
IBinding leftLeftBinding = ((IASTFieldReference) expr1).getFieldName().resolveBinding();
IBinding rightLeftBinding = ((IASTFieldReference) expr2).getFieldName().resolveBinding();
if (CPPVisitor.areEquivalentBindings(leftLeftBinding, rightLeftBinding,
expr1.getTranslationUnit().getIndex())) {
return true;
}
}
return false;
}
private boolean processDirect(IASTBinaryExpression expression) {
IASTBinaryExpression binary = expression;
if ((binary.getOperator() == IASTBinaryExpression.op_notequals
|| binary.getOperator() == IASTBinaryExpression.op_equals)
&& (isFloat(binary.getOperand1().getExpressionType())
|| isFloat(binary.getOperand2().getExpressionType()))) {
reportProblem(ERR_ID, expression);
return true;
}
return false;
}
private boolean processIndirect(IASTBinaryExpression binary) {
if (binary.getOperator() == IASTBinaryExpression.op_logicalAnd) {
return processIndirect(binary, false);
} else if (binary.getOperator() == IASTBinaryExpression.op_logicalOr) {
return processIndirect(binary, true);
}
return false;
}
private boolean processIndirect(IASTBinaryExpression expression, boolean invert) {
int cond1Test = IASTBinaryExpression.op_lessEqual;
int cond2Test = IASTBinaryExpression.op_greaterEqual;
if (invert) {
cond1Test = IASTBinaryExpression.op_lessThan;
cond2Test = IASTBinaryExpression.op_greaterThan;
}
IASTBinaryExpression binary = expression;
IASTExpression left = binary.getOperand1();
IASTExpression right = binary.getOperand2();
if (left instanceof IASTBinaryExpression && right instanceof IASTBinaryExpression) {
int leftOp = ((IASTBinaryExpression) left).getOperator();
int rightOp = ((IASTBinaryExpression) right).getOperator();
if ((leftOp == cond1Test && rightOp == cond2Test)
|| (rightOp == cond1Test && leftOp == cond2Test)) {
//Case 1:
// a <= b && c >= d
// Ex. f <= 3.14 && f >= 3.14
//Case 2:
// a >= b && c <= d
// Ex. f >= 3.14 && f <= 3.14
IASTExpression leftLeft = ((IASTBinaryExpression) left).getOperand1();
IASTExpression leftRight = ((IASTBinaryExpression) left).getOperand2();
IASTExpression rightLeft = ((IASTBinaryExpression) right).getOperand1();
IASTExpression rightRight = ((IASTBinaryExpression) right).getOperand2();
if (equals(leftLeft, rightLeft) && equals(leftRight, rightRight)
&& (isFloat(leftLeft.getExpressionType()) || isFloat(leftRight.getExpressionType()))) {
reportProblem(ERR_ID, expression);
return true;
}
}
}
return false;
}
@Override
public int visit(IASTExpression expression) {
if (expression instanceof IASTBinaryExpression) {
boolean res = processDirect((IASTBinaryExpression) expression);
if (!res)
res = processIndirect((IASTBinaryExpression) expression);
}
return PROCESS_CONTINUE;
}
});
}
private boolean isFloat(IType type) {
type = SemanticUtil.getNestedType(type, SemanticUtil.REF | SemanticUtil.TDEF | SemanticUtil.ALLCVQ);
if (!(type instanceof IBasicType)) {
return false;
}
IBasicType.Kind k = ((IBasicType) type).getKind();
switch (k) {
case eFloat:
case eDouble:
case eFloat128:
return true;
default:
return false;
}
}
}

221
codan/org.eclipse.cdt.codan.core.tests/src/org/eclipse/cdt/codan/core/internal/checkers/FloatCompareCheckerTest.java Normal file
View file

@ -0,0 +1,221 @@
/*******************************************************************************
* Copyright (c) 2019 Marco Stornelli
*
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
* which accompanies this distribution, and is available at
* https://www.eclipse.org/legal/epl-2.0/
*
* SPDX-License-Identifier: EPL-2.0
*******************************************************************************/
package org.eclipse.cdt.codan.core.internal.checkers;
import org.eclipse.cdt.codan.core.tests.CheckerTestCase;
import org.eclipse.cdt.codan.internal.checkers.FloatCompareChecker;
/**
* Test for {@link FloatCompareChecker} class
*/
public class FloatCompareCheckerTest extends CheckerTestCase {
public static final String ERR_ID = FloatCompareChecker.ERR_ID;
@Override
public void setUp() throws Exception {
super.setUp();
enableProblems(ERR_ID);
}
@Override
public boolean isCpp() {
return true;
}
@Override
public boolean isHeader() {
return true;
}
//int main() {
// float a;
// float b;
// if (a == b)
// return 1;
// return 0;
//}
public void testEqualWithFloat() throws Exception {
loadCodeAndRun(getAboveComment());
checkErrorLine(4, ERR_ID);
}
//int main() {
// float a;
// float b;
// if (a != b)
// return 1;
// return 0;
//}
public void testDifferentWithFloat() throws Exception {
loadCodeAndRun(getAboveComment());
checkErrorLine(4, ERR_ID);
}
//int main() {
// double a;
// double b;
// if (a == b)
// return 1;
// return 0;
//}
public void testWithDouble() throws Exception {
loadCodeAndRun(getAboveComment());
checkErrorLine(4, ERR_ID);
}
//int main() {
// double a;
// float b;
// if (a == b)
// return 1;
// return 0;
//}
public void testWithMixedTypes() throws Exception {
loadCodeAndRun(getAboveComment());
checkErrorLine(4, ERR_ID);
}
//typedef float myType;
//int main() {
// myType a;
// myType b;
// if (a == b)
// return 1;
// return 0;
//}
public void testWithTypedef() throws Exception {
loadCodeAndRun(getAboveComment());
checkErrorLine(5, ERR_ID);
}
//int main() {
// float a1;
// float b1;
// const float& a = a1;
// const float& b = b1;
// if (a == b)
// return 1;
// return 0;
//}
public void testWithQualififedTypes() throws Exception {
loadCodeAndRun(getAboveComment());
checkErrorLine(6, ERR_ID);
}
//int main() {
// float a1;
// float b1;
// float* a = &a1;
// float* b = &b1;
// if (*a == *b)
// return 1;
// return 0;
//}
public void testWithDerefPointers() throws Exception {
loadCodeAndRun(getAboveComment());
checkErrorLine(6, ERR_ID);
}
//int main() {
// float a1;
// float b1;
// float* a = &a1;
// float* b = &b1;
// if (a == b)
// return 1;
// return 0;
//}
public void testWithPointers() throws Exception {
loadCodeAndRun(getAboveComment());
checkNoErrorsOfKind(ERR_ID);
}
//int foo() {
// float a1;
// float b1;
// return a1 == b1 ? 0 : 1;
//}
public void testWithTernaryOperator() throws Exception {
loadCodeAndRun(getAboveComment());
checkErrorLine(4, ERR_ID);
}
//int foo() {
// float a1;
// if (a1 <= 3.14 && a1 >= 3.14)
// a1++;
//}
public void testWithIndirectEqual1() throws Exception {
loadCodeAndRun(getAboveComment());
checkErrorLine(3, ERR_ID);
}
//int foo() {
// float a1;
// if (a1 >= 3.14 && a1 <= 3.14)
// a1++;
//}
public void testWithIndirectEqual2() throws Exception {
loadCodeAndRun(getAboveComment());
checkErrorLine(3, ERR_ID);
}
//int foo() {
// float a1;
// if (!(a1 != 2))
// a1++;
//}
public void testWithIndirectEqual3() throws Exception {
loadCodeAndRun(getAboveComment());
checkErrorLine(3, ERR_ID);
}
//int foo() {
// float a1;
// if (!(a1 < 3.14 || a1 > 3.14))
// a1++;
//}
public void testWithIndirectEqual4() throws Exception {
loadCodeAndRun(getAboveComment());
checkErrorLine(3, ERR_ID);
}
//int foo() {
// float a1;
// if (a1 < 3.14 || a1 > 3.14)
// a1++;
//}
public void testWithIndirectNotEqual1() throws Exception {
loadCodeAndRun(getAboveComment());
checkErrorLine(3, ERR_ID);
}
//int foo() {
// float a1;
// if (a1 > 3.14 || a1 < 3.14)
// a1++;
//}
public void testWithIndirectNotEqual2() throws Exception {
loadCodeAndRun(getAboveComment());
checkErrorLine(3, ERR_ID);
}
//int foo() {
// float a1;
// if (a1 < 3.14 || a1 > 3.15)
// a1++;
//}
public void testWithUpperLowerBounds() throws Exception {
loadCodeAndRun(getAboveComment());
checkNoErrorsOfKind(ERR_ID);
}
}

4
codan/org.eclipse.cdt.codan.core.tests/src/org/eclipse/cdt/codan/core/tests/AutomatedIntegrationSuite.java
View file

@ -26,6 +26,7 @@ import org.eclipse.cdt.codan.core.internal.checkers.CommentCheckerLineTests;
import org.eclipse.cdt.codan.core.internal.checkers.CommentCheckerNestedTests;
import org.eclipse.cdt.codan.core.internal.checkers.CopyrightCheckerTest;
import org.eclipse.cdt.codan.core.internal.checkers.DecltypeAutoCheckerTest;
import org.eclipse.cdt.codan.core.internal.checkers.FloatCompareCheckerTest;
import org.eclipse.cdt.codan.core.internal.checkers.FormatStringCheckerTest;
import org.eclipse.cdt.codan.core.internal.checkers.GotoStatementCheckerTest;
import org.eclipse.cdt.codan.core.internal.checkers.NonVirtualDestructorCheckerTest;
@ -37,6 +38,7 @@ import org.eclipse.cdt.codan.core.internal.checkers.SuggestedParenthesisCheckerT
import org.eclipse.cdt.codan.core.internal.checkers.SuspiciousSemicolonCheckerTest;
import org.eclipse.cdt.codan.core.internal.checkers.SwitchCaseCheckerTest;
import org.eclipse.cdt.codan.core.internal.checkers.UnusedSymbolInFileScopeCheckerTest;
import org.eclipse.cdt.codan.core.internal.checkers.UsingInHeaderCheckerTest;
import org.eclipse.cdt.codan.core.internal.checkers.VariablesCheckerTest;
import org.eclipse.cdt.codan.core.internal.checkers.VirtualMethodCallCheckerTest;
import org.eclipse.cdt.codan.internal.checkers.ui.quickfix.AssignmentInConditionQuickFixTest;
@ -99,6 +101,8 @@ public class AutomatedIntegrationSuite extends TestSuite {
suite.addTestSuite(VirtualMethodCallCheckerTest.class);
suite.addTestSuite(AssignmentOperatorCheckerTest.class);
suite.addTestSuite(VariablesCheckerTest.class);
suite.addTestSuite(UsingInHeaderCheckerTest.class);
suite.addTestSuite(FloatCompareCheckerTest.class);
// framework
suite.addTest(CodanFastTestSuite.suite());
// quick fixes

2
core/org.eclipse.cdt.core/META-INF/MANIFEST.MF
View file

@ -2,7 +2,7 @@ Manifest-Version: 1.0
Bundle-ManifestVersion: 2
Bundle-Name: %pluginName
Bundle-SymbolicName: org.eclipse.cdt.core; singleton:=true
Bundle-Version: 6.9.0.qualifier
Bundle-Version: 6.10.0.qualifier
Bundle-Activator: org.eclipse.cdt.core.CCorePlugin
Bundle-Vendor: %providerName
Bundle-Localization: plugin

2
core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/semantics/CPPVisitor.java
View file

@ -1733,7 +1733,7 @@ public class CPPVisitor extends ASTQueries {
return areEquivalentBindings(candidate, target, index);
}
private static boolean areEquivalentBindings(IBinding binding1, IBinding binding2, IIndex index) {
public static boolean areEquivalentBindings(IBinding binding1, IBinding binding2, IIndex index) {
if (binding1.equals(binding2)) {
return true;
}