ValueFlow: Evaluate if statement for function returns#4908
ValueFlow: Evaluate if statement for function returns#4908firewave merged 14 commits intocppcheck-opensource:mainfrom
Conversation
| { | ||
| if (v.isImpossible()) { | ||
| return v.intvalue == 0; | ||
| } else { |
There was a problem hiding this comment.
We don't have readability-else-after-return enabled, but it's still redundant.
There was a problem hiding this comment.
I did not enable that because it's just too many changes and it doesn't really improve things much. In some cases it even breaks the if-else flow of some bugger chains since some scopes return and others don't which makes it feel "wrong".
| return lhs; | ||
| return execute(expr->astOperand2(), pm); | ||
| else if (isTrue(lhs)) | ||
| return execute(expr->astOperand2(), pm); |
There was a problem hiding this comment.
No else after return
| checkUnusedFunctions.parseTokens(tokenizer, filename.c_str(), &mSettings); | ||
|
|
||
| // handling of "simple" rules has been removed. | ||
| // cppcheck-suppress knownConditionTrueFalse |
There was a problem hiding this comment.
The implementation is disabled and it simply returns false when HAVE_RULES is not set. Depending on the case it could be considered a false positive. In this case I think it is fine and we could even move the whole method into the conditional code. The function is also private - would the function be protected or public it would be a different story as it would break the API if we make the function conditional.
I will prepare a PR adjusting these.
I think we had similar warnings with caseInsensitiveFilesystem() in path.cpp at some point. In that case I would consider it a false positive. But that looks like a case where it should use a constexpr instead of a function and thus it would not be an issue anymore.
There was a problem hiding this comment.
We may want to improve diagnostics in the future when its based on function call with #if conditionals in it(I believe we had a similar issue in the past). Its not trivial to implement and would make this PR much larger, so I think it would be better to merge this and then we can address this case in the future.
3 participants
No description provided.