This revision was automatically updated to reflect the committed changes.
Closed by commit rL277449: [analyzer] Respect statement-specific data in
CloneDetection. (authored by dergachev).
Changed prior to commit:
https://reviews.llvm.org/D22514?vs=66309&id=66460#toc
Repository:
rL LLVM
http
teemperor updated this revision to Diff 66309.
teemperor added a comment.
- s/super class/base class/g
https://reviews.llvm.org/D22514
Files:
include/clang/Analysis/CloneDetection.h
lib/Analysis/CloneDetection.cpp
test/Analysis/copypaste/asm.cpp
test/Analysis/copypaste/attributes.cpp
teemperor updated this revision to Diff 66308.
teemperor added a comment.
- No longer re-initialize memory created from resize().
- Added test for checking that StmtDataCollector visits all parent classes.
https://reviews.llvm.org/D22514
Files:
include/clang/Analysis/CloneDetection.h
lib/An
NoQ added a comment.
Congrats on the fantastic hack that nobody else noticed! I've a feeling we
cannot add tests for that, because any test would quickly break if we change
the hash, though demonstrating that we fixed the problem would still be cool
(eg. take two unsupported expressions of diff
teemperor updated this revision to Diff 66261.
teemperor added a comment.
- Added typedef for the contents of the data vector.
- Fixed that StmtDataCollector isn't visiting all parent classes of a statement.
- Removed 'test-' prefix from test files.
- Fixed the other problems as pointed out by Art
teemperor marked an inline comment as done.
Comment at: lib/Analysis/CloneDetection.cpp:134
@@ +133,3 @@
+ DEF_ADD_DATA(Stmt, { addData(S->getStmtClass()); })
+ DEF_ADD_DATA(Expr, { addData(S->getType()); })
+
NoQ wrote:
> I noticed something: with this patch, y
NoQ added inline comments.
Comment at: lib/Analysis/CloneDetection.cpp:91
@@ +90,3 @@
+ ASTContext &Context;
+ std::vector &CollectedData;
+
Maybe it's a good idea to introduce a typedef for `unsigned` here, because it'd
be nice to be able to change it (eg., so
teemperor updated this revision to Diff 65909.
teemperor added a comment.
- Actually removed the duplicate test now.
https://reviews.llvm.org/D22514
Files:
lib/Analysis/CloneDetection.cpp
test/Analysis/copypaste/false-positives.cpp
test/Analysis/copypaste/functions.cpp
test/Analysis/cop
teemperor added a comment.
Is there a specific situation/bug we want to test against with these tests? I
looks to me as if they would mainly test against non-determinism (i.e. same
statements have different data due to non-determinism).
Comment at: lib/Analysis/CloneDetection.
teemperor updated this revision to Diff 65857.
teemperor marked 5 inline comments as done.
teemperor added a comment.
- Removed duplicate test case.
https://reviews.llvm.org/D22514
Files:
lib/Analysis/CloneDetection.cpp
test/Analysis/copypaste/false-positives.cpp
test/Analysis/copypaste/f
teemperor updated this revision to Diff 65854.
teemperor added a comment.
- Rebased patch.
- Fixed code style (`const type` instead of `type const`).
- Fixed missing const on some variables in StmtDataCollector.
- Rewrote a few comments/variable names.
https://reviews.llvm.org/D22514
Files:
l
v.g.vassilev added a comment.
Could we stress test the implementation by running on files from llvm's/clang's
test suite. For example, for a test TST we could do something like: cat TST >>
tmp_TST; cat TST >> tmp_TST and run our our copy-paste checker on tmp_TST.
This way we know how many clon
teemperor updated this revision to Diff 65086.
teemperor added a comment.
- StmtDataCollector is now using ConstStmtVisitor
- Added tests for StmtDataCollector
https://reviews.llvm.org/D22514
Files:
lib/Analysis/CloneDetection.cpp
test/Analysis/copypaste/test-asm.cpp
test/Analysis/copypas
NoQ added a comment.
The natural way to avoid both heavy artillery with `StmtNodes.inc` and
low-performance if-chains is to use a //Visitor//.
(`Const`)`StmtVisitor` is implemented as a huge switch, contents of which are
auto-generated from `StmtNodes.inc`. In fact, your approach with
`StmtNod
teemperor added a comment.
Binary size increases from 33017160 Bytes to 33060464 Bytes (+40 KiB).
I'm not sure if that's too much for such a minor feature, so I've added two new
revisions:
- One is the original patch with the other mentioned issues fixed (so, same +40
KiB size increase here).
teemperor updated this revision to Diff 64983.
teemperor added a comment.
- Replaced visitor-style utility functions with chained `if`s to reduce binary
size.
https://reviews.llvm.org/D22514
Files:
lib/Analysis/CloneDetection.cpp
test/Analysis/copypaste/test-min-max.cpp
Index: test/Analys
teemperor updated this revision to Diff 64982.
teemperor marked 3 inline comments as done.
teemperor added a comment.
- Added `FIXME:` and removed duplicate "for example".
https://reviews.llvm.org/D22514
Files:
lib/Analysis/CloneDetection.cpp
test/Analysis/copypaste/test-min-max.cpp
Index:
v.g.vassilev requested changes to this revision.
v.g.vassilev added a comment.
I guess the question about the binary size holds here, too. What would be the
impact on the binary size?
Comment at: lib/Analysis/CloneDetection.cpp:104
@@ +103,3 @@
+/// defines what a 'similar' clo
teemperor planned changes to this revision.
teemperor added a comment.
- Expand test suite to test newly added code.
https://reviews.llvm.org/D22514
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listin
teemperor created this revision.
teemperor added reviewers: v.g.vassilev, zaks.anna, NoQ.
teemperor added a subscriber: cfe-commits.
So far the CloneDetector only respected the class of each statement when
searching for clones. This means that nodes that differentiate in any other
attribute are
20 matches
Mail list logo
