Skip to content

Commit

Permalink
Fix assertion evaluating binary shifts of comparison op results
Browse files Browse the repository at this point in the history
Currently CDT evaluates result type of comparison ops to the converted type of
their operands. When floating point values are compared and then the result is
binary shifted this trips an assertion trying to promote floating point value.

Fix this by limiting operand types of binary shifts to integral or unscoped
enumerations. Additionally fix return type of comparison op to be boolean value.
  • Loading branch information
i-garrison authored and jonahgraham committed Mar 16, 2023
1 parent c69eed4 commit f1c15e3
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@
import org.eclipse.cdt.core.dom.ast.ISemanticProblem;
import org.eclipse.cdt.core.dom.ast.IType;
import org.eclipse.cdt.core.dom.ast.ITypedef;
import org.eclipse.cdt.core.dom.ast.IValue;
import org.eclipse.cdt.core.dom.ast.IVariable;
import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTBinaryExpression;
import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTCastExpression;
Expand Down Expand Up @@ -147,6 +148,7 @@
import org.eclipse.cdt.core.parser.ParserLanguage;
import org.eclipse.cdt.core.parser.util.AttributeUtil;
import org.eclipse.cdt.core.parser.util.CharArrayUtils;
import org.eclipse.cdt.internal.core.dom.parser.IntegralValue;
import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTNameBase;
import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPBasicType;
import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPClassInstance;
Expand Down Expand Up @@ -13845,4 +13847,29 @@ public void testArithmeticConversionIssue_265() throws Exception {
helper.assertVariableValue("test_32", 1);
helper.assertVariableValue("test_64", 1);
}

// constexpr auto shiftdouble = (1. << 1);
public void testArithmeticEvaluationWithDoubleShiftOp() throws Exception {
BindingAssertionHelper helper = getAssertionHelper();
IVariable shiftdouble = helper.assertNonProblem("shiftdouble = ", 11);
IValue value = shiftdouble.getInitialValue();
assertTrue(IntegralValue.ERROR.equals(value) || IntegralValue.UNKNOWN.equals(value));
}

// constexpr auto shiftdouble = (1. <<= 1);
public void testArithmeticEvaluationWithDoubleShiftAssignOp() throws Exception {
BindingAssertionHelper helper = getAssertionHelper();
IVariable shiftdouble = helper.assertNonProblem("shiftdouble = ", 11);
IValue value = shiftdouble.getInitialValue();
assertTrue(IntegralValue.ERROR.equals(value) || IntegralValue.UNKNOWN.equals(value));
}

// enum Shift { Horizontal, Vertical };
// constexpr double zero = 0.;
// constexpr double x = 1., y = 1.;
// constexpr int shiftpack = ((x > zero) << Horizontal) | ((y > zero) << Vertical);
public void testArithmeticEvaluationOfRelationalOps() throws Exception {
BindingAssertionHelper helper = getAssertionHelper();
helper.assertVariableValue("shiftpack", 3);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,12 @@ public final IType convertOperandTypes(int operator, IType op1, IType op2) {
return convert(op1, op2);

case IASTBinaryExpression.op_shiftLeft:
case IASTBinaryExpression.op_shiftLeftAssign:
case IASTBinaryExpression.op_shiftRight:
case IASTBinaryExpression.op_shiftRightAssign:
if (!isIntegralOrUnscopedEnum(op1) || !isIntegralOrUnscopedEnum(op2)) {
return null;
}
return promote(op1, getDomain(op1));

default:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ public class EvalBinary extends CPPDependentEvaluation {
private ICPPFunction fOverload = CPPFunction.UNINITIALIZED_FUNCTION;
private ICPPEvaluation fOverloadCall;
private IType fType;
private IType fCommonType;
private boolean fCheckedIsConstantExpression;
private boolean fIsConstantExpression;

Expand Down Expand Up @@ -160,6 +161,13 @@ public IType getType() {
return fType;
}

private IType getCommonType() {
if (fCommonType == null) {
fCommonType = computeCommonType();
}
return fCommonType;
}

private ICPPEvaluation createOperatorOverloadEvaluation(ICPPFunction overload, ICPPEvaluation arg1,
ICPPEvaluation arg2) {
EvalFunctionCall operatorCall;
Expand Down Expand Up @@ -240,7 +248,7 @@ public IValue getValue() {

Number num2 = v2.numberValue();
if (num2 != null) {
return ValueFactory.evaluateBinaryExpression(fOperator, v1, v2, getType());
return ValueFactory.evaluateBinaryExpression(fOperator, v1, v2, getCommonType());
}
}
return DependentValue.create(this);
Expand Down Expand Up @@ -352,6 +360,26 @@ public IType computeType() {
if (o != null)
return typeFromFunctionCall(o);

switch (fOperator) {
case op_lessEqual:
case op_lessThan:
case op_greaterEqual:
case op_greaterThan:
case op_logicalAnd:
case op_logicalOr:
case op_equals:
case op_notequals:
return CPPBasicType.BOOLEAN;

case op_threewaycomparison:
// TODO: implement for <=>
return ProblemType.UNKNOWN_FOR_EXPRESSION;
}

return getCommonType();
}

private IType computeCommonType() {
final IType originalType1 = fArg1.getType();
final IType type1 = prvalueTypeWithResolvedTypedefs(originalType1);
if (type1 instanceof ISemanticProblem) {
Expand Down Expand Up @@ -379,20 +407,6 @@ public IType computeType() {
}
return ProblemType.UNKNOWN_FOR_EXPRESSION;

case op_lessEqual:
case op_lessThan:
case op_greaterEqual:
case op_greaterThan:
case op_logicalAnd:
case op_logicalOr:
case op_equals:
case op_notequals:
return CPPBasicType.BOOLEAN;

case op_threewaycomparison:
// TODO: implement for <=>
return ProblemType.UNKNOWN_FOR_EXPRESSION;

case op_plus:
if (type1 instanceof IPointerType) {
return ExpressionTypes.restoreTypedefs(type1, originalType1);
Expand Down

0 comments on commit f1c15e3

Please sign in to comment.