[clang] e517e5c - [clang] improve accuracy of ExprMutAnalyzer
Jonas Toth via cfe-commits
cfe-commits at lists.llvm.org
Fri Oct 9 04:46:03 PDT 2020
Author: Jonas Toth
Date: 2020-10-09T13:45:32+02:00
New Revision: e517e5cfec90a00a488ad8df901ab9b903ebb966
URL: https://github.com/llvm/llvm-project/commit/e517e5cfec90a00a488ad8df901ab9b903ebb966
DIFF: https://github.com/llvm/llvm-project/commit/e517e5cfec90a00a488ad8df901ab9b903ebb966.diff
LOG: [clang] improve accuracy of ExprMutAnalyzer
This patch extracts the ExprMutAnalyzer changes from https://reviews.llvm.org/D54943
into its own revision for simpler review and more atomic changes.
The analysis results are improved. Nested expressions (e.g. conditional
operators) are now detected properly. Some edge cases, especially
template induced imprecisions are improved upon.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D88088
Added:
Modified:
clang/lib/Analysis/ExprMutationAnalyzer.cpp
clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
Removed:
################################################################################
diff --git a/clang/lib/Analysis/ExprMutationAnalyzer.cpp b/clang/lib/Analysis/ExprMutationAnalyzer.cpp
index 2f80285f17b4..c3da5cc47d78 100644
--- a/clang/lib/Analysis/ExprMutationAnalyzer.cpp
+++ b/clang/lib/Analysis/ExprMutationAnalyzer.cpp
@@ -6,7 +6,10 @@
//
//===----------------------------------------------------------------------===//
#include "clang/Analysis/Analyses/ExprMutationAnalyzer.h"
+#include "clang/AST/Expr.h"
+#include "clang/AST/OperationKinds.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
#include "llvm/ADT/STLExtras.h"
namespace clang {
@@ -24,11 +27,11 @@ AST_MATCHER_P(CXXForRangeStmt, hasRangeStmt,
return InnerMatcher.matches(*Range, Finder, Builder);
}
-AST_MATCHER_P(Expr, maybeEvalCommaExpr,
- ast_matchers::internal::Matcher<Expr>, InnerMatcher) {
- const Expr* Result = &Node;
+AST_MATCHER_P(Expr, maybeEvalCommaExpr, ast_matchers::internal::Matcher<Expr>,
+ InnerMatcher) {
+ const Expr *Result = &Node;
while (const auto *BOComma =
- dyn_cast_or_null<BinaryOperator>(Result->IgnoreParens())) {
+ dyn_cast_or_null<BinaryOperator>(Result->IgnoreParens())) {
if (!BOComma->isCommaOp())
break;
Result = BOComma->getRHS();
@@ -36,6 +39,55 @@ AST_MATCHER_P(Expr, maybeEvalCommaExpr,
return InnerMatcher.matches(*Result, Finder, Builder);
}
+AST_MATCHER_P(Expr, canResolveToExpr, ast_matchers::internal::Matcher<Expr>,
+ InnerMatcher) {
+ auto DerivedToBase = [](const ast_matchers::internal::Matcher<Expr> &Inner) {
+ return implicitCastExpr(anyOf(hasCastKind(CK_DerivedToBase),
+ hasCastKind(CK_UncheckedDerivedToBase)),
+ hasSourceExpression(Inner));
+ };
+ auto IgnoreDerivedToBase =
+ [&DerivedToBase](const ast_matchers::internal::Matcher<Expr> &Inner) {
+ return ignoringParens(expr(anyOf(Inner, DerivedToBase(Inner))));
+ };
+
+ // The 'ConditionalOperator' matches on `<anything> ? <expr> : <expr>`.
+ // This matching must be recursive because `<expr>` can be anything resolving
+ // to the `InnerMatcher`, for example another conditional operator.
+ // The edge-case `BaseClass &b = <cond> ? DerivedVar1 : DerivedVar2;`
+ // is handled, too. The implicit cast happens outside of the conditional.
+ // This is matched by `IgnoreDerivedToBase(canResolveToExpr(InnerMatcher))`
+ // below.
+ auto const ConditionalOperator = conditionalOperator(anyOf(
+ hasTrueExpression(ignoringParens(canResolveToExpr(InnerMatcher))),
+ hasFalseExpression(ignoringParens(canResolveToExpr(InnerMatcher)))));
+ auto const ElvisOperator = binaryConditionalOperator(anyOf(
+ hasTrueExpression(ignoringParens(canResolveToExpr(InnerMatcher))),
+ hasFalseExpression(ignoringParens(canResolveToExpr(InnerMatcher)))));
+
+ auto const ComplexMatcher = ignoringParens(
+ expr(anyOf(IgnoreDerivedToBase(InnerMatcher),
+ maybeEvalCommaExpr(IgnoreDerivedToBase(InnerMatcher)),
+ IgnoreDerivedToBase(ConditionalOperator),
+ IgnoreDerivedToBase(ElvisOperator))));
+
+ return ComplexMatcher.matches(Node, Finder, Builder);
+}
+
+// Similar to 'hasAnyArgument', but does not work because 'InitListExpr' does
+// not have the 'arguments()' method.
+AST_MATCHER_P(InitListExpr, hasAnyInit, ast_matchers::internal::Matcher<Expr>,
+ InnerMatcher) {
+ for (const Expr *Arg : Node.inits()) {
+ ast_matchers::internal::BoundNodesTreeBuilder Result(*Builder);
+ if (InnerMatcher.matches(*Arg, Finder, &Result)) {
+ *Builder = std::move(Result);
+ return true;
+ }
+ }
+ return false;
+}
+
const ast_matchers::internal::VariadicDynCastAllOfMatcher<Stmt, CXXTypeidExpr>
cxxTypeidExpr;
@@ -151,7 +203,7 @@ bool ExprMutationAnalyzer::isUnevaluated(const Expr *Exp) {
NodeID<Expr>::value,
match(
findAll(
- expr(equalsNode(Exp),
+ expr(canResolveToExpr(equalsNode(Exp)),
anyOf(
// `Exp` is part of the underlying expression of
// decltype/typeof if it has an ancestor of
@@ -202,29 +254,43 @@ const Stmt *ExprMutationAnalyzer::findDeclPointeeMutation(
const Stmt *ExprMutationAnalyzer::findDirectMutation(const Expr *Exp) {
// LHS of any assignment operators.
const auto AsAssignmentLhs = binaryOperator(
- isAssignmentOperator(),
- hasLHS(maybeEvalCommaExpr(ignoringParenImpCasts(equalsNode(Exp)))));
+ isAssignmentOperator(), hasLHS(canResolveToExpr(equalsNode(Exp))));
// Operand of increment/decrement operators.
const auto AsIncDecOperand =
unaryOperator(anyOf(hasOperatorName("++"), hasOperatorName("--")),
- hasUnaryOperand(maybeEvalCommaExpr(
- ignoringParenImpCasts(equalsNode(Exp)))));
+ hasUnaryOperand(canResolveToExpr(equalsNode(Exp))));
// Invoking non-const member function.
// A member function is assumed to be non-const when it is unresolved.
const auto NonConstMethod = cxxMethodDecl(unless(isConst()));
- const auto AsNonConstThis =
- expr(anyOf(cxxMemberCallExpr(callee(NonConstMethod),
- on(maybeEvalCommaExpr(equalsNode(Exp)))),
- cxxOperatorCallExpr(callee(NonConstMethod),
- hasArgument(0,
- maybeEvalCommaExpr(equalsNode(Exp)))),
- callExpr(callee(expr(anyOf(
- unresolvedMemberExpr(
- hasObjectExpression(maybeEvalCommaExpr(equalsNode(Exp)))),
- cxxDependentScopeMemberExpr(
- hasObjectExpression(maybeEvalCommaExpr(equalsNode(Exp))))))))));
+
+ const auto AsNonConstThis = expr(anyOf(
+ cxxMemberCallExpr(callee(NonConstMethod),
+ on(canResolveToExpr(equalsNode(Exp)))),
+ cxxOperatorCallExpr(callee(NonConstMethod),
+ hasArgument(0, canResolveToExpr(equalsNode(Exp)))),
+ // In case of a templated type, calling overloaded operators is not
+ // resolved and modelled as `binaryOperator` on a dependent type.
+ // Such instances are considered a modification, because they can modify
+ // in
diff erent instantiations of the template.
+ binaryOperator(hasEitherOperand(
+ allOf(ignoringImpCasts(canResolveToExpr(equalsNode(Exp))),
+ isTypeDependent()))),
+ // Within class templates and member functions the member expression might
+ // not be resolved. In that case, the `callExpr` is considered to be a
+ // modification.
+ callExpr(
+ callee(expr(anyOf(unresolvedMemberExpr(hasObjectExpression(
+ canResolveToExpr(equalsNode(Exp)))),
+ cxxDependentScopeMemberExpr(hasObjectExpression(
+ canResolveToExpr(equalsNode(Exp)))))))),
+ // Match on a call to a known method, but the call itself is type
+ // dependent (e.g. `vector<T> v; v.push(T{});` in a templated function).
+ callExpr(allOf(isTypeDependent(),
+ callee(memberExpr(hasDeclaration(NonConstMethod),
+ hasObjectExpression(canResolveToExpr(
+ equalsNode(Exp)))))))));
// Taking address of 'Exp'.
// We're assuming 'Exp' is mutated as soon as its address is taken, though in
@@ -234,38 +300,51 @@ const Stmt *ExprMutationAnalyzer::findDirectMutation(const Expr *Exp) {
unaryOperator(hasOperatorName("&"),
// A NoOp implicit cast is adding const.
unless(hasParent(implicitCastExpr(hasCastKind(CK_NoOp)))),
- hasUnaryOperand(maybeEvalCommaExpr(equalsNode(Exp))));
+ hasUnaryOperand(canResolveToExpr(equalsNode(Exp))));
const auto AsPointerFromArrayDecay =
castExpr(hasCastKind(CK_ArrayToPointerDecay),
unless(hasParent(arraySubscriptExpr())),
- has(maybeEvalCommaExpr(equalsNode(Exp))));
+ has(canResolveToExpr(equalsNode(Exp))));
// Treat calling `operator->()` of move-only classes as taking address.
// These are typically smart pointers with unique ownership so we treat
// mutation of pointee as mutation of the smart pointer itself.
- const auto AsOperatorArrowThis =
- cxxOperatorCallExpr(hasOverloadedOperatorName("->"),
- callee(cxxMethodDecl(ofClass(isMoveOnly()),
- returns(nonConstPointerType()))),
- argumentCountIs(1),
- hasArgument(0, maybeEvalCommaExpr(equalsNode(Exp))));
+ const auto AsOperatorArrowThis = cxxOperatorCallExpr(
+ hasOverloadedOperatorName("->"),
+ callee(
+ cxxMethodDecl(ofClass(isMoveOnly()), returns(nonConstPointerType()))),
+ argumentCountIs(1), hasArgument(0, canResolveToExpr(equalsNode(Exp))));
// Used as non-const-ref argument when calling a function.
// An argument is assumed to be non-const-ref when the function is unresolved.
// Instantiated template functions are not handled here but in
// findFunctionArgMutation which has additional smarts for handling forwarding
// references.
- const auto NonConstRefParam = forEachArgumentWithParam(
- maybeEvalCommaExpr(equalsNode(Exp)),
- parmVarDecl(hasType(nonConstReferenceType())));
+ const auto NonConstRefParam = forEachArgumentWithParamType(
+ anyOf(canResolveToExpr(equalsNode(Exp)),
+ memberExpr(hasObjectExpression(canResolveToExpr(equalsNode(Exp))))),
+ nonConstReferenceType());
const auto NotInstantiated = unless(hasDeclaration(isInstantiated()));
+ const auto TypeDependentCallee =
+ callee(expr(anyOf(unresolvedLookupExpr(), unresolvedMemberExpr(),
+ cxxDependentScopeMemberExpr(),
+ hasType(templateTypeParmType()), isTypeDependent())));
+
const auto AsNonConstRefArg = anyOf(
callExpr(NonConstRefParam, NotInstantiated),
cxxConstructExpr(NonConstRefParam, NotInstantiated),
- callExpr(callee(expr(anyOf(unresolvedLookupExpr(), unresolvedMemberExpr(),
- cxxDependentScopeMemberExpr(),
- hasType(templateTypeParmType())))),
- hasAnyArgument(maybeEvalCommaExpr(equalsNode(Exp)))),
- cxxUnresolvedConstructExpr(hasAnyArgument(maybeEvalCommaExpr(equalsNode(Exp)))));
+ callExpr(TypeDependentCallee,
+ hasAnyArgument(canResolveToExpr(equalsNode(Exp)))),
+ cxxUnresolvedConstructExpr(
+ hasAnyArgument(canResolveToExpr(equalsNode(Exp)))),
+ // Previous False Positive in the following Code:
+ // `template <typename T> void f() { int i = 42; new Type<T>(i); }`
+ // Where the constructor of `Type` takes its argument as reference.
+ // The AST does not resolve in a `cxxConstructExpr` because it is
+ // type-dependent.
+ parenListExpr(hasDescendant(expr(canResolveToExpr(equalsNode(Exp))))),
+ // If the initializer is for a reference type, there is no cast for
+ // the variable. Values are cast to RValue first.
+ initListExpr(hasAnyInit(expr(canResolveToExpr(equalsNode(Exp))))));
// Captured by a lambda by reference.
// If we're initializing a capture with 'Exp' directly then we're initializing
@@ -278,17 +357,22 @@ const Stmt *ExprMutationAnalyzer::findDirectMutation(const Expr *Exp) {
// For returning by value there will be an ImplicitCastExpr <LValueToRValue>.
// For returning by const-ref there will be an ImplicitCastExpr <NoOp> (for
// adding const.)
- const auto AsNonConstRefReturn = returnStmt(hasReturnValue(
- maybeEvalCommaExpr(equalsNode(Exp))));
+ const auto AsNonConstRefReturn =
+ returnStmt(hasReturnValue(canResolveToExpr(equalsNode(Exp))));
+
+ // It is used as a non-const-reference for initalizing a range-for loop.
+ const auto AsNonConstRefRangeInit = cxxForRangeStmt(
+ hasRangeInit(declRefExpr(allOf(canResolveToExpr(equalsNode(Exp)),
+ hasType(nonConstReferenceType())))));
const auto Matches = match(
- traverse(
- ast_type_traits::TK_AsIs,
- findAll(stmt(anyOf(AsAssignmentLhs, AsIncDecOperand, AsNonConstThis,
- AsAmpersandOperand, AsPointerFromArrayDecay,
- AsOperatorArrowThis, AsNonConstRefArg,
- AsLambdaRefCaptureInit, AsNonConstRefReturn))
- .bind("stmt"))),
+ traverse(ast_type_traits::TK_AsIs,
+ findAll(stmt(anyOf(AsAssignmentLhs, AsIncDecOperand,
+ AsNonConstThis, AsAmpersandOperand,
+ AsPointerFromArrayDecay, AsOperatorArrowThis,
+ AsNonConstRefArg, AsLambdaRefCaptureInit,
+ AsNonConstRefReturn, AsNonConstRefRangeInit))
+ .bind("stmt"))),
Stm, Context);
return selectFirst<Stmt>("stmt", Matches);
}
@@ -296,9 +380,10 @@ const Stmt *ExprMutationAnalyzer::findDirectMutation(const Expr *Exp) {
const Stmt *ExprMutationAnalyzer::findMemberMutation(const Expr *Exp) {
// Check whether any member of 'Exp' is mutated.
const auto MemberExprs =
- match(findAll(expr(anyOf(memberExpr(hasObjectExpression(equalsNode(Exp))),
- cxxDependentScopeMemberExpr(
- hasObjectExpression(equalsNode(Exp)))))
+ match(findAll(expr(anyOf(memberExpr(hasObjectExpression(
+ canResolveToExpr(equalsNode(Exp)))),
+ cxxDependentScopeMemberExpr(hasObjectExpression(
+ canResolveToExpr(equalsNode(Exp))))))
.bind(NodeID<Expr>::value)),
Stm, Context);
return findExprMutation(MemberExprs);
@@ -306,43 +391,112 @@ const Stmt *ExprMutationAnalyzer::findMemberMutation(const Expr *Exp) {
const Stmt *ExprMutationAnalyzer::findArrayElementMutation(const Expr *Exp) {
// Check whether any element of an array is mutated.
- const auto SubscriptExprs = match(
- findAll(arraySubscriptExpr(hasBase(ignoringImpCasts(equalsNode(Exp))))
- .bind(NodeID<Expr>::value)),
- Stm, Context);
+ const auto SubscriptExprs =
+ match(findAll(arraySubscriptExpr(
+ anyOf(hasBase(canResolveToExpr(equalsNode(Exp))),
+ hasBase(implicitCastExpr(
+ allOf(hasCastKind(CK_ArrayToPointerDecay),
+ hasSourceExpression(canResolveToExpr(
+ equalsNode(Exp))))))))
+ .bind(NodeID<Expr>::value)),
+ Stm, Context);
return findExprMutation(SubscriptExprs);
}
const Stmt *ExprMutationAnalyzer::findCastMutation(const Expr *Exp) {
+ // If the 'Exp' is explicitly casted to a non-const reference type the
+ // 'Exp' is considered to be modified.
+ const auto ExplicitCast = match(
+ findAll(
+ stmt(castExpr(hasSourceExpression(canResolveToExpr(equalsNode(Exp))),
+ explicitCastExpr(
+ hasDestinationType(nonConstReferenceType()))))
+ .bind("stmt")),
+ Stm, Context);
+
+ if (const auto *CastStmt = selectFirst<Stmt>("stmt", ExplicitCast))
+ return CastStmt;
+
// If 'Exp' is casted to any non-const reference type, check the castExpr.
- const auto Casts =
- match(findAll(castExpr(hasSourceExpression(equalsNode(Exp)),
- anyOf(explicitCastExpr(hasDestinationType(
- nonConstReferenceType())),
- implicitCastExpr(hasImplicitDestinationType(
- nonConstReferenceType()))))
- .bind(NodeID<Expr>::value)),
- Stm, Context);
+ const auto Casts = match(
+ findAll(
+ expr(castExpr(hasSourceExpression(canResolveToExpr(equalsNode(Exp))),
+ anyOf(explicitCastExpr(
+ hasDestinationType(nonConstReferenceType())),
+ implicitCastExpr(hasImplicitDestinationType(
+ nonConstReferenceType())))))
+ .bind(NodeID<Expr>::value)),
+ Stm, Context);
+
if (const Stmt *S = findExprMutation(Casts))
return S;
// Treat std::{move,forward} as cast.
const auto Calls =
match(findAll(callExpr(callee(namedDecl(
hasAnyName("::std::move", "::std::forward"))),
- hasArgument(0, equalsNode(Exp)))
+ hasArgument(0, canResolveToExpr(equalsNode(Exp))))
.bind("expr")),
Stm, Context);
return findExprMutation(Calls);
}
const Stmt *ExprMutationAnalyzer::findRangeLoopMutation(const Expr *Exp) {
+ // Keep the ordering for the specific initialization matches to happen first,
+ // because it is cheaper to match all potential modifications of the loop
+ // variable.
+
+ // The range variable is a reference to a builtin array. In that case the
+ // array is considered modified if the loop-variable is a non-const reference.
+ const auto DeclStmtToNonRefToArray = declStmt(hasSingleDecl(varDecl(hasType(
+ hasUnqualifiedDesugaredType(referenceType(pointee(arrayType())))))));
+ const auto RefToArrayRefToElements = match(
+ findAll(stmt(cxxForRangeStmt(
+ hasLoopVariable(varDecl(hasType(nonConstReferenceType()))
+ .bind(NodeID<Decl>::value)),
+ hasRangeStmt(DeclStmtToNonRefToArray),
+ hasRangeInit(canResolveToExpr(equalsNode(Exp)))))
+ .bind("stmt")),
+ Stm, Context);
+
+ if (const auto *BadRangeInitFromArray =
+ selectFirst<Stmt>("stmt", RefToArrayRefToElements))
+ return BadRangeInitFromArray;
+
+ // Small helper to match special cases in range-for loops.
+ //
+ // It is possible that containers do not provide a const-overload for their
+ // iterator accessors. If this is the case, the variable is used non-const
+ // no matter what happens in the loop. This requires special detection as it
+ // is then faster to find all mutations of the loop variable.
+ // It aims at a
diff erent modification as well.
+ const auto HasAnyNonConstIterator =
+ anyOf(allOf(hasMethod(allOf(hasName("begin"), unless(isConst()))),
+ unless(hasMethod(allOf(hasName("begin"), isConst())))),
+ allOf(hasMethod(allOf(hasName("end"), unless(isConst()))),
+ unless(hasMethod(allOf(hasName("end"), isConst())))));
+
+ const auto DeclStmtToNonConstIteratorContainer = declStmt(
+ hasSingleDecl(varDecl(hasType(hasUnqualifiedDesugaredType(referenceType(
+ pointee(hasDeclaration(cxxRecordDecl(HasAnyNonConstIterator)))))))));
+
+ const auto RefToContainerBadIterators =
+ match(findAll(stmt(cxxForRangeStmt(allOf(
+ hasRangeStmt(DeclStmtToNonConstIteratorContainer),
+ hasRangeInit(canResolveToExpr(equalsNode(Exp))))))
+ .bind("stmt")),
+ Stm, Context);
+
+ if (const auto *BadIteratorsContainer =
+ selectFirst<Stmt>("stmt", RefToContainerBadIterators))
+ return BadIteratorsContainer;
+
// If range for looping over 'Exp' with a non-const reference loop variable,
// check all declRefExpr of the loop variable.
const auto LoopVars =
match(findAll(cxxForRangeStmt(
hasLoopVariable(varDecl(hasType(nonConstReferenceType()))
.bind(NodeID<Decl>::value)),
- hasRangeInit(equalsNode(Exp)))),
+ hasRangeInit(canResolveToExpr(equalsNode(Exp))))),
Stm, Context);
return findDeclMutation(LoopVars);
}
@@ -356,7 +510,8 @@ const Stmt *ExprMutationAnalyzer::findReferenceMutation(const Expr *Exp) {
hasOverloadedOperatorName("*"),
callee(cxxMethodDecl(ofClass(isMoveOnly()),
returns(nonConstReferenceType()))),
- argumentCountIs(1), hasArgument(0, equalsNode(Exp)))
+ argumentCountIs(1),
+ hasArgument(0, canResolveToExpr(equalsNode(Exp))))
.bind(NodeID<Expr>::value)),
Stm, Context);
if (const Stmt *S = findExprMutation(Ref))
@@ -367,13 +522,12 @@ const Stmt *ExprMutationAnalyzer::findReferenceMutation(const Expr *Exp) {
stmt(forEachDescendant(
varDecl(
hasType(nonConstReferenceType()),
- hasInitializer(anyOf(equalsNode(Exp),
- conditionalOperator(anyOf(
- hasTrueExpression(equalsNode(Exp)),
- hasFalseExpression(equalsNode(Exp)))))),
+ hasInitializer(anyOf(canResolveToExpr(equalsNode(Exp)),
+ memberExpr(hasObjectExpression(
+ canResolveToExpr(equalsNode(Exp)))))),
hasParent(declStmt().bind("stmt")),
- // Don't follow the reference in range statement, we've handled
- // that separately.
+ // Don't follow the reference in range statement, we've
+ // handled that separately.
unless(hasParent(declStmt(hasParent(
cxxForRangeStmt(hasRangeStmt(equalsBoundNode("stmt"))))))))
.bind(NodeID<Decl>::value))),
@@ -383,7 +537,7 @@ const Stmt *ExprMutationAnalyzer::findReferenceMutation(const Expr *Exp) {
const Stmt *ExprMutationAnalyzer::findFunctionArgMutation(const Expr *Exp) {
const auto NonConstRefParam = forEachArgumentWithParam(
- equalsNode(Exp),
+ canResolveToExpr(equalsNode(Exp)),
parmVarDecl(hasType(nonConstReferenceType())).bind("parm"));
const auto IsInstantiated = hasDeclaration(isInstantiated());
const auto FuncDecl = hasDeclaration(functionDecl().bind("func"));
diff --git a/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp b/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
index 9d26eeb6af73..31cb7ed33dab 100644
--- a/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
+++ b/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
@@ -7,6 +7,7 @@
//===----------------------------------------------------------------------===//
#include "clang/Analysis/Analyses/ExprMutationAnalyzer.h"
+#include "clang/AST/TypeLoc.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/ASTMatchers/ASTMatchers.h"
#include "clang/Tooling/Tooling.h"
@@ -19,9 +20,7 @@ namespace clang {
using namespace clang::ast_matchers;
using ::testing::ElementsAre;
-using ::testing::IsEmpty;
using ::testing::ResultOf;
-using ::testing::StartsWith;
using ::testing::Values;
namespace {
@@ -63,12 +62,16 @@ mutatedBy(const SmallVectorImpl<BoundNodes> &Results, ASTUnit *AST) {
const auto *const S = selectFirst<Stmt>("stmt", Results);
SmallVector<std::string, 1> Chain;
ExprMutationAnalyzer Analyzer(*S, AST->getASTContext());
+
for (const auto *E = selectFirst<Expr>("expr", Results); E != nullptr;) {
const Stmt *By = Analyzer.findMutation(E);
- std::string buffer;
- llvm::raw_string_ostream stream(buffer);
- By->printPretty(stream, nullptr, AST->getASTContext().getPrintingPolicy());
- Chain.push_back(StringRef(stream.str()).trim().str());
+ if (!By)
+ break;
+
+ std::string Buffer;
+ llvm::raw_string_ostream Stream(Buffer);
+ By->printPretty(Stream, nullptr, AST->getASTContext().getPrintingPolicy());
+ Chain.emplace_back(StringRef(Stream.str()).trim().str());
E = dyn_cast<DeclRefExpr>(By);
}
return Chain;
@@ -111,7 +114,13 @@ TEST(ExprMutationAnalyzerTest, Trivial) {
class AssignmentTest : public ::testing::TestWithParam<std::string> {};
+// This test is for the most basic and direct modification of a variable,
+// assignment to it (e.g. `x = 10;`).
+// It additionally tests that references to a variable are not only captured
+// directly but expressions that result in the variable are handled, too.
+// This includes the comma operator, parens and the ternary operator.
TEST_P(AssignmentTest, AssignmentModifies) {
+ // Test the detection of the raw expression modifications.
{
const std::string ModExpr = "x " + GetParam() + " 10";
const auto AST = buildASTFromCode("void f() { int x; " + ModExpr + "; }");
@@ -120,6 +129,7 @@ TEST_P(AssignmentTest, AssignmentModifies) {
EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre(ModExpr));
}
+ // Test the detection if the expression is surrounded by parens.
{
const std::string ModExpr = "(x) " + GetParam() + " 10";
const auto AST = buildASTFromCode("void f() { int x; " + ModExpr + "; }");
@@ -127,12 +137,89 @@ TEST_P(AssignmentTest, AssignmentModifies) {
match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre(ModExpr));
}
+
+ // Test the detection if the comma operator yields the expression as result.
+ {
+ const std::string ModExpr = "x " + GetParam() + " 10";
+ const auto AST = buildASTFromCodeWithArgs(
+ "void f() { int x, y; y, " + ModExpr + "; }", {"-Wno-unused-value"});
+ const auto Results =
+ match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+ EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre(ModExpr));
+ }
+
+ // Ensure no detection if the comma operator does not yield the expression as
+ // result.
+ {
+ const std::string ModExpr = "y, x, y " + GetParam() + " 10";
+ const auto AST = buildASTFromCodeWithArgs(
+ "void f() { int x, y; " + ModExpr + "; }", {"-Wno-unused-value"});
+ const auto Results =
+ match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+ EXPECT_FALSE(isMutated(Results, AST.get()));
+ }
+
+ // Test the detection if the a ternary operator can result in the expression.
+ {
+ const std::string ModExpr = "(y != 0 ? y : x) " + GetParam() + " 10";
+ const auto AST =
+ buildASTFromCode("void f() { int y = 0, x; " + ModExpr + "; }");
+ const auto Results =
+ match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+ EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre(ModExpr));
+ }
+
+ // Test the detection if the a ternary operator can result in the expression
+ // through multiple nesting of ternary operators.
+ {
+ const std::string ModExpr =
+ "(y != 0 ? (y > 5 ? y : x) : (y)) " + GetParam() + " 10";
+ const auto AST =
+ buildASTFromCode("void f() { int y = 0, x; " + ModExpr + "; }");
+ const auto Results =
+ match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+ EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre(ModExpr));
+ }
+
+ // Test the detection if the a ternary operator can result in the expression
+ // with additional parens.
+ {
+ const std::string ModExpr = "(y != 0 ? (y) : ((x))) " + GetParam() + " 10";
+ const auto AST =
+ buildASTFromCode("void f() { int y = 0, x; " + ModExpr + "; }");
+ const auto Results =
+ match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+ EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre(ModExpr));
+ }
+
+ // Test the detection for the binary conditional operator.
+ {
+ const std::string ModExpr = "(y ?: x) " + GetParam() + " 10";
+ const auto AST =
+ buildASTFromCode("void f() { int y = 0, x; " + ModExpr + "; }");
+ const auto Results =
+ match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+ EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre(ModExpr));
+ }
}
INSTANTIATE_TEST_CASE_P(AllAssignmentOperators, AssignmentTest,
Values("=", "+=", "-=", "*=", "/=", "%=", "&=", "|=",
"^=", "<<=", ">>="), );
+TEST(ExprMutationAnalyzerTest, AssignmentConditionalWithInheritance) {
+ const auto AST = buildASTFromCode("struct Base {void nonconst(); };"
+ "struct Derived : Base {};"
+ "static void f() {"
+ " Derived x, y;"
+ " Base &b = true ? x : y;"
+ " b.nonconst();"
+ "}");
+ const auto Results =
+ match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+ EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("b", "b.nonconst()"));
+}
+
class IncDecTest : public ::testing::TestWithParam<std::string> {};
TEST_P(IncDecTest, IncDecModifies) {
@@ -147,6 +234,8 @@ INSTANTIATE_TEST_CASE_P(AllIncDecOperators, IncDecTest,
Values("++x", "--x", "x++", "x--", "++(x)", "--(x)",
"(x)++", "(x)--"), );
+// Section: member functions
+
TEST(ExprMutationAnalyzerTest, NonConstMemberFunc) {
const auto AST = buildASTFromCode(
"void f() { struct Foo { void mf(); }; Foo x; x.mf(); }");
@@ -185,6 +274,18 @@ TEST(ExprMutationAnalyzerTest, ConstMemberFunc) {
EXPECT_FALSE(isMutated(Results, AST.get()));
}
+TEST(ExprMutationAnalyzerTest, TypeDependentMemberCall) {
+ const auto AST = buildASTFromCodeWithArgs(
+ "template <class T> class vector { void push_back(T); }; "
+ "template <class T> void f() { vector<T> x; x.push_back(T()); }",
+ {"-fno-delayed-template-parsing"});
+ const auto Results =
+ match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+ EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x.push_back(T())"));
+}
+
+// Section: overloaded operators
+
TEST(ExprMutationAnalyzerTest, NonConstOperator) {
const auto AST = buildASTFromCode(
"void f() { struct Foo { Foo& operator=(int); }; Foo x; x = 10; }");
@@ -201,6 +302,19 @@ TEST(ExprMutationAnalyzerTest, ConstOperator) {
EXPECT_FALSE(isMutated(Results, AST.get()));
}
+TEST(ExprMutationAnalyzerTest, UnresolvedOperator) {
+ const auto AST = buildASTFromCodeWithArgs(
+ "template <typename Stream> void input_operator_template() {"
+ "Stream x; unsigned y = 42;"
+ "x >> y; }",
+ {"-fno-delayed-template-parsing"});
+ const auto Results =
+ match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+ EXPECT_TRUE(isMutated(Results, AST.get()));
+}
+
+// Section: expression as call argument
+
TEST(ExprMutationAnalyzerTest, ByValueArgument) {
auto AST = buildASTFromCode("void g(int); void f() { int x; g(x); }");
auto Results =
@@ -322,6 +436,22 @@ TEST(ExprMutationAnalyzerTest, ByNonConstRefArgument) {
EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("A<0>::mf(x)"));
}
+TEST(ExprMutationAnalyzerTest, ByNonConstRefArgumentFunctionTypeDependent) {
+ auto AST = buildASTFromCodeWithArgs(
+ "enum MyEnum { foo, bar };"
+ "void tryParser(unsigned& first, MyEnum Type) { first++, (void)Type; }"
+ "template <MyEnum Type> void parse() {"
+ " auto parser = [](unsigned& first) { first++; tryParser(first, Type); "
+ "};"
+ " unsigned x = 42;"
+ " parser(x);"
+ "}",
+ {"-fno-delayed-template-parsing"});
+ auto Results =
+ match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+ EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("parser(x)"));
+}
+
TEST(ExprMutationAnalyzerTest, ByConstRefArgument) {
auto AST = buildASTFromCode("void g(const int&); void f() { int x; g(x); }");
auto Results =
@@ -394,24 +524,30 @@ TEST(ExprMutationAnalyzerTest, ByConstRRefArgument) {
"void g(const int&&); void f() { int x; g(static_cast<int&&>(x)); }");
auto Results =
match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
- EXPECT_FALSE(isMutated(Results, AST.get()));
+ EXPECT_THAT(mutatedBy(Results, AST.get()),
+ ElementsAre("static_cast<int &&>(x)"));
AST = buildASTFromCode("struct A {}; A operator+(const A&&, int);"
"void f() { A x; static_cast<A&&>(x) + 1; }");
Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
- EXPECT_FALSE(isMutated(Results, AST.get()));
+ EXPECT_THAT(mutatedBy(Results, AST.get()),
+ ElementsAre("static_cast<A &&>(x)"));
AST = buildASTFromCode("void f() { struct A { A(const int&&); }; "
"int x; A y(static_cast<int&&>(x)); }");
Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
- EXPECT_FALSE(isMutated(Results, AST.get()));
+ EXPECT_THAT(mutatedBy(Results, AST.get()),
+ ElementsAre("static_cast<int &&>(x)"));
AST = buildASTFromCode("void f() { struct A { A(); A(const A&&); }; "
"A x; A y(static_cast<A&&>(x)); }");
Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
- EXPECT_FALSE(isMutated(Results, AST.get()));
+ EXPECT_THAT(mutatedBy(Results, AST.get()),
+ ElementsAre("static_cast<A &&>(x)"));
}
+// section: explicit std::move and std::forward testing
+
TEST(ExprMutationAnalyzerTest, Move) {
auto AST = buildASTFromCode(StdRemoveReference + StdMove +
"void f() { struct A {}; A x; std::move(x); }");
@@ -490,6 +626,9 @@ TEST(ExprMutationAnalyzerTest, Forward) {
ElementsAre("std::forward<A &>(x) = y"));
}
+// section: template constellations that prohibit reasoning about modifications
+// as it depends on instantiations.
+
TEST(ExprMutationAnalyzerTest, CallUnresolved) {
auto AST =
buildASTFromCodeWithArgs("template <class T> void f() { T x; g(x); }",
@@ -543,6 +682,8 @@ TEST(ExprMutationAnalyzerTest, CallUnresolved) {
EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("T(x)"));
}
+// section: return values
+
TEST(ExprMutationAnalyzerTest, ReturnAsValue) {
auto AST = buildASTFromCode("int f() { int x; return x; }");
auto Results =
@@ -579,7 +720,7 @@ TEST(ExprMutationAnalyzerTest, ReturnAsNonConstRRef) {
const auto Results =
match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
EXPECT_THAT(mutatedBy(Results, AST.get()),
- ElementsAre("return static_cast<int &&>(x);"));
+ ElementsAre("static_cast<int &&>(x)"));
}
TEST(ExprMutationAnalyzerTest, ReturnAsConstRRef) {
@@ -587,9 +728,12 @@ TEST(ExprMutationAnalyzerTest, ReturnAsConstRRef) {
"const int&& f() { int x; return static_cast<int&&>(x); }");
const auto Results =
match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
- EXPECT_FALSE(isMutated(Results, AST.get()));
+ EXPECT_THAT(mutatedBy(Results, AST.get()),
+ ElementsAre("static_cast<int &&>(x)"));
}
+// section: taking the address of a variable and pointers
+
TEST(ExprMutationAnalyzerTest, TakeAddress) {
const auto AST = buildASTFromCode("void g(int*); void f() { int x; g(&x); }");
const auto Results =
@@ -621,6 +765,9 @@ TEST(ExprMutationAnalyzerTest, TemplateWithArrayToPointerDecay) {
EXPECT_THAT(mutatedBy(ResultsY, AST.get()), ElementsAre("y"));
}
+// section: special case: all created references are non-mutating themself
+// and therefore all become 'const'/the value is not modified!
+
TEST(ExprMutationAnalyzerTest, FollowRefModified) {
auto AST = buildASTFromCode(
"void f() { int x; int& r0 = x; int& r1 = r0; int& r2 = r1; "
@@ -792,6 +939,8 @@ TEST(ExprMutationAnalyzerTest, FollowFuncArgNotModified) {
EXPECT_FALSE(isMutated(Results, AST.get()));
}
+// section: builtin arrays
+
TEST(ExprMutationAnalyzerTest, ArrayElementModified) {
const auto AST = buildASTFromCode("void f() { int x[2]; x[0] = 10; }");
const auto Results =
@@ -806,6 +955,8 @@ TEST(ExprMutationAnalyzerTest, ArrayElementNotModified) {
EXPECT_FALSE(isMutated(Results, AST.get()));
}
+// section: member modifications
+
TEST(ExprMutationAnalyzerTest, NestedMemberModified) {
auto AST =
buildASTFromCode("void f() { struct A { int vi; }; struct B { A va; }; "
@@ -849,6 +1000,8 @@ TEST(ExprMutationAnalyzerTest, NestedMemberNotModified) {
EXPECT_FALSE(isMutated(Results, AST.get()));
}
+// section: casts
+
TEST(ExprMutationAnalyzerTest, CastToValue) {
const auto AST =
buildASTFromCode("void f() { int x; static_cast<double>(x); }");
@@ -863,13 +1016,13 @@ TEST(ExprMutationAnalyzerTest, CastToRefModified) {
auto Results =
match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
EXPECT_THAT(mutatedBy(Results, AST.get()),
- ElementsAre("static_cast<int &>(x) = 10"));
+ ElementsAre("static_cast<int &>(x)"));
AST = buildASTFromCode("typedef int& IntRef;"
"void f() { int x; static_cast<IntRef>(x) = 10; }");
Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
EXPECT_THAT(mutatedBy(Results, AST.get()),
- ElementsAre("static_cast<IntRef>(x) = 10"));
+ ElementsAre("static_cast<IntRef>(x)"));
}
TEST(ExprMutationAnalyzerTest, CastToRefNotModified) {
@@ -877,7 +1030,8 @@ TEST(ExprMutationAnalyzerTest, CastToRefNotModified) {
buildASTFromCode("void f() { int x; static_cast<int&>(x); }");
const auto Results =
match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
- EXPECT_FALSE(isMutated(Results, AST.get()));
+ EXPECT_THAT(mutatedBy(Results, AST.get()),
+ ElementsAre("static_cast<int &>(x)"));
}
TEST(ExprMutationAnalyzerTest, CastToConstRef) {
@@ -893,6 +1047,8 @@ TEST(ExprMutationAnalyzerTest, CastToConstRef) {
EXPECT_FALSE(isMutated(Results, AST.get()));
}
+// section: comma expressions
+
TEST(ExprMutationAnalyzerTest, CommaExprWithAnAssigment) {
const auto AST = buildASTFromCodeWithArgs(
"void f() { int x; int y; (x, y) = 5; }", {"-Wno-unused-value"});
@@ -1019,6 +1175,18 @@ TEST(ExprMutationAnalyzerTest, CommaExprAsUniquePtr) {
EXPECT_TRUE(isMutated(Results, AST.get()));
}
+TEST(ExprMutationAnalyzerTest, CommaNestedConditional) {
+ const std::string Code = "void f() { int x, y = 42;"
+ " y, (true ? x : y) = 42; }";
+ const auto AST = buildASTFromCodeWithArgs(Code, {"-Wno-unused-value"});
+ const auto Results =
+ match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+ EXPECT_THAT(mutatedBy(Results, AST.get()),
+ ElementsAre("(true ? x : y) = 42"));
+}
+
+// section: lambda captures
+
TEST(ExprMutationAnalyzerTest, LambdaDefaultCaptureByValue) {
const auto AST = buildASTFromCode("void f() { int x; [=]() { x; }; }");
const auto Results =
@@ -1049,25 +1217,29 @@ TEST(ExprMutationAnalyzerTest, LambdaExplicitCaptureByRef) {
ElementsAre(ResultOf(removeSpace, "[&x](){x=10;}")));
}
+// section: range-for loops
+
TEST(ExprMutationAnalyzerTest, RangeForArrayByRefModified) {
auto AST =
buildASTFromCode("void f() { int x[2]; for (int& e : x) e = 10; }");
auto Results =
match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
- EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("e", "e = 10"));
+ EXPECT_THAT(mutatedBy(Results, AST.get()),
+ ElementsAre("for (int &e : x)\n e = 10;"));
AST = buildASTFromCode("typedef int& IntRef;"
"void f() { int x[2]; for (IntRef e : x) e = 10; }");
Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
- EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("e", "e = 10"));
+ EXPECT_THAT(mutatedBy(Results, AST.get()),
+ ElementsAre("for (IntRef e : x)\n e = 10;"));
}
-TEST(ExprMutationAnalyzerTest, RangeForArrayByRefNotModified) {
+TEST(ExprMutationAnalyzerTest, RangeForArrayByRefModifiedByImplicitInit) {
const auto AST =
buildASTFromCode("void f() { int x[2]; for (int& e : x) e; }");
const auto Results =
match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
- EXPECT_FALSE(isMutated(Results, AST.get()));
+ EXPECT_TRUE(isMutated(Results, AST.get()));
}
TEST(ExprMutationAnalyzerTest, RangeForArrayByValue) {
@@ -1107,7 +1279,8 @@ TEST(ExprMutationAnalyzerTest, RangeForNonArrayByRefModified) {
"void f() { V x; for (int& e : x) e = 10; }");
const auto Results =
match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
- EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("e", "e = 10"));
+ EXPECT_THAT(mutatedBy(Results, AST.get()),
+ ElementsAre("for (int &e : x)\n e = 10;"));
}
TEST(ExprMutationAnalyzerTest, RangeForNonArrayByRefNotModified) {
@@ -1115,7 +1288,7 @@ TEST(ExprMutationAnalyzerTest, RangeForNonArrayByRefNotModified) {
"void f() { V x; for (int& e : x) e; }");
const auto Results =
match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
- EXPECT_FALSE(isMutated(Results, AST.get()));
+ EXPECT_TRUE(isMutated(Results, AST.get()));
}
TEST(ExprMutationAnalyzerTest, RangeForNonArrayByValue) {
@@ -1136,6 +1309,8 @@ TEST(ExprMutationAnalyzerTest, RangeForNonArrayByConstRef) {
EXPECT_FALSE(isMutated(Results, AST.get()));
}
+// section: unevaluated expressions
+
TEST(ExprMutationAnalyzerTest, UnevaluatedExpressions) {
auto AST = buildASTFromCode("void f() { int x, y; decltype(x = 10) z = y; }");
auto Results =
@@ -1189,6 +1364,8 @@ TEST(ExprMutationAnalyzerTest, NotUnevaluatedExpressions) {
EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x.f()"));
}
+// section: special case: smartpointers
+
TEST(ExprMutationAnalyzerTest, UniquePtr) {
const std::string UniquePtrDef =
"template <class T> struct UniquePtr {"
@@ -1246,6 +1423,24 @@ TEST(ExprMutationAnalyzerTest, UniquePtr) {
EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x->mf()"));
}
+// section: complex problems detected on real code
+
+TEST(ExprMutationAnalyzerTest, UnevaluatedContext) {
+ const std::string Example =
+ "template <typename T>"
+ "struct to_construct : T { to_construct(int &j) {} };"
+ "template <typename T>"
+ "void placement_new_in_unique_ptr() { int x = 0;"
+ " new to_construct<T>(x);"
+ "}";
+ auto AST =
+ buildASTFromCodeWithArgs(Example, {"-fno-delayed-template-parsing"});
+ auto Results =
+ match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+ EXPECT_TRUE(isMutated(Results, AST.get()));
+ EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("(x)"));
+}
+
TEST(ExprMutationAnalyzerTest, ReproduceFailureMinimal) {
const std::string Reproducer =
"namespace std {"
More information about the cfe-commits
mailing list