[PATCH] D31320: [analyzer] Teach CloneDetection about Qt Meta-Object Compiler

[Raphael Isemann via Phabricator via cfe-commits]

teemperor added a comment.

See my inline comments about technical changes, but otherwise this looks ready to land. Please update and I'll give green light ASAP. Thanks!



================
Comment at: lib/Analysis/CloneDetection.cpp:375
+    const Decl *D = S.getContainingDecl();
+    const SourceManager &SM = D->getASTContext().getSourceManager();
+    std::string Filename = std::string(SM.getFilename(D->getLocation()));
----------------
You can skip the `const Decl *D = S.getContainingDecl();` and just do `const SourceManager &SM = S.getASTContext().getSourceManager();`


================
Comment at: lib/Analysis/CloneDetection.cpp:378
+    // Get Basename
+    const size_t LastSlash = Filename.find_last_of("\\/");
+    if (LastSlash != std::string::npos)
----------------
Let's get the basename with the LLVM path class instead:
 http://llvm.org/docs/doxygen/html/namespacellvm_1_1sys_1_1path.html#a799b002e67dcf41330fa8d6fa11823dc

E.g. `moc_test\.cpp` is a valid filename on Linux and then this code isn't doing the right thing.


================
Comment at: lib/Analysis/CloneDetection.cpp:383
+    if (LastDot != std::string::npos)
+      Filename.erase(LastDot);
+    llvm::Regex R(StringRef("^(" + IgnoredFilesPattern.str() + "$)"));
----------------
Hmm, we can't filter by file extension this way? Can we remove this extension stripping and just make people type the file extension in the filter, this feels more intuitive.


================
Comment at: lib/Analysis/CloneDetection.cpp:384
+      Filename.erase(LastDot);
+    llvm::Regex R(StringRef("^(" + IgnoredFilesPattern.str() + "$)"));
+    if (R.match(StringRef(Filename)))
----------------
Artem suggested we could move this Regex out of the loop, I think we should even make it a member of the AutoGeneratedCloneConstraint so that we only parse/generate the regex state machine once per invocation. Currently we reparse this Regex a few thousand times and performance is quite important in this Checker.


Repository:
  rL LLVM

https://reviews.llvm.org/D31320