Fix #11093, #12377 FP mismatchingContainerIterator #5911
Conversation
chrchr-github
changed the title
| @@ -866,6 +879,8 @@ void CheckStl::mismatchingContainerIterator() | |||
| ValueFlow::Value val = getLifetimeIteratorValue(iterTok); | |||
| if (!val.tokvalue) | |||
| continue; | |||
| if (!val.isKnown() && Token::simpleMatch(val.tokvalue->astParent(), ":")) | |||
There was a problem hiding this comment.
The lifetime values are never known. Also, this will lead to FNs as well. I think it would be better to check that the set of lifetimes are the same when there are multiple lifetimes.
There was a problem hiding this comment.
Isn't the problem that there is only one value? Otherwise, getLifetimeIteratorValue() would return an empty value.
There was a problem hiding this comment.
Ah yea, thats right, but it should be multiple values. I wonder why we are not getting both lifetime.
I assumed the error is that one variable has the lifetime v1 and the other has the lifetime v2. It should have both lifetimes.
lib/checkstl.cpp
Outdated
| for (std::size_t i = 0; i < address1.size(); ++i) | ||
| for (std::size_t j = 0; j < address2.size(); ++j) | ||
| if (isSameExpression(true, false, address1[i], address2[j], library, false, false)) | ||
| return true; |
There was a problem hiding this comment.
I am not sure the reason for doing this, but it seems like it would be better to check if the entire set matches. This could be written as a nested std::any_of but it should probably be a nested std::all_of.
There was a problem hiding this comment.
It's for #12377. We used to get stuck on pv, which actually has two values (get1 or get2).
|
@pfultz2 I think we should merge this, even if the ValueFlow is still wrong. As it stands, this check produces mostly FPs in daca. |
…r to get reference to container