[PATCH] D21992: [clang-tidy] new cppcoreguidelines-slicing

[Clement Courbet via cfe-commits]

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 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);
> ```
Actually these will still create a CXXConstructExpr in the AST, e.g for case (2):

```
CallExpr 0x3d6e560 </usr/local/google/home/courbet/test2.cc:10:3, col:6> 'void'
|-ImplicitCastExpr 0x3d6e548 <col:3> 'void (*)(class A)' <FunctionToPointerDecay>
| `-DeclRefExpr 0x3d6e4f8 <col:3> 'void (class A)' lvalue Function 0x3d66550 'g' 'void (class A)'
`-CXXConstructExpr 0x3d6e5c8 <col:5> 'class A' 'void (const class A &) throw()'
  `-ImplicitCastExpr 0x3d6e5b0 <col:5> 'const class A' lvalue <NoOp>
    `-ImplicitCastExpr 0x3d6e590 <col:5> 'class A' lvalue <DerivedToBase (A)>
      `-DeclRefExpr 0x3d6e4d0 <col:5> 'class B' lvalue Var 0x3d6e300 'b' 'class B'
```

I alreagy have a test to case (2) and I've added one for case (1).


================
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;
----------------
hokein wrote:
> 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.
I think we still want to point out to the user which overrides are going to be discarded:

class A { virtual void f();  virtual void g(); }
class B : public A { void f() override;  void g() override; int a; }

The messages will be:
"slicing object from type 'B' to 'A' discards override 'f' "
"slicing object from type 'B' to 'A' discards override 'g' "
"slicing object from type 'B' to 'A' discards 4*sizeof(char) bytes of state"





http://reviews.llvm.org/D21992