avoid duplicated evaluation in ProgramMemoryState::removeModifiedVars()#6628
avoid duplicated evaluation in ProgramMemoryState::removeModifiedVars()#6628firewave merged 1 commit intocppcheck-opensource:mainfrom
ProgramMemoryState::removeModifiedVars()#6628Conversation
|
Clang 18 @pfultz2 please have a look |
firewave
changed the title
ProgramMemoryState::removeModifiedVars()ProgramMemoryState::removeModifiedVars()
|
This should help with https://trac.cppcheck.net/ticket/12271. |
|
Looks like this (anti-) pattern also exists in |
Yes, I did not touch those because they are not in the hot path. Well one of them shows up in the profiler but changing it let to strange results with one of the compilers so I left it for now. |
| { | ||
| if (!condition) | ||
| return false; | ||
| MathLib::bigint result = 0; |
There was a problem hiding this comment.
To be on the safe side, we should initialize result before the first return.
| return {1}; | ||
| if (result == 0) | ||
| return {0}; | ||
| } |
There was a problem hiding this comment.
I think it would be better to just call execute and use the isTrue and isFalse functions:
auto result = execute(cond, pm, *settings);
if(isTrue(result))
return {1};
if(isFalse(result))
return {0};
return {};There was a problem hiding this comment.
Then there is no need to refactor the evaluateCondition function.
There was a problem hiding this comment.
And this same code could be used in evaluateInt as well, we just need to expose the isTrue and isFalse functions.
There was a problem hiding this comment.
Done. As mentioned before I will not touch other code until I encounter it in a profile.
…::removeModifiedVars()` Co-authored-by: Paul Fultz II <pfultz2@yahoo.com>
3 participants
No description provided.