This revision was automatically updated to reflect the committed changes.
Closed by commit rL276782: [analyzer] Add basic capabilities to detect source
code clones. (authored by dergachev).
Changed prior to commit:
https://reviews.llvm.org/D20795?vs=65506=65563#toc
Repository:
rL LLVM
NoQ accepted this revision.
NoQ added a reviewer: NoQ.
NoQ added a comment.
Great! Will commit.
https://reviews.llvm.org/D20795
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
teemperor updated this revision to Diff 65506.
teemperor added a comment.
- Now passing ChildSignatures by const reference.
https://reviews.llvm.org/D20795
Files:
include/clang/Analysis/CloneDetection.h
include/clang/StaticAnalyzer/Checkers/Checkers.td
lib/Analysis/CMakeLists.txt
NoQ added inline comments.
Comment at: lib/Analysis/CloneDetection.cpp:148
@@ +147,3 @@
+
+// FIXME: This function has quadratic runtime right now. Check if skipping
+// this function for too long CompoundStmts is an option.
I've a feeling this comment
teemperor marked 3 inline comments as done.
teemperor added a comment.
https://reviews.llvm.org/D20795
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
teemperor added inline comments.
Comment at: lib/Analysis/CloneDetection.cpp:178
@@ +177,3 @@
+
+ bool VisitFunctionDecl(FunctionDecl *D) {
+// If we found a function, we start the clone search on its body statement.
NoQ wrote:
> You'd probably want to add
teemperor updated this revision to Diff 65441.
teemperor added a comment.
- The CloneGroup values in StringMap no longer store a copy of their own key.
https://reviews.llvm.org/D20795
Files:
include/clang/Analysis/CloneDetection.h
include/clang/StaticAnalyzer/Checkers/Checkers.td
teemperor updated this revision to Diff 65439.
teemperor marked 7 inline comments as done.
teemperor added a comment.
- Fixed the minor problems as pointed out by Artem.
- Now using AnalysisConsumer instead of RecursiveASTVisitor.
- Function names are now confirming to LLVM code-style.
- Renamed
NoQ added a comment.
The idea with strings as keys is curious! I've got a few more minor comments :)
Comment at: lib/Analysis/CloneDetection.cpp:104
@@ +103,3 @@
+// Storage for the signatures of the direct child statements. This is only
+// needed if the current
teemperor marked 9 inline comments as done.
Comment at: include/clang/AST/CloneDetection.h:149
@@ +148,3 @@
+/// (e.g. function bodies). Other clones (e.g. cloned comments or declarations)
+/// are not supported.
+///
Put the idea on the future TODO list, but we
teemperor updated this revision to Diff 65291.
teemperor added a comment.
Ok, so I think I've addressed the points from last meeting in this patch:
It was planned to replace custom hashing with FoldingSetNodeID and a hashmap:
In this patch I removed all custom hashing. It's now done via LLVM's
v.g.vassilev accepted this revision.
v.g.vassilev added a comment.
This revision is now accepted and ready to land.
LGTM, given the comments are addressed.
Comment at: include/clang/Analysis/CloneDetection.h:37
@@ +36,3 @@
+ /// Stmt, then S is a pointer to this Stmt.
+ Stmt
NoQ added a comment.
Regarding the cache stack - it feels easier for me to allocate a separate stack
for each statement, and put the stack on stack (!) rather than having it
global. This way it'd be automatically cleaned for you when VisitStmt() exits,
and you'd be able to address child cache
teemperor added inline comments.
Comment at: include/clang/AST/CloneDetection.h:131
@@ +130,3 @@
+ bool operator<(const StmtSequence ) const {
+return std::tie(S, StartIndex, EndIndex) <
+ std::tie(Other.S, Other.StartIndex, Other.EndIndex);
teemperor updated this revision to Diff 63851.
teemperor marked 18 inline comments as done.
teemperor added a comment.
- Checker is now in the alpha package and hidden.
- MinGroupComplexity is now a parameter for the checker.
- StmtData is now called CloneSignature.
- Replaced the std::map inside
NoQ added a comment.
Your code is well-written and easy to understand, and makes me want to use it
on my code! Added some minor comments, and there seems to be a small logic
error in the compound statement hasher.
Not sure if that was already explained in detail, but i seem to agree that the
zaks.anna added a comment.
Hi,
Thank you for working on this!
Here are several comments from a **partial** review.
Can you talk a bit about how do you envision to generalize this to copy and
paste detection, where the pasted code is only partially modified? Do you think
this could be
teemperor updated this revision to Diff 63266.
teemperor added a comment.
- StmtSequence is now treating all Stmt objects as const.
http://reviews.llvm.org/D20795
Files:
include/clang/AST/CloneDetection.h
include/clang/StaticAnalyzer/Checkers/Checkers.td
lib/AST/CMakeLists.txt
teemperor updated this revision to Diff 63262.
teemperor marked 7 inline comments as done.
http://reviews.llvm.org/D20795
Files:
include/clang/AST/CloneDetection.h
include/clang/StaticAnalyzer/Checkers/Checkers.td
lib/AST/CMakeLists.txt
lib/AST/CloneDetection.cpp
teemperor updated this revision to Diff 63161.
teemperor marked 5 inline comments as done.
teemperor added a comment.
- Fixed type of StmtSequence::iterator.
http://reviews.llvm.org/D20795
Files:
include/clang/AST/CloneDetection.h
include/clang/StaticAnalyzer/Checkers/Checkers.td
teemperor updated this revision to Diff 63159.
teemperor added a comment.
- Fixed the problems pointed out by Vassil (Thanks!)
- Comments now have less line breaks.
- Added more documentation to HashValue, it's hash function and the used prime
numbers.
- Added a typedef for
v.g.vassilev requested changes to this revision.
This revision now requires changes to proceed.
Comment at: include/clang/AST/CloneDetection.h:148
@@ +147,3 @@
+/// This class only searches for clones in exectuable source code
+/// (e.g. function bodies). Other clones (e.g.
teemperor updated this revision to Diff 62899.
teemperor added a comment.
- Fixed a few typos in comments and documentation.
http://reviews.llvm.org/D20795
Files:
include/clang/AST/CloneDetection.h
include/clang/StaticAnalyzer/Checkers/Checkers.td
lib/AST/CMakeLists.txt
teemperor updated this revision to Diff 62888.
teemperor added a comment.
- Using doxygen-style comments for all private members.
http://reviews.llvm.org/D20795
Files:
include/clang/AST/CloneDetection.h
include/clang/StaticAnalyzer/Checkers/Checkers.td
lib/AST/CMakeLists.txt
24 matches
Mail list logo
