Uh oh!
There was an error while loading. Please reload this page.
some updates and fixes for clang-14 - #3794
Conversation
firewave
commented
Feb 27, 2022
I think the compiler warnings reported in |
danmar
commented
Mar 7, 2022
Yeah that is by intention. I don't agree that a boolean must never be used in a bitwise operation. |
| void CheckCondition::duplicateConditionError(const Token *tok1, const Token *tok2, ErrorPath errorPath) | ||
| { | ||
| if (diag(tok1) & diag(tok2)) |
There was a problem hiding this comment.
as diag might be relatively expensive and as far as I see there are no desired side effects in rhs, I think a jump here makes sense.
There was a problem hiding this comment.
If the intention is that both function should run (I wasn't aware the bitwise operator would not short circuit until I just read up on it) the code should reflect that. This just looks too much like a typo.
This seems to affect the output though as the unit tests now fail. I don't know if that is correct.
There was a problem hiding this comment.
If the intention is that both function should run (I wasn't aware the bitwise operator would not short circuit until I just read up on it) the code should reflect that. This just looks too much like a typo.
imho I don't agree it looks like a typo.
| ValueFlow::Value result(0); | ||
| result.condition = value1.condition ? value1.condition : value2.condition; | ||
| result.setInconclusive(value1.isInconclusive() | value2.isInconclusive()); | ||
| result.setInconclusive(value1.isInconclusive() || value2.isInconclusive()); |
There was a problem hiding this comment.
isInconclusive can be easily inlined here. I don't like the clang warning here.
There was a problem hiding this comment.
I think it has to do with the bitwise operator having precedence over the logical ones so if you would use ! in there the result would be unexpected. Still this just looks like a typo to me.
There was a problem hiding this comment.
so if you would use ! in there the result would be unexpected
That would have been unexpected to me.. but ! still has higher precedence than both the logical and bitwise and.
It is not a typo in our code to use bitwise or/and. We do that intentionally sometimes. I am against this clang warning.
…we are not interested in
firewave
commented
May 11, 2022
I merged this into #4004 since that contains the actual update to clang-14 and it already had overlaps. |
The unused member function warnings is probably not clang-14 specific but occurred since I did not have Z3 enabled.
I will also check and file tickets if necessary since this seems like things we should detect ourselves.
A cleanup of all the existing clang warnings will happen after the simplecpp compiler warning fixes have been merged and hit this rep.
@danmar@pfultz2 Please review the operator changes and unit test failures since you introduced that code.