courbet added a comment.
I can submit, thanks.
https://reviews.llvm.org/D21992
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
hokein accepted this revision.
hokein added a comment.
Looks good now. Do you need I submit for you?
https://reviews.llvm.org/D21992
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
courbet updated this revision to Diff 65028.
courbet marked an inline comment as done.
https://reviews.llvm.org/D21992
Files:
clang-tidy/cppcoreguidelines/CMakeLists.txt
clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
clang-tidy/cppcoreguidelines/SlicingCheck.cpp
clang-tidy/c
Prazek added a comment.
Is it in upstream yet?
Comment at: clang-tidy/cppcoreguidelines/SlicingCheck.h:19
@@ +18,3 @@
+
+/// Flags slicing of member variables or vtable. See:
+/// -
courbet wrote:
> Prazek wrote:
> > some short description what does this check
courbet marked an inline comment as done.
Comment at: clang-tidy/cppcoreguidelines/SlicingCheck.h:19
@@ +18,3 @@
+
+/// Flags slicing of member variables or vtable. See:
+/// -
Prazek wrote:
> some short description what does this check do?
There is already a mo
Prazek added a subscriber: Prazek.
Comment at: clang-tidy/cppcoreguidelines/SlicingCheck.cpp:30
@@ +29,3 @@
+ // or
+ // - B does not define any additional members (either variables or
+ // overrides) wrt A.
What is A and what is B? I guess you are missin
courbet updated this revision to Diff 64642.
courbet added a comment.
rebase
https://reviews.llvm.org/D21992
Files:
clang-tidy/cppcoreguidelines/CMakeLists.txt
clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
clang-tidy/cppcoreguidelines/SlicingCheck.cpp
clang-tidy/cppcoregu
courbet added a comment.
Thanks for the review.
Comment at: clang-tidy/cppcoreguidelines/SlicingCheck.cpp:86
@@ +85,3 @@
+ "slicing object from type %0 to %1 discards override %2")
+ << &DerivedDecl << &BaseDecl << Method;
+}
alexfh wrote:
courbet updated this revision to Diff 64641.
courbet marked an inline comment as done.
courbet added a comment.
cosmetics
https://reviews.llvm.org/D21992
Files:
clang-tidy/cppcoreguidelines/CMakeLists.txt
clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
clang-tidy/cppcoreguide
alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.
LG with a nit.
Comment at: clang-tidy/cppcoreguidelines/SlicingCheck.cpp:86
@@ +85,3 @@
+ "slicing object from type %0 to %1 discards override %2")
+ << &De
courbet updated this revision to Diff 64302.
courbet marked 3 inline comments as done.
courbet added a comment.
Cosmetics.
https://reviews.llvm.org/D21992
Files:
clang-tidy/cppcoreguidelines/CMakeLists.txt
clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
clang-tidy/cppcoreguid
hokein added inline comments.
Comment at: clang-tidy/cppcoreguidelines/SlicingCheck.cpp:48
@@ +47,3 @@
+
+ // Assignement slicing: "a = b;" and "a = std::move(b);" variants.
+ const auto SlicesObjectInAssignment =
courbet wrote:
> hokein wrote:
> > Looks like yo
courbet added a comment.
I've ran this check on llvm. There are 0 instances of virtual function slicing
(which is not surprising since these usually result in actual bugs) and 71
instances of member varaible slicing:
- 'FullSourceLoc' to 'SourceLocation': 57
- 'APSInt' to 'APInt': 5
- 'DeducedT
courbet added a comment.
Thanks for the comments.
Comment at: clang-tidy/cppcoreguidelines/SlicingCheck.cpp:48
@@ +47,3 @@
+
+ // Assignement slicing: "a = b;" and "a = std::move(b);" variants.
+ const auto SlicesObjectInAssignment =
hokein wrote:
> Looks like
courbet updated this revision to Diff 63477.
courbet marked 4 inline comments as done.
courbet added a comment.
- Add a test case following comments.
- Add more comments in tests.
- Add examples in doc.
- Simplify code a bit.
http://reviews.llvm.org/D21992
Files:
clang-tidy/cppcoreguidelines/
hokein added inline comments.
Comment at: clang-tidy/cppcoreguidelines/SlicingCheck.cpp:48
@@ +47,3 @@
+
+ // Assignement slicing: "a = b;" and "a = std::move(b);" variants.
+ const auto SlicesObjectInAssignment =
Looks like you are missing some cases here, like
alexfh added a comment.
Thanks for the new awesome check!
Please run the check on LLVM and include your analysis of the results in the
patch description. Another couple of comments below.
Comment at: clang-tidy/cppcoreguidelines/SlicingCheck.cpp:93
@@ +92,3 @@
+ continue;
courbet updated this revision to Diff 62845.
courbet added a comment.
Update release notes.
http://reviews.llvm.org/D21992
Files:
clang-tidy/cppcoreguidelines/CMakeLists.txt
clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
clang-tidy/cppcoreguidelines/SlicingCheck.cpp
clang-
Eugene.Zelenko added a subscriber: Eugene.Zelenko.
Eugene.Zelenko added a comment.
Please mention this check in docs/ReleaseNotes.rst (in alphabetical order).
http://reviews.llvm.org/D21992
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
htt
19 matches
Mail list logo
