Skip to content

Commit 82c5abe

Browse files
committed
merge
2 parents c214761 + a3361b3 commit 82c5abe

148 files changed

Lines changed: 9722 additions & 1109 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
# Automated code review of pull requests using Claude
2+
#
3+
# A member/owner/collaborator requests a review by writing a PR comment
4+
# containing "@claude review".
5+
#
6+
# pull_request_target is not used because the Claude GitHub App token exchange
7+
# rejects OIDC tokens from that event (401 "Invalid OIDC token").
8+
#
9+
# issue_comment runs the workflow file from the default branch with access to
10+
# secrets. The PR code is never checked out or executed.
11+
name: claude-review
12+
13+
on:
14+
issue_comment:
15+
types: [created]
16+
17+
jobs:
18+
review:
19+
if: |
20+
github.event.issue.pull_request &&
21+
contains(github.event.comment.body, '@claude review') &&
22+
contains(fromJSON('["OWNER","MEMBER","COLLABORATOR"]'), github.event.comment.author_association)
23+
24+
runs-on: ubuntu-24.04
25+
26+
permissions:
27+
contents: read
28+
pull-requests: write
29+
id-token: write
30+
31+
steps:
32+
# checks out the base branch, not the PR head
33+
- uses: actions/checkout@v7
34+
with:
35+
fetch-depth: 1
36+
37+
- name: Claude review
38+
uses: anthropics/claude-code-action@v1
39+
with:
40+
anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }}
41+
prompt: |
42+
REPO: ${{ github.repository }}
43+
PR NUMBER: ${{ github.event.issue.number }}
44+
45+
Review this pull request. Focus on correctness bugs, potential
46+
false positives/false negatives in checkers, performance problems
47+
and missing tests. Be concise and only report real issues.
48+
49+
Use `gh pr diff` to see the changes. Post specific issues as inline
50+
comments with `mcp__github_inline_comment__create_inline_comment`
51+
and post a short overall summary with `gh pr comment`.
52+
claude_args: |
53+
--allowedTools "mcp__github_inline_comment__create_inline_comment,Bash(gh pr comment:*),Bash(gh pr diff:*),Bash(gh pr view:*)"

‎Makefile‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -615,7 +615,7 @@ $(libcppdir)/forwardanalyzer.o: lib/forwardanalyzer.cpp lib/analyzer.h lib/astut
615615
$(libcppdir)/fwdanalysis.o: lib/fwdanalysis.cpp lib/astutils.h lib/checkers.h lib/config.h lib/errortypes.h lib/fwdanalysis.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/smallvector.h lib/sourcelocation.h lib/standards.h lib/symboldatabase.h lib/templatesimplifier.h lib/token.h lib/utils.h lib/vfvalue.h
616616
$(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/fwdanalysis.cpp
617617

618-
$(libcppdir)/importproject.o: lib/importproject.cpp externals/picojson/picojson.h externals/tinyxml2/tinyxml2.h lib/checkers.h lib/config.h lib/errortypes.h lib/filesettings.h lib/importproject.h lib/json.h lib/library.h lib/mathlib.h lib/path.h lib/pathmatch.h lib/platform.h lib/settings.h lib/smallvector.h lib/standards.h lib/suppressions.h lib/templatesimplifier.h lib/token.h lib/tokenlist.h lib/utils.h lib/vfvalue.h lib/xml.h
618+
$(libcppdir)/importproject.o: lib/importproject.cpp externals/picojson/picojson.h externals/tinyxml2/tinyxml2.h lib/checkers.h lib/config.h lib/filesettings.h lib/importproject.h lib/json.h lib/library.h lib/mathlib.h lib/path.h lib/pathmatch.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/utils.h lib/xml.h
619619
$(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/importproject.cpp
620620

621621
$(libcppdir)/infer.o: lib/infer.cpp lib/calculate.h lib/config.h lib/errortypes.h lib/infer.h lib/mathlib.h lib/smallvector.h lib/templatesimplifier.h lib/token.h lib/utils.h lib/valueptr.h lib/vfvalue.h
@@ -819,7 +819,7 @@ test/testfunctions.o: test/testfunctions.cpp lib/check.h lib/checkers.h lib/chec
819819
test/testgarbage.o: test/testgarbage.cpp lib/check.h lib/checkers.h lib/checks.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/settings.h lib/smallvector.h lib/standards.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/vfvalue.h test/fixture.h test/helpers.h
820820
$(CXX) ${INCLUDE_FOR_TEST} ${CFLAGS_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/testgarbage.cpp
821821

822-
test/testimportproject.o: test/testimportproject.cpp externals/tinyxml2/tinyxml2.h lib/check.h lib/checkers.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/filesettings.h lib/importproject.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/utils.h lib/xml.h test/fixture.h test/redirect.h
822+
test/testimportproject.o: test/testimportproject.cpp lib/check.h lib/checkers.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/filesettings.h lib/importproject.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/tokenize.h lib/tokenlist.h lib/utils.h test/fixture.h test/helpers.h test/redirect.h
823823
$(CXX) ${INCLUDE_FOR_TEST} ${CFLAGS_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/testimportproject.cpp
824824

825825
test/testincompletestatement.o: test/testincompletestatement.cpp lib/check.h lib/checkers.h lib/checkimpl.h lib/checkother.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/settings.h lib/standards.h lib/tokenize.h lib/tokenlist.h lib/utils.h test/fixture.h test/helpers.h

‎lib/astutils.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1554,7 +1554,7 @@ bool isUsedAsBool(const Token* const tok, const Settings& settings)
15541554
return true;
15551555
if (parent->isCast())
15561556
return !Token::simpleMatch(parent->astOperand1(), "dynamic_cast") && isUsedAsBool(parent, settings);
1557-
if (Token::Match(parent, "==|!=") && tok->valueType() && tok->valueType()->pointer &&
1557+
if (Token::Match(parent, "==|!=") && ((tok->valueType() && tok->valueType()->pointer) || tok->function()) &&
15581558
tok->astSibling()->hasKnownIntValue() && tok->astSibling()->getKnownIntValue() == 0)
15591559
return true;
15601560
if (parent->str() == "(" && astIsRHS(tok) && Token::Match(parent->astOperand1(), "if|while"))

‎lib/checkother.cpp‎

Lines changed: 107 additions & 102 deletions
Original file line numberDiff line numberDiff line change
@@ -594,6 +594,14 @@ void CheckOtherImpl::invalidPointerCastError(const Token* tok, const std::string
594594
// Detect redundant assignments: x = 0; x = 4;
595595
//---------------------------------------------------------------------------
596596

597+
static bool isAssignmentOrInit(const Token* tok) {
598+
if (tok->astParent() || !tok->astOperand1())
599+
return false;
600+
if (tok->isAssignmentOp() || tok->tokType() == Token::eIncDecOp)
601+
return true;
602+
return Token::Match(tok, "[{(]") && tok->astOperand1()->variable() && tok->astOperand1() == tok->astOperand1()->variable()->nameToken();
603+
}
604+
597605
void CheckOtherImpl::checkRedundantAssignment()
598606
{
599607
if (!mSettings.severity.isEnabled(Severity::style) &&
@@ -614,122 +622,119 @@ void CheckOtherImpl::checkRedundantAssignment()
614622
if (Token::simpleMatch(tok, "try {"))
615623
// todo: check try blocks
616624
tok = tok->linkAt(1);
617-
if ((tok->isAssignmentOp() || tok->tokType() == Token::eIncDecOp) && tok->astOperand1()) {
618-
if (tok->astParent())
619-
continue;
620-
621-
// Do not warn about redundant initialization when rhs is trivial
622-
// TODO : do not simplify the variable declarations
623-
bool isInitialization = false;
624-
if (Token::Match(tok->tokAt(-2), "; %var% =") && tok->tokAt(-2)->isSplittedVarDeclEq()) {
625-
isInitialization = true;
626-
bool trivial = true;
627-
visitAstNodes(tok->astOperand2(),
628-
[&](const Token *rhs) {
629-
if (Token::simpleMatch(rhs, "{ 0 }"))
630-
return ChildrenToVisit::none;
631-
if (Token::Match(rhs, "%num%|%name%") && !rhs->varId())
632-
return ChildrenToVisit::none;
633-
if (Token::Match(rhs, ":: %name%") && rhs->hasKnownIntValue())
634-
return ChildrenToVisit::none;
635-
if (rhs->isCast())
636-
return rhs->astOperand2() ? ChildrenToVisit::op2 : ChildrenToVisit::op1;
637-
trivial = false;
638-
return ChildrenToVisit::done;
639-
});
640-
if (trivial)
641-
continue;
642-
}
643-
644-
const Token* rhs = tok->astOperand2();
645-
// Do not warn about assignment with 0 / NULL
646-
if ((rhs && MathLib::isNullValue(rhs->str())) || isNullOperand(rhs))
647-
continue;
625+
if (!isAssignmentOrInit(tok))
626+
continue;
648627

649-
if (tok->astOperand1()->variable() && tok->astOperand1()->variable()->isReference())
650-
// todo: check references
628+
// Do not warn about redundant initialization when rhs is trivial
629+
// TODO : do not simplify the variable declarations
630+
bool isInitialization = false;
631+
if ((Token::Match(tok->tokAt(-2), "; %var% =") && tok->tokAt(-2)->isSplittedVarDeclEq()) || Token::Match(tok, "[{(]")) {
632+
isInitialization = true;
633+
bool trivial = true;
634+
visitAstNodes(tok->astOperand2(),
635+
[&](const Token *rhs) {
636+
if (Token::simpleMatch(rhs, "{ 0 }"))
637+
return ChildrenToVisit::none;
638+
if (Token::Match(rhs, "%num%|%name%") && !rhs->varId())
639+
return ChildrenToVisit::none;
640+
if (Token::Match(rhs, ":: %name%") && rhs->hasKnownIntValue())
641+
return ChildrenToVisit::none;
642+
if (rhs->isCast())
643+
return rhs->astOperand2() ? ChildrenToVisit::op2 : ChildrenToVisit::op1;
644+
trivial = false;
645+
return ChildrenToVisit::done;
646+
});
647+
if (trivial)
651648
continue;
649+
}
652650

653-
if (tok->astOperand1()->variable() && tok->astOperand1()->variable()->isStatic())
654-
// todo: check static variables
655-
continue;
651+
const Token* rhs = tok->astOperand2();
652+
// Do not warn about assignment with 0 / NULL
653+
if ((rhs && MathLib::isNullValue(rhs->str())) || isNullOperand(rhs))
654+
continue;
656655

657-
bool inconclusive = false;
658-
if (tok->isCpp() && tok->astOperand1()->valueType()) {
659-
// If there is a custom assignment operator => this is inconclusive
660-
if (tok->astOperand1()->valueType()->typeScope) {
661-
const std::string op = "operator" + tok->str();
662-
const std::list<Function>& fList = tok->astOperand1()->valueType()->typeScope->functionList;
663-
inconclusive = std::any_of(fList.cbegin(), fList.cend(), [&](const Function& f) {
664-
return f.name() == op;
665-
});
666-
}
667-
// assigning a smart pointer has side effects
668-
if (tok->astOperand1()->valueType()->type == ValueType::SMART_POINTER)
669-
break;
670-
}
671-
if (inconclusive && !mSettings.certainty.isEnabled(Certainty::inconclusive))
672-
continue;
656+
if (tok->astOperand1()->variable() && tok->astOperand1()->variable()->isReference())
657+
// todo: check references
658+
continue;
673659

674-
FwdAnalysis fwdAnalysis(mSettings);
675-
if (fwdAnalysis.hasOperand(tok->astOperand2(), tok->astOperand1()))
676-
continue;
660+
if (tok->astOperand1()->variable() && tok->astOperand1()->variable()->isStatic())
661+
// todo: check static variables
662+
continue;
677663

678-
// Is there a redundant assignment?
679-
const Token *start;
680-
if (tok->isAssignmentOp())
681-
start = tok->astOperand2();
682-
else
683-
start = tok->findExpressionStartEndTokens().second->next();
664+
bool inconclusive = false;
665+
if (tok->isCpp() && tok->astOperand1()->valueType()) {
666+
// If there is a custom assignment operator => this is inconclusive
667+
if (tok->astOperand1()->valueType()->typeScope) {
668+
const std::string op = "operator" + tok->str();
669+
const std::list<Function>& fList = tok->astOperand1()->valueType()->typeScope->functionList;
670+
inconclusive = std::any_of(fList.cbegin(), fList.cend(), [&](const Function& f) {
671+
return f.name() == op;
672+
});
673+
}
674+
// assigning a smart pointer has side effects
675+
if (tok->astOperand1()->valueType()->type == ValueType::SMART_POINTER)
676+
break;
677+
}
678+
if (inconclusive && !mSettings.certainty.isEnabled(Certainty::inconclusive))
679+
continue;
684680

685-
const Token * tokenToCheck = tok->astOperand1();
681+
FwdAnalysis fwdAnalysis(mSettings);
682+
if (fwdAnalysis.hasOperand(tok->astOperand2(), tok->astOperand1()))
683+
continue;
686684

687-
// Check if we are working with union
688-
for (const Token* tempToken = tokenToCheck; Token::simpleMatch(tempToken, ".");) {
689-
tempToken = tempToken->astOperand1();
690-
if (tempToken && tempToken->variable() && tempToken->variable()->type() && tempToken->variable()->type()->isUnionType())
691-
tokenToCheck = tempToken;
692-
}
685+
// Is there a redundant assignment?
686+
const Token *start;
687+
if (tok->isAssignmentOp())
688+
start = tok->astOperand2();
689+
else
690+
start = tok->findExpressionStartEndTokens().second->next();
691+
const Token * tokenToCheck = tok->astOperand1();
692+
693+
// Check if we are working with union
694+
for (const Token* tempToken = tokenToCheck; Token::simpleMatch(tempToken, ".");) {
695+
tempToken = tempToken->astOperand1();
696+
if (tempToken && tempToken->variable() && tempToken->variable()->type() && tempToken->variable()->type()->isUnionType())
697+
tokenToCheck = tempToken;
698+
}
693699

694-
if (start->hasKnownSymbolicValue(tokenToCheck) && Token::simpleMatch(start->astParent(), "=") && !diag(tok)) {
695-
const ValueFlow::Value* val = start->getKnownValue(ValueFlow::Value::ValueType::SYMBOLIC);
696-
if (val->intvalue == 0) // no offset
697-
redundantAssignmentSameValueError(tokenToCheck, val, tok->astOperand1()->expressionString());
698-
}
700+
if (start->hasKnownSymbolicValue(tokenToCheck) && Token::simpleMatch(start->astParent(), "=") && !diag(tok)) {
701+
const ValueFlow::Value* val = start->getKnownValue(ValueFlow::Value::ValueType::SYMBOLIC);
702+
if (val->intvalue == 0) // no offset
703+
redundantAssignmentSameValueError(tokenToCheck, val, tok->astOperand1()->expressionString());
704+
}
699705

700-
// Get next assignment..
701-
const Token *nextAssign = fwdAnalysis.reassign(tokenToCheck, start, scope->bodyEnd);
702-
// extra check for union
703-
if (nextAssign && tokenToCheck != tok->astOperand1()) {
704-
nextAssign = fwdAnalysis.reassign(tok->astOperand1(), start, scope->bodyEnd);
705-
// reading another member of the same union in the rhs is a use through aliasing
706-
if (nextAssign && fwdAnalysis.hasOperand(nextAssign->astOperand2(), tokenToCheck))
707-
nextAssign = nullptr;
708-
}
706+
// Get next assignment..
707+
const Token *nextAssign = fwdAnalysis.reassign(tokenToCheck, start, scope->bodyEnd);
708+
// extra check for union
709+
if (nextAssign && tokenToCheck != tok->astOperand1()) {
710+
nextAssign = fwdAnalysis.reassign(tok->astOperand1(), start, scope->bodyEnd);
711+
// reading another member of the same union in the rhs is a use through aliasing
712+
if (nextAssign && fwdAnalysis.hasOperand(nextAssign->astOperand2(), tokenToCheck))
713+
nextAssign = nullptr;
714+
}
709715

710-
if (!nextAssign)
711-
continue;
716+
if (!nextAssign)
717+
continue;
712718

713-
// there is redundant assignment. Is there a case between the assignments?
714-
bool hasCase = false;
715-
for (const Token *tok2 = tok; tok2 != nextAssign; tok2 = tok2->next()) {
716-
if (tok2->str() == "break" || tok2->str() == "return")
717-
break;
718-
if (tok2->str() == "case") {
719-
hasCase = true;
720-
break;
721-
}
719+
// there is redundant assignment. Is there a case between the assignments?
720+
bool hasCase = false;
721+
for (const Token *tok2 = tok; tok2 != nextAssign; tok2 = tok2->next()) {
722+
if (tok2->str() == "break" || tok2->str() == "return")
723+
break;
724+
if (tok2->str() == "case") {
725+
hasCase = true;
726+
break;
722727
}
728+
}
723729

724-
// warn
725-
if (hasCase)
726-
redundantAssignmentInSwitchError(tok, nextAssign, tok->astOperand1()->expressionString());
727-
else if (isInitialization)
728-
redundantInitializationError(tok, nextAssign, tok->astOperand1()->expressionString(), inconclusive);
729-
else {
730-
diag(nextAssign);
731-
redundantAssignmentError(tok, nextAssign, tok->astOperand1()->expressionString(), inconclusive);
732-
}
730+
// warn
731+
if (hasCase)
732+
redundantAssignmentInSwitchError(tok, nextAssign, tok->astOperand1()->expressionString());
733+
else if (isInitialization)
734+
redundantInitializationError(tok, nextAssign, tok->astOperand1()->expressionString(), inconclusive);
735+
else {
736+
diag(nextAssign);
737+
redundantAssignmentError(tok, nextAssign, tok->astOperand1()->expressionString(), inconclusive);
733738
}
734739
}
735740
}

‎lib/checkstl.cpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -267,7 +267,8 @@ void CheckStlImpl::outOfBoundsError(const Token *tok, const std::string &contain
267267
}
268268

269269
reportError(std::move(errorPath),
270-
(containerSize && !containerSize->errorSeverity()) || (indexValue && !indexValue->errorSeverity()) ? Severity::warning : Severity::error,
270+
(containerSize && (!containerSize->errorSeverity() || containerSize->conditional)) ||
271+
(indexValue && (!indexValue->errorSeverity() || indexValue->conditional)) ? Severity::warning : Severity::error,
271272
"containerOutOfBounds",
272273
"$symbol:" + containerName +"\n" + errmsg,
273274
CWE398,

0 commit comments

Comments
 (0)