[analyzer] CloneDetector now checks template arguments of function calls.

[teemperor]

Summary

The CloneDetector currently ignores template arguments in function calls which leads to false-positive clones such as isa<Expr>(S) and isa<Stmt>(S).

This patch adds functionality to handle template arguments for those function calls.

[NoQ]

lib/Analysis/CloneDetection.cpp

154 ↗

(On Diff #68173)

Maybe if (const FunctionDecl *D = S->getDirectCallee()) here, and then use D everywhere inside the if?

164 ↗

(On Diff #68173)

We don't discriminate between foo<A, AA>() and foo<AA, A>() here, because in both cases the stuff we hash gets concatenated to AAA. Just thought that we might be having this common problem everywhere, even though it barely shows up "in real life".

test/Analysis/copypaste/call.cpp

41 ↗

(On Diff #68173)

"temp" feels a lot more like "temporary" than like "template" (?)

[v.g.vassilev]

LGTM once the @NoQ remarks are addressed.

[teemperor]

lib/Analysis/CloneDetection.cpp

164 ↗

(On Diff #68173)

Yes, that could be a problem. I'll fix this issue here in this patch and the others in another patch.

cfe/trunk/lib/Analysis/CloneDetection.cpp

DEF_ADD_DATA(TypeTraitExpr, {
     addData(S->getTrait());
     for (unsigned i = 0; i < S->getNumArgs(); ++i)
       addData(S->getArg(i)->getType());
   })
 
   //--- Calls --------------------------------------------------------------//
   DEF_ADD_DATA(CallExpr, {
     // Function pointers don't have a callee and we just skip hashing it.
-    if (S->getDirectCallee())
-      addData(S->getDirectCallee()->getQualifiedNameAsString());
+    if (const FunctionDecl *D = S->getDirectCallee()) {
+      // If the function is a template instantiation, we also need to handle
+      // the template arguments as they are no included in the qualified name.
+      if (D->isTemplateInstantiation()) {
+        auto Args = D->getTemplateSpecializationArgs();
+        std::string ArgString;
+
+        // Print all template arguments into ArgString
+        llvm::raw_string_ostream OS(ArgString);
+        for (unsigned i = 0; i < Args->size(); ++i) {
+          Args->get(i).print(Context.getLangOpts(), OS);
+          // Add a padding character so that 'foo<X, XX>()' != 'foo<XX, X>()'.
+          OS << '\n';
+        }
+        OS.flush();
+
+        addData(ArgString);
+      }
+      addData(D->getQualifiedNameAsString());
+    }
   })
 
   //--- Exceptions ---------------------------------------------------------//
   DEF_ADD_DATA(CXXCatchStmt, { addData(S->getCaughtType()); })
 
   //--- C++ OOP Stmts ------------------------------------------------------//
   DEF_ADD_DATA(CXXDeleteExpr, {
     addData(S->isArrayFormAsWritten());

cfe/trunk/test/Analysis/copypaste/call.cpp

 
 bool fooPtr1(int x) {
   if (x > 0)
     return false;
   else if (x < 0)
     return funcPtr(1);
   return true;
 }
+
+// Test that we respect the template arguments of function templates
+
+template<typename T, unsigned N>
+bool templateFunc() { unsigned i = N; return false; }
+
+bool fooTemplate1(int x) {
+  if (x > 0)
+    return false;
+  else if (x < 0)
+    return templateFunc<int, 1>();
+  return true;
+}
+
+bool fooTemplate2(int x) {
+  if (x > 0)
+    return false;
+  else if (x < 0)
+    return templateFunc<long, 1>();
+  return true;
+}
+
+bool fooTemplate3(int x) {
+  if (x > 0)
+    return false;
+  else if (x < 0)
+    return templateFunc<long, 2>();
+  return true;
+}
+
+// Test that we don't just concatenate the template arguments into a string
+// without having any padding between them (e.g. foo<X, XX>() != foo<XX, X>()).
+
+class X {};
+class XX {};
+
+template<typename T1, typename T2>
+bool templatePaddingFunc() { return false; }
+
+bool fooTemplatePadding1(int x) {
+  if (x > 0)
+    return false;
+  else if (x < 0)
+    return templatePaddingFunc<X, XX>();
+  return true;
+}
+
+bool fooTemplatePadding2(int x) {
+  if (x > 0)
+    return false;
+  else if (x < 0)
+    return templatePaddingFunc<XX, X>();
+  return true;
+}