This revision was automatically updated to reflect the committed changes.
Closed by commit rL277757: [analyzer] Make CloneDetector recognize different
variable patterns. (authored by dergachev).
Changed prior to commit:
https://reviews.llvm.org/D22982?vs=66839=66842#toc
Repository:
rL LLVM
teemperor updated this revision to Diff 66839.
teemperor marked 3 inline comments as done.
teemperor added a comment.
- Patch should no longer cause merge conflicts.
- Improved comments and tests in functions.cpp.
https://reviews.llvm.org/D22982
Files:
lib/Analysis/CloneDetection.cpp
NoQ requested changes to this revision.
NoQ added a comment.
This revision now requires changes to proceed.
Whoops, because of a merge conflict while applying the patch, i noticed a few
problems in the tests, they seem obvious, but could you have a quick look?
Comment at:
v.g.vassilev accepted this revision.
v.g.vassilev added a comment.
LGTM. My comments were clarified in a Skype chat.
https://reviews.llvm.org/D22982
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
NoQ accepted this revision.
This revision is now accepted and ready to land.
Comment at: lib/Analysis/CloneDetection.cpp:99
@@ +98,3 @@
+ /// Every item in this list is unique.
+ std::vector Variables;
+
NoQ wrote:
> I've a feeling this is essentially a `map`,
NoQ added a comment.
Looks good, minor comments :)
Comment at: lib/Analysis/CloneDetection.cpp:99
@@ +98,3 @@
+ /// Every item in this list is unique.
+ std::vector Variables;
+
I've a feeling this is essentially a `map`, because
that harmonizes with our
NoQ added a comment.
Uhm. I've a feeling that now i realize how it should have been written from the
start. That's a pretty bad feeling to have.
I wish we had a //series of passes//. Before the first pass, all statements are
considered equivalent. After the first pass, they're split into
teemperor added a comment.
I've deleted false-positives.cpp because all false-positives in there are now
resolved and the test did no longer serve a purpose. We could leave the test
file in there, but I think new false-positives either belong to an existing
test category or deserve their own
teemperor marked 2 inline comments as done.
teemperor added a comment.
https://reviews.llvm.org/D22982
___
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 66262.
teemperor added a comment.
- Fixed a few typos in the comments.
- Removed unnecessary `std::`.
- Added the new tests to functions.cpp instead of their own file.
https://reviews.llvm.org/D22982
Files:
lib/Analysis/CloneDetection.cpp
vsk added a subscriber: vsk.
vsk added a comment.
This is neat!
I may be missing something, but could you explain why 'false-positives.cpp' was
deleted? I'd expect the test to be incorporated into 'functions.cpp' with the
'expected-*' directives removed.
Comment at:
teemperor created this revision.
teemperor added reviewers: v.g.vassilev, NoQ, zaks.anna.
teemperor added a subscriber: cfe-commits.
One of the current false-positives the CloneDetector produces is that the
statements `a < b ? b` and `b < a ? b` are considered clones of each other,
even though
12 matches
Mail list logo
