Skip to content

Improve isSameExpression for literals#2514

Merged
danmar merged 1 commit into
cppcheck-opensource:masterfrom
rikardfalkeborn:improve-issameexpression-for-literals
Feb 1, 2020
Merged

Improve isSameExpression for literals#2514
danmar merged 1 commit into
cppcheck-opensource:masterfrom
rikardfalkeborn:improve-issameexpression-for-literals

Conversation

@rikardfalkeborn
Copy link
Copy Markdown
Contributor

Improve isSameExpression() for literals with same value but different
representation, for example the following different ways of
representing 9 as double: 9.0, 0.9e1 and 0x1.2p3.

With this change, cppcheck can (for example) correctly detect that the
else if statements are always false in the following example:

void f(double x) {
	if (x < 9.0) {}
	else if (x < 0x1.2p3) {}
	else if (x < 0.9e1) {}
}

test-my-pr shows no problems with this, except... negative floating point zero, which seems to be optional in the standard (but IEEE floating point standard mandates it, and at least on my computer (linux), gcc (and glibc) treats them slightly different). From my understanding 0.0 and -0.0 are equal in most aspects, (for example 0.0 == -0.0 is true, 0.0 > -0.0 is false). But! Math functions might treat them separately. For example atan2(0.0, -0.0) equals 3.1415..., while atan2(-0.0, -0.0) equals -3.1415....

With this change, 0.0 and -0.0 are considered equal (previously, they were not).

This allows cppcheck to detect duplicate conditions like the following

if (x == 0.0) {}
else if (x == -0.0) {} // TP, expression is always false since it matches the if

which is good, but it also gives false positives on the following

if (x == -0.0) {
    x = 0.0; // FP, redundant assignment
}

(you could argue that there are alread FPs in the following code:

if (x == 0.0) {
    x = 0.0; // FP, redundant assignment
}

Since x might be negative zero, and the assignment assures that x is positive zero.

This is not a huge problem. I ran test-my-pr with 1000 packages and counted 6 FP duplicateConditionalAssign, 1 TP multiCondition.

Is it worth adding a flag to isSameExpression() so that checkers can decided for themselves whether or not they want to treat positive and negative zero as same expression?

Improve isSameExpression() for literals with same value but different
representation, for example  the following different ways of
representing 9 as double: 9.0, 0.9e1 and 0x1.2p3.

With this change, cppcheck can (for example) correctly detect that the
else if statements are always false in the following example:

	void f(double x) {
		if (x < 9.0) {}
		else if (x < 0x1.2p3) {}
		else if (x < 0.9e1) {}
	}
@danmar
Copy link
Copy Markdown
Collaborator

danmar commented Feb 1, 2020

I'll merge it now.. let's see how it works.

@danmar danmar merged commit ff9c04d into cppcheck-opensource:master Feb 1, 2020
@rikardfalkeborn rikardfalkeborn deleted the improve-issameexpression-for-literals branch February 1, 2020 08:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants