Skip to content

Commit d3f0365

Browse files
author
Your Name
committed
Update comments
1 parent 9fdfa55 commit d3f0365

4 files changed

Lines changed: 31 additions & 42 deletions

File tree

lib/analyzer.h

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -156,13 +156,9 @@ struct Analyzer {
156156
Quiet = (1 << 0),
157157
Absolute = (1 << 1),
158158
ContainerEmpty = (1 << 2),
159-
// The branch this condition guards is still pending traversal (it is walked by a
160-
// separate path), so this assume must not record the program state at the branch
161-
// boundaries - those states would be premature. When this is not set the branch has
162-
// already been traversed and control is continuing past it, so the assumed state is
163-
// anchored at the block's end (see the analyzer's assume()): this keeps assumptions on
164-
// variables modified inside the block from being discarded as "modified" once control
165-
// leaves it.
159+
// The branch this condition guards is not traversed yet (a separate path walks it), so
160+
// the assume must not record the program state at the branch boundaries - they would be
161+
// premature. When unset, the branch has been traversed and control is leaving it.
166162
Pending = (1 << 3),
167163
};
168164
};

lib/forwardanalyzer.cpp

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -426,9 +426,8 @@ namespace {
426426
branch.escape = true;
427427
branch.escapeUnknown = false;
428428
} else {
429-
// Detect an escape that the traversal did not flag (e.g. an unknown noreturn call);
430-
// isEscapeScope also reports a possible (unknown) escape via escapeUnknown. Such a
431-
// branch may not fall through, so the fork must not continue past it.
429+
// Detect an escape the traversal did not flag (e.g. an unknown noreturn call);
430+
// escapeUnknown reports a possible (unknown) escape.
432431
branch.escape = isEscapeScope(branch.endBlock, branch.escapeUnknown);
433432
if (terminate != Analyzer::Terminate::None && terminate != Analyzer::Terminate::Modified) {
434433
branch.action |= analyzeScope(branch.endBlock);
@@ -679,10 +678,9 @@ namespace {
679678
Token* condTok = getCondTokFromEnd(tok);
680679
if (!condTok)
681680
return Break();
682-
// When leaving the 'then' branch, control only reaches here when the
683-
// condition was true if the 'else' branch escapes (e.g. it returns). In that
684-
// case the value established in 'then' is still definite, so keep it known
685-
// instead of lowering it to possible.
681+
// When the 'else' branch escapes (e.g. returns), control can only continue
682+
// here via the 'then' branch, so the value established there is still
683+
// definite - keep it known instead of lowering to possible.
686684
bool elseEscape = false;
687685
bool unknownEscape = false;
688686
if (!inLoop && !inElse && hasElse)
@@ -788,18 +786,18 @@ namespace {
788786
} else {
789787
const bool conditional = stopOnCondition(condTok);
790788
// The value only flows into the then-branch when the condition can split
791-
// it. For an opaque or correlated condition (e.g. 'if (f(x))' or
792-
// 'if (do_write)') it does not really reach there, so fork in analyze-only
793-
// mode: the branch's effect is still tracked but nothing is reported in it.
789+
// it; for an opaque or correlated condition (e.g. 'if (f(x))') it does
790+
// not, so fork in analyze-only mode: the branch's effect is still tracked
791+
// but nothing is reported in it.
794792
ForwardTraversal ft = fork(!analyzer->updateScope(thenBranch.endBlock, false));
795793
// The branch is traversed below, so don't record its boundary state here.
796794
ft.analyzer->assume(condTok, true, Analyzer::Assume::Pending);
797795
Progress pThen = ft.updateBranch(thenBranch, depth - 1);
798796

799-
// Only commit the condition as false on the main path when it actually
800-
// matters. With an else the else block is traversed below (so suppress the
801-
// boundary state); without one the false path continues past the closing
802-
// brace and must record the assumed state there.
797+
// Commit the condition as false on the main path only when the then-branch
798+
// is dead. The else block, if any, is traversed separately (Pending); with
799+
// no else the false path continues past the closing brace, so record the
800+
// assumed state there (None).
803801
if (thenBranch.isDead())
804802
analyzer->assume(condTok, false, hasElse ? Analyzer::Assume::Pending : Analyzer::Assume::None);
805803
// The else block is traversed on the main path. If it kills the value

lib/programmemory.cpp

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1308,9 +1308,8 @@ namespace {
13081308
struct Executor {
13091309
ProgramMemory* pm;
13101310
const Settings& settings;
1311-
// The values that are being tracked by the forward/reverse analysis. These take precedence
1312-
// over what is stored in the program memory: a tracked value is the authoritative current
1313-
// value of its expression, so any cached value that depends on it must be re-evaluated.
1311+
// Values tracked by the forward/reverse analysis. A tracked value is the authoritative
1312+
// current value of its expression and takes precedence over the program memory.
13141313
const ProgramMemory::Map* vars = nullptr;
13151314
int fdepth = 4;
13161315
int depth = 10;
@@ -1328,9 +1327,8 @@ namespace {
13281327
return it == vars->end() ? nullptr : &it->second;
13291328
}
13301329

1331-
// Does the expression read a tracked value? If so, any value cached in the program memory
1332-
// for it may be stale (the tracked value may have changed since it was cached), so it must
1333-
// be re-evaluated rather than served from the cache.
1330+
// Does the expression read a tracked value? If so, any value cached for it may be stale
1331+
// (the tracked value may have changed since), so it must be re-evaluated, not served cached.
13341332
bool dependsOnTrackedValue(const Token* expr) const {
13351333
if (!vars || vars->empty())
13361334
return false;
@@ -1637,9 +1635,8 @@ namespace {
16371635
}
16381636
return execute(expr->astOperand1());
16391637
}
1640-
// A tracked value is the authoritative current value of its expression. Write it back
1641-
// into the program memory when it differs from what is stored, so that later reads see
1642-
// the same value (matching the substitution done by fillProgramMemoryFromAssignments).
1638+
// Return the tracked value and write it back when it differs, so later reads see the
1639+
// same value (as fillProgramMemoryFromAssignments used to do).
16431640
if (const ValueFlow::Value* tracked = getTrackedValue(expr)) {
16441641
const ValueFlow::Value* stored = pm->getValue(expr->exprId(), /*impossible*/ true);
16451642
if (!stored || *stored != *tracked)

lib/vf_analyzers.cpp

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -678,10 +678,9 @@ struct ValueFlowAnalyzer : Analyzer {
678678
if (const ValueFlow::Value* v = tok->getKnownValue(ValueFlow::Value::ValueType::INT))
679679
return {v->intvalue};
680680
std::vector<MathLib::bigint> result;
681-
// The tracked values are authoritative: pass them so a value cached in the program memory
682-
// that depends on a tracked value (e.g. 'h(p)' after 'p' was reassigned) is re-evaluated
683-
// rather than served stale. The program memory is built from the same state, so compute it
684-
// once here and hand it to the builder.
681+
// Pass the tracked values so a cached program-memory value that depends on one (e.g. 'h(p)'
682+
// after 'p' was reassigned) is re-evaluated rather than served stale. The memory is built
683+
// from the same state, so compute it once and hand it to the builder.
685684
const ProgramState vars = getProgramState();
686685
ProgramMemory pm = getProgramMemoryFunc(vars);
687686
if (Token::Match(tok, "&&|%oror%")) {
@@ -743,11 +742,10 @@ struct ValueFlowAnalyzer : Analyzer {
743742
endBlock = startBlock->link();
744743
}
745744

746-
// Pending is set only for the pre-traversal assume; without it the 'then' block has already
747-
// been traversed and control is leaving it, so anchor the assumed state at the end of the
748-
// block rather than at the condition. Assumptions about variables modified inside the block
749-
// (e.g. an 'if' that narrows a value computed in the block) then survive past it, instead of
750-
// being discarded because the variable was "modified" since the condition was evaluated.
745+
// Without Pending the 'then' block has been traversed and control is leaving it, so anchor
746+
// the assumed state at the block end instead of the condition. That keeps assumptions on
747+
// variables modified inside the block (e.g. an 'if' narrowing a value computed there) from
748+
// being discarded as "modified" once control leaves the block.
751749
const bool scopeEnd = !(flags & Assume::Pending) && state && endBlock;
752750
const Token* anchor = scopeEnd ? endBlock : tok;
753751
const Token* origin = scopeEnd ? endBlock : nullptr;
@@ -757,9 +755,9 @@ struct ValueFlowAnalyzer : Analyzer {
757755
pms.addState(anchor, getProgramState());
758756
pms.assume(tok, state, flags & Assume::ContainerEmpty, origin);
759757

760-
// On the false path the block was already traversed (the true path is handled by scopeEnd
761-
// above), so record the assumed state where control continues: past the else block, or past
762-
// the closing brace when there is no else, so it is available to the enclosing scope.
758+
// The false path (the true path uses scopeEnd above): record the assumed state where control
759+
// continues - the end of the else block, or the closing brace when there is no else - so it
760+
// reaches the enclosing scope.
763761
if (isCondBlock && !(flags & Assume::Pending) && !state) {
764762
if (Token::simpleMatch(endBlock, "} else {"))
765763
pms.addState(endBlock->linkAt(2)->previous(), getProgramState());

0 commit comments

Comments
 (0)