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 assigning a Subclass object
from a function call to a Baseclass object, passing a Subclass object to a
function whose parameter is a BaseClass.
But I think it's fine to implement in a separate patch, but you'd better to add
a TODO here.
```
SubClass f1();
BaseClass a = f1();
void f1(BaseClass a);
SubClass sc;
f1(sc);
```
================
Comment at: clang-tidy/cppcoreguidelines/SlicingCheck.cpp:84
@@ +83,3 @@
+ diag(Call.getExprLoc(),
+ "slicing object from type %0 to %1 discards override %2")
+ << &DerivedDecl << &BaseDecl << Method;
----------------
There are two diagnostic messages in the code. For subclasses with override
base class methods and extra member, this will print two messages, which is a
bit of redundant.
================
Comment at: clang-tidy/cppcoreguidelines/SlicingCheck.cpp:94
@@ +93,3 @@
+ CXXRecordDecl *BaseRecord =
+
cast_or_null<CXXRecordDecl>(BaseRecordType->getDecl()->getDefinition());
+ if (!BaseRecord)
----------------
The code can be simplified like:
```
if (const auto *BR =
cast_or_null<CXXRecordDecl>(BaseRecordType->getDecl()->getDefinition())) {
DiagnoseSlicedOverriddenMethods(Call, *BR, BaseDecl);
}
```
================
Comment at: clang-tidy/cppcoreguidelines/SlicingCheck.cpp:112
@@ +111,3 @@
+ // We're looking through all the methods in the derived class and see if they
+ // override some method in the base class.
+ // It's not enough to just test whether the class is polymorphic because we
----------------
s/method/methods
http://reviews.llvm.org/D21992
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
