Utils::containsNonConstMemberCall() now honours assign operator

Was returning false previously.
Fixes a false positive in range-loop.

CCMAIL: Ch.Ehrlicher@gmx.de

8 files changed, 44 insertions, 41 deletions

diff --git a/src/TypeUtils.cpp b/src/TypeUtils.cpp
index ebb0af83..0dea5120 100644
--- a/src/TypeUtils.cpp
+++ b/src/TypeUtils.cpp
@@ -22,6 +22,7 @@
 #include "TypeUtils.h"
 #include "Utils.h"
 #include "StmtBodyRange.h"
+#include "ClazyContext.h"
 #include <HierarchyUtils.h>
 #include <StringUtils.h>
 
@@ -30,7 +31,7 @@
 
 using namespace clang;
 
-bool TypeUtils::classifyQualType(const ASTContext *context, const VarDecl *varDecl, QualTypeClassification &classif, clang::Stmt *body)
+bool TypeUtils::classifyQualType(const ClazyContext *context, const VarDecl *varDecl, QualTypeClassification &classif, clang::Stmt *body)
 {
     if (!varDecl)
         return false;
@@ -43,7 +44,7 @@ bool TypeUtils::classifyQualType(const ASTContext *context, const VarDecl *varDe
     if (isUndeducibleAuto(paramType))
         return false;
 
-    classif.size_of_T = context->getTypeSize(qualType) / 8;
+    classif.size_of_T = context->astContext.getTypeSize(qualType) / 8;
     classif.isBig = classif.size_of_T > 16;
     CXXRecordDecl *recordDecl = paramType->getAsCXXRecordDecl();
     classif.isNonTriviallyCopyable = recordDecl && (recordDecl->hasNonTrivialCopyConstructor() || recordDecl->hasNonTrivialDestructor());
@@ -61,8 +62,9 @@ bool TypeUtils::classifyQualType(const ASTContext *context, const VarDecl *varDe
     } else if (classif.isConst && classif.isReference && !classif.isNonTriviallyCopyable && !classif.isBig) {
         classif.passSmallTrivialByValue = true;
     } else if (!classif.isConst && !classif.isReference && (classif.isBig || classif.isNonTriviallyCopyable)) {
-        if (body && (Utils::containsNonConstMemberCall(body, varDecl) || Utils::isPassedToFunction(StmtBodyRange(body), varDecl, /*byrefonly=*/ true)))
+        if (body && (Utils::containsNonConstMemberCall(context->parentMap, body, varDecl) || Utils::isPassedToFunction(StmtBodyRange(body), varDecl, /*byrefonly=*/ true)))
             return true;
+
         classif.passNonTriviallyCopyableByConstRef = classif.isNonTriviallyCopyable;
         if (classif.isBig) {
             classif.passBigTypeByConstRef = true;
diff --git a/src/TypeUtils.h b/src/TypeUtils.h
index ea62b431..b025d333 100644
--- a/src/TypeUtils.h
+++ b/src/TypeUtils.h
@@ -43,6 +43,8 @@ class CXXRecordDecl;
 class CXXBaseSpecifier;
 }
 
+class ClazyContext;
+
 namespace TypeUtils
 {
     /**
@@ -74,9 +76,9 @@ namespace TypeUtils
      * The optional parameter body is in order to advise non-const-ref -> value, since the body
      * needs to be inspected to see if we that would compile.
      */
-    CLAZYLIB_EXPORT bool classifyQualType(const clang::ASTContext *context, const clang::VarDecl *varDecl,
-                          QualTypeClassification &classification,
-                          clang::Stmt *body = nullptr);
+    CLAZYLIB_EXPORT bool classifyQualType(const ClazyContext *context, const clang::VarDecl *varDecl,
+                                          QualTypeClassification &classification,
+                                          clang::Stmt *body = nullptr);
 
     /**
      * If qt is a reference, return it without a reference.
diff --git a/src/Utils.cpp b/src/Utils.cpp
index 251a75bf..c24f5b7c 100644
--- a/src/Utils.cpp
+++ b/src/Utils.cpp
@@ -232,41 +232,40 @@ clang::ValueDecl * Utils::valueDeclForCallExpr(clang::CallExpr *expr)
     return nullptr;
 }
 
-
-bool Utils::containsNonConstMemberCall(Stmt *body, const VarDecl *varDecl)
+bool Utils::containsNonConstMemberCall(clang::ParentMap *map, Stmt *body, const VarDecl *varDecl)
 {
-    std::vector<CXXMemberCallExpr*> memberCalls;
-    HierarchyUtils::getChilds<CXXMemberCallExpr>(body, memberCalls);
-
-    for (CXXMemberCallExpr *memberCall : memberCalls) {
-        CXXMethodDecl *methodDecl = memberCall->getMethodDecl();
-        if (!methodDecl || methodDecl->isConst())
-            continue;
-
-        ValueDecl *valueDecl = Utils::valueDeclForMemberCall(memberCall);
-        if (!valueDecl)
-            continue;
+    if (!varDecl)
+        return false;
 
-        if (valueDecl == varDecl)
-            return true;
-    }
+    std::vector<MemberExpr*> memberExprs;
+    HierarchyUtils::getChilds<MemberExpr>(body, memberExprs);
 
-    // Check for operator calls:
-    std::vector<CXXOperatorCallExpr*> operatorCalls;
-    HierarchyUtils::getChilds<CXXOperatorCallExpr>(body, operatorCalls);
-    for (CXXOperatorCallExpr *operatorExpr : operatorCalls) {
-        FunctionDecl *fDecl = operatorExpr->getDirectCallee();
-        if (!fDecl)
-            continue;
-        CXXMethodDecl *methodDecl = dyn_cast<CXXMethodDecl>(fDecl);
-        if (methodDecl == nullptr || methodDecl->isConst())
-            continue;
+    for (MemberExpr *memberExpr : memberExprs) {
+        auto memberCall = HierarchyUtils::getFirstParentOfType<CXXMemberCallExpr>(map, memberExpr);
+        if (memberCall) {
+            CXXMethodDecl *methodDecl = memberCall->getMethodDecl();
+            if (methodDecl && !methodDecl->isConst()) {
+                ValueDecl *valueDecl = Utils::valueDeclForMemberCall(memberCall);
+                if (valueDecl == varDecl)
+                    return true;
+            }
+        }
 
-        ValueDecl *valueDecl = Utils::valueDeclForOperatorCall(operatorExpr);
-        if (!valueDecl)
-            continue;
+        auto operatorExpr = HierarchyUtils::getFirstParentOfType<CXXOperatorCallExpr>(map, memberExpr);
+        if (operatorExpr) {
+            FunctionDecl *fDecl = operatorExpr->getDirectCallee();
+            if (fDecl) {
+                auto methodDecl = dyn_cast<CXXMethodDecl>(fDecl);
+                if (methodDecl && methodDecl->isConst()) {
+                    ValueDecl *valueDecl = Utils::valueDeclForOperatorCall(operatorExpr);
+                    if (valueDecl == varDecl)
+                        return true;
+                }
+            }
+        }
 
-        if (valueDecl == varDecl)
+        auto binaryOperator = HierarchyUtils::getFirstParentOfType<BinaryOperator>(map, memberExpr);
+        if (binaryOperator && binaryOperator->isAssignmentOp())
             return true;
     }
 
diff --git a/src/Utils.h b/src/Utils.h
index 6aabee32..49dbc8a1 100644
--- a/src/Utils.h
+++ b/src/Utils.h
@@ -103,7 +103,7 @@ namespace Utils {
     // For example:
     // Foo foo; // this is the varDecl
     // while (bar) { foo.setValue(); // non-const call }
-    CLAZYLIB_EXPORT bool containsNonConstMemberCall(clang::Stmt *body, const clang::VarDecl *varDecl);
+    CLAZYLIB_EXPORT bool containsNonConstMemberCall(clang::ParentMap *map, clang::Stmt *body, const clang::VarDecl *varDecl);
 
     // Returns true if there's an assignment to varDecl in body
     // Example: our_var = something_else
diff --git a/src/checks/level1/foreach.cpp b/src/checks/level1/foreach.cpp
index 0c2d0ad2..881eea47 100644
--- a/src/checks/level1/foreach.cpp
+++ b/src/checks/level1/foreach.cpp
@@ -139,7 +139,7 @@ void Foreach::checkBigTypeMissingRef()
         return;
 
     TypeUtils::QualTypeClassification classif;
-    bool success = TypeUtils::classifyQualType(&m_astContext, varDecl, /*by-ref*/classif, forStatements.at(0));
+    bool success = TypeUtils::classifyQualType(m_context, varDecl, /*by-ref*/classif, forStatements.at(0));
     if (!success)
         return;
 
diff --git a/src/checks/level1/range-loop.cpp b/src/checks/level1/range-loop.cpp
index ed7ce1f2..bbe87b3c 100644
--- a/src/checks/level1/range-loop.cpp
+++ b/src/checks/level1/range-loop.cpp
@@ -83,7 +83,7 @@ void RangeLoop::checkPassByConstRefCorrectness(CXXForRangeStmt *rangeLoop)
 {
     TypeUtils::QualTypeClassification classif;
     auto varDecl = rangeLoop->getLoopVariable();
-    bool success = TypeUtils::classifyQualType(&m_astContext, varDecl, /*by-ref*/classif, rangeLoop);
+    bool success = TypeUtils::classifyQualType(m_context, varDecl, /*by-ref*/classif, rangeLoop);
     if (!success)
         return;
 
diff --git a/src/checks/level2/function-args-by-ref.cpp b/src/checks/level2/function-args-by-ref.cpp
index a8f2f5c2..a1fa94b7 100644
--- a/src/checks/level2/function-args-by-ref.cpp
+++ b/src/checks/level2/function-args-by-ref.cpp
@@ -114,7 +114,7 @@ void FunctionArgsByRef::processFunction(FunctionDecl *func)
             continue;
 
         TypeUtils::QualTypeClassification classif;
-        bool success = TypeUtils::classifyQualType(&m_astContext, param, classif, body);
+        bool success = TypeUtils::classifyQualType(m_context, param, classif, body);
         if (!success)
             continue;
 
diff --git a/src/checks/level2/function-args-by-value.cpp b/src/checks/level2/function-args-by-value.cpp
index 98efbee0..0c9e1fff 100644
--- a/src/checks/level2/function-args-by-value.cpp
+++ b/src/checks/level2/function-args-by-value.cpp
@@ -124,7 +124,7 @@ void FunctionArgsByValue::processFunction(FunctionDecl *func)
             continue;
 
         TypeUtils::QualTypeClassification classif;
-        bool success = TypeUtils::classifyQualType(&m_astContext, param, classif, body);
+        bool success = TypeUtils::classifyQualType(m_context, param, classif, body);
         if (!success)
             continue;