Skip to content

Commit d214ce0

Browse files
pfultz2pfultz2
committed
Improve isIterator
1 parent b805909 commit d214ce0

3 files changed

Lines changed: 36 additions & 44 deletions

File tree

‎lib/astutils.cpp‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1074,11 +1074,9 @@ bool isIteratorOf(const Token* tok, nonneg int exprId)
10741074
if (!astIsIterator(tok))
10751075
return false;
10761076
// An iterator into a subcontainer (e.g. c[0].begin()) aliases the container but iterates
1077-
// an unrelated range, so require an Iterator lifetime referring to the container itself
1077+
// an unrelated range, so require an iterator value recording the container itself
10781078
return std::any_of(tok->values().cbegin(), tok->values().cend(), [&](const ValueFlow::Value& v) {
1079-
return v.isLocalLifetimeValue() && !v.isInconclusive() &&
1080-
v.lifetimeKind == ValueFlow::Value::LifetimeKind::Iterator && v.tokvalue &&
1081-
v.tokvalue->exprId() == exprId;
1079+
return v.isIteratorValue() && v.container && v.container->exprId() == exprId;
10821080
});
10831081
}
10841082

‎lib/checkstl.cpp‎

Lines changed: 12 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -765,17 +765,14 @@ static ValueFlow::Value getLifetimeIteratorValue(const Token* tok, MathLib::bigi
765765
return ValueFlow::Value{};
766766
}
767767

768-
// Whether a container size value found on an iterator token belongs to the range of that
769-
// iterator. The size value records its container when known; the container of the iterator
770-
// is its lifetime container (getLifetimeIteratorValue(tok).tokvalue).
771-
static bool sizeValueAppliesToIterator(const ValueFlow::Value& sizeValue, const Token* iterContainer)
768+
// Whether a container size value found on an iterator token belongs to the range of the given
769+
// iterator value. Both values record the container they belong to when it is known.
770+
static bool sizeValueAppliesToIterator(const ValueFlow::Value& sizeValue, const ValueFlow::Value& iterValue)
772771
{
773-
if (!sizeValue.container)
774-
return true; // the origin of the size is not known
775-
if (!iterContainer)
776-
return true; // the range of the iterator is not known
777-
return iterContainer == sizeValue.container ||
778-
(iterContainer->exprId() != 0 && iterContainer->exprId() == sizeValue.container->exprId());
772+
if (!sizeValue.container || !iterValue.container)
773+
return true; // the container of the size or of the iterator is not known
774+
return iterValue.container == sizeValue.container ||
775+
(iterValue.container->exprId() != 0 && iterValue.container->exprId() == sizeValue.container->exprId());
779776
}
780777

781778
bool CheckStlImpl::checkIteratorPair(const Token* tok1, const Token* tok2)
@@ -2522,15 +2519,6 @@ void CheckStlImpl::checkDereferenceInvalidIterator2()
25222519
[&](const ValueFlow::Value& value) {
25232520
return isUsableValue(value, mSettings) && value.isContainerSizeValue();
25242521
});
2525-
if (!contValues.empty()) {
2526-
const Token* const iterContainer = getLifetimeIteratorValue(tok).tokvalue;
2527-
contValues.erase(std::remove_if(contValues.begin(),
2528-
contValues.end(),
2529-
[&](const ValueFlow::Value& value) {
2530-
return !sizeValueAppliesToIterator(value, iterContainer);
2531-
}),
2532-
contValues.end());
2533-
}
25342522

25352523
// Can iterator point to END or before START?
25362524
for (const ValueFlow::Value& value:tok->values()) {
@@ -2548,6 +2536,8 @@ void CheckStlImpl::checkDereferenceInvalidIterator2()
25482536
auto it = std::find_if(contValues.cbegin(), contValues.cend(), [&](const ValueFlow::Value& c) {
25492537
if (value.path != c.path)
25502538
return false;
2539+
if (!sizeValueAppliesToIterator(c, value))
2540+
return false;
25512541
if (value.isIteratorStartValue() && value.intvalue >= c.intvalue)
25522542
return true;
25532543
if (value.isIteratorEndValue() && -value.intvalue > c.intvalue)
@@ -3469,10 +3459,9 @@ static IteratorPosition getIteratorPosition(const Token* tok, const Settings& se
34693459
});
34703460
if (!position.value)
34713461
return position;
3472-
const Token* const iterContainer = getLifetimeIteratorValue(tok).tokvalue;
34733462
position.sizeValue = selectPreferredValue(tok, [&](const ValueFlow::Value& value) {
34743463
return isUsableValue(value, settings) && value.isContainerSizeValue() && value.path == position.value->path &&
3475-
sizeValueAppliesToIterator(value, iterContainer);
3464+
sizeValueAppliesToIterator(value, *position.value);
34763465
});
34773466
return position;
34783467
}
@@ -3530,7 +3519,6 @@ static ElementCount findInsufficientSpace(const Token* tok,
35303519
BestCandidate insufficient;
35313520
if (!tok)
35323521
return insufficient.best;
3533-
const Token* const iterContainer = getLifetimeIteratorValue(tok).tokvalue;
35343522
const auto consider = [&](const ElementCount& candidate) {
35353523
if (!candidate)
35363524
return;
@@ -3552,7 +3540,7 @@ static ElementCount findInsufficientSpace(const Token* tok,
35523540
for (const ValueFlow::Value& sizeValue : tok->values()) {
35533541
if (!isUsableValue(sizeValue, settings) || !sizeValue.isContainerSizeValue() || sizeValue.path != value.path)
35543542
continue;
3555-
if (!sizeValueAppliesToIterator(sizeValue, iterContainer))
3543+
if (!sizeValueAppliesToIterator(sizeValue, value))
35563544
continue;
35573545
position.sizeValue = &sizeValue;
35583546
consider(getAvailableSpace(position));
@@ -3577,8 +3565,6 @@ static ElementCount findExcessiveDistance(const Token* firstTok,
35773565
return; // the access is within bounds
35783566
excessive.consider(candidate);
35793567
};
3580-
const Token* const firstContainer = getLifetimeIteratorValue(firstTok).tokvalue;
3581-
const Token* const lastContainer = getLifetimeIteratorValue(lastTok).tokvalue;
35823568
for (const ValueFlow::Value& firstValue : firstTok->values()) {
35833569
if (!isUsableValue(firstValue, settings) || !firstValue.isIteratorValue())
35843570
continue;
@@ -3596,12 +3582,11 @@ static ElementCount findExcessiveDistance(const Token* firstTok,
35963582
}
35973583
IteratorPosition& endPosition = first.fromEnd() ? first : last;
35983584
const Token* const endTok = first.fromEnd() ? firstTok : lastTok;
3599-
const Token* const endContainer = first.fromEnd() ? firstContainer : lastContainer;
36003585
for (const ValueFlow::Value& sizeValue : endTok->values()) {
36013586
if (!isUsableValue(sizeValue, settings) || !sizeValue.isContainerSizeValue() ||
36023587
sizeValue.path != endPosition.value->path)
36033588
continue;
3604-
if (!sizeValueAppliesToIterator(sizeValue, endContainer))
3589+
if (!sizeValueAppliesToIterator(sizeValue, *endPosition.value))
36053590
continue;
36063591
endPosition.sizeValue = &sizeValue;
36073592
consider(getIteratorDistance(first, last));

‎lib/valueflow.cpp‎

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -401,10 +401,14 @@ void ValueFlow::combineValueProperties(const ValueFlow::Value &value1, const Val
401401
result.valueType = value2.valueType;
402402
result.tokvalue = value2.tokvalue;
403403
}
404-
if (value1.isIteratorValue())
404+
if (value1.isIteratorValue()) {
405405
result.valueType = value1.valueType;
406-
if (value2.isIteratorValue())
406+
result.container = value1.container;
407+
}
408+
if (value2.isIteratorValue()) {
407409
result.valueType = value2.valueType;
410+
result.container = value2.container;
411+
}
408412
result.condition = value1.condition ? value1.condition : value2.condition;
409413
result.varId = (value1.varId != 0) ? value1.varId : value2.varId;
410414
result.varvalue = (result.varId == value1.varId) ? value1.varvalue : value2.varvalue;
@@ -6451,17 +6455,22 @@ static void valueFlowIterators(TokenList& tokenlist, const Settings& settings)
64516455
const Library::Container::Yield yield = findIteratorYield(tok, ftok, settings.library);
64526456
if (!ftok)
64536457
continue;
6454-
if (yield == Library::Container::Yield::START_ITERATOR) {
6455-
ValueFlow::Value v(0);
6456-
v.setKnown();
6457-
v.valueType = ValueFlow::Value::ValueType::ITERATOR_START;
6458-
setTokenValue(const_cast<Token*>(ftok)->next(), std::move(v), settings);
6459-
} else if (yield == Library::Container::Yield::END_ITERATOR) {
6460-
ValueFlow::Value v(0);
6461-
v.setKnown();
6462-
v.valueType = ValueFlow::Value::ValueType::ITERATOR_END;
6463-
setTokenValue(const_cast<Token*>(ftok)->next(), std::move(v), settings);
6464-
}
6458+
if (yield != Library::Container::Yield::START_ITERATOR && yield != Library::Container::Yield::END_ITERATOR)
6459+
continue;
6460+
// The iterator value records the container it iterates. A pointer or a reference only
6461+
// transports the iterator, so record the container it refers to instead.
6462+
const Token* containerTok = tok;
6463+
if (astIsPointer(containerTok) || (containerTok->variable() && containerTok->variable()->isReference())) {
6464+
const ValueFlow::Value lifetime = ValueFlow::getLifetimeObjValue(containerTok);
6465+
if (lifetime.tokvalue && astIsContainer(lifetime.tokvalue) && !astIsPointer(lifetime.tokvalue))
6466+
containerTok = lifetime.tokvalue;
6467+
}
6468+
ValueFlow::Value v(0);
6469+
v.setKnown();
6470+
v.valueType = yield == Library::Container::Yield::START_ITERATOR ? ValueFlow::Value::ValueType::ITERATOR_START
6471+
: ValueFlow::Value::ValueType::ITERATOR_END;
6472+
v.container = containerTok;
6473+
setTokenValue(const_cast<Token*>(ftok)->next(), std::move(v), settings);
64656474
}
64666475
}
64676476

0 commit comments

Comments
 (0)