+++ /dev/null
-//===--- SlicingCheck.cpp - clang-tidy-------------------------------------===//
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===----------------------------------------------------------------------===//
-
-#include "SlicingCheck.h"
-#include "clang/AST/ASTContext.h"
-#include "clang/AST/RecordLayout.h"
-#include "clang/ASTMatchers/ASTMatchFinder.h"
-#include "clang/ASTMatchers/ASTMatchers.h"
-
-using namespace clang::ast_matchers;
-
-namespace clang {
-namespace tidy {
-namespace cppcoreguidelines {
-
-void SlicingCheck::registerMatchers(MatchFinder *Finder) {
- // When we see:
- // class B : public A { ... };
- // A a;
- // B b;
- // a = b;
- // The assignment is OK if:
- // - the assignment operator is defined as taking a B as second parameter,
- // or
- // - B does not define any additional members (either variables or
- // overrides) wrt A.
- //
- // The same holds for copy ctor calls. This also captures stuff like:
- // void f(A a);
- // f(b);
-
- // Helpers.
- const auto OfBaseClass = ofClass(cxxRecordDecl().bind("BaseDecl"));
- const auto IsDerivedFromBaseDecl =
- cxxRecordDecl(isDerivedFrom(equalsBoundNode("BaseDecl")))
- .bind("DerivedDecl");
- const auto HasTypeDerivedFromBaseDecl =
- anyOf(hasType(IsDerivedFromBaseDecl),
- hasType(references(IsDerivedFromBaseDecl)));
- const auto IsWithinDerivedCtor =
- hasParent(cxxConstructorDecl(ofClass(equalsBoundNode("DerivedDecl"))));
-
- // Assignement slicing: "a = b;" and "a = std::move(b);" variants.
- const auto SlicesObjectInAssignment =
- callExpr(callee(cxxMethodDecl(anyOf(isCopyAssignmentOperator(),
- isMoveAssignmentOperator()),
- OfBaseClass)),
- hasArgument(1, HasTypeDerivedFromBaseDecl));
-
- // Construction slicing: "A a{b};" and "f(b);" variants. Note that in case of
- // slicing the letter will create a temporary and therefore call a ctor.
- const auto SlicesObjectInCtor = cxxConstructExpr(
- hasDeclaration(cxxConstructorDecl(
- anyOf(isCopyConstructor(), isMoveConstructor()), OfBaseClass)),
- hasArgument(0, HasTypeDerivedFromBaseDecl),
- // We need to disable matching on the call to the base copy/move
- // constructor in DerivedDecl's constructors.
- unless(IsWithinDerivedCtor));
-
- Finder->addMatcher(
- expr(anyOf(SlicesObjectInAssignment, SlicesObjectInCtor)).bind("Call"),
- this);
-}
-
-/// Warns on methods overridden in DerivedDecl with respect to BaseDecl.
-/// FIXME: this warns on all overrides outside of the sliced path in case of
-/// multiple inheritance.
-void SlicingCheck::DiagnoseSlicedOverriddenMethods(
- const Expr &Call, const CXXRecordDecl &DerivedDecl,
- const CXXRecordDecl &BaseDecl) {
- if (DerivedDecl.getCanonicalDecl() == BaseDecl.getCanonicalDecl())
- return;
- for (const auto &Method : DerivedDecl.methods()) {
- // Virtual destructors are OK. We're ignoring constructors since they are
- // tagged as overrides.
- if (isa<CXXConstructorDecl>(Method) || isa<CXXDestructorDecl>(Method))
- continue;
- if (Method->size_overridden_methods() > 0) {
- diag(Call.getExprLoc(),
- "slicing object from type %0 to %1 discards override %2")
- << &DerivedDecl << &BaseDecl << Method;
- }
- }
- // Recursively process bases.
- for (const auto &Base : DerivedDecl.bases()) {
- if (const auto *BaseRecordType = Base.getType()->getAs<RecordType>()) {
- if (const auto *BaseRecord = cast_or_null<CXXRecordDecl>(
- BaseRecordType->getDecl()->getDefinition()))
- DiagnoseSlicedOverriddenMethods(Call, *BaseRecord, BaseDecl);
- }
- }
-}
-
-void SlicingCheck::check(const MatchFinder::MatchResult &Result) {
- const auto *BaseDecl = Result.Nodes.getNodeAs<CXXRecordDecl>("BaseDecl");
- const auto *DerivedDecl =
- Result.Nodes.getNodeAs<CXXRecordDecl>("DerivedDecl");
- const auto *Call = Result.Nodes.getNodeAs<Expr>("Call");
- assert(BaseDecl != nullptr);
- assert(DerivedDecl != nullptr);
- assert(Call != nullptr);
-
- // Warn when slicing the vtable.
- // We're looking through all the methods in the derived class and see if they
- // override some methods in the base class.
- // It's not enough to just test whether the class is polymorphic because we
- // would be fine slicing B to A if no method in B (or its bases) overrides
- // anything in A:
- // class A { virtual void f(); };
- // class B : public A {};
- // because in that case calling A::f is the same as calling B::f.
- DiagnoseSlicedOverriddenMethods(*Call, *DerivedDecl, *BaseDecl);
-
- // Warn when slicing member variables.
- const auto &BaseLayout =
- BaseDecl->getASTContext().getASTRecordLayout(BaseDecl);
- const auto &DerivedLayout =
- DerivedDecl->getASTContext().getASTRecordLayout(DerivedDecl);
- const auto StateSize = DerivedLayout.getDataSize() - BaseLayout.getDataSize();
- if (StateSize.isPositive()) {
- diag(Call->getExprLoc(), "slicing object from type %0 to %1 discards "
- "%2*sizeof(char) bytes of state")
- << DerivedDecl << BaseDecl << static_cast<int>(StateSize.getQuantity());
- }
-}
-
-} // namespace cppcoreguidelines
-} // namespace tidy
-} // namespace clang
+++ /dev/null
-//===--- SlicingCheck.h - clang-tidy-----------------------------*- C++ -*-===//
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===----------------------------------------------------------------------===//
-
-#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_SLICING_H
-#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_SLICING_H
-
-#include "../ClangTidy.h"
-
-namespace clang {
-namespace tidy {
-namespace cppcoreguidelines {
-
-/// Flags slicing (incomplete copying of an object's state) of member variables
-/// or vtable. See:
-/// - https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#es63-dont-slice
-/// for the former, and
-/// - https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#c145-access-polymorphic-objects-through-pointers-and-references
-/// for the latter
-///
-/// For the user-facing documentation see:
-/// http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-slicing.html
-class SlicingCheck : public ClangTidyCheck {
-public:
- SlicingCheck(StringRef Name, ClangTidyContext *Context)
- : ClangTidyCheck(Name, Context) {}
- void registerMatchers(ast_matchers::MatchFinder *Finder) override;
- void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
-
-private:
- void DiagnoseSlicedOverriddenMethods(const Expr &call,
- const CXXRecordDecl &DerivedDecl,
- const CXXRecordDecl &BaseDecl);
-};
-
-} // namespace cppcoreguidelines
-} // namespace tidy
-} // namespace clang
-
-#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_SLICING_H
+++ /dev/null
-// RUN: %check_clang_tidy %s cppcoreguidelines-slicing %t
-
-class Base {
- int i;
- void f() {}
- virtual void g() {}
-};
-
-class DerivedWithMemberVariables : public Base {
- void f();
- int j;
-};
-
-class TwiceDerivedWithNoMemberVariables : public DerivedWithMemberVariables {
- void f();
-};
-
-class DerivedWithOverride : public Base {
- void f();
- void g() override {}
-};
-
-class TwiceDerivedWithNoOverride : public DerivedWithOverride {
- void f();
-};
-
-void TakesBaseByValue(Base base);
-
-DerivedWithMemberVariables ReturnsDerived();
-
-void positivesWithMemberVariables() {
- DerivedWithMemberVariables b;
- Base a{b};
- // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: slicing object from type 'DerivedWithMemberVariables' to 'Base' discards {{[0-9]*}}*sizeof(char) bytes of state [cppcoreguidelines-slicing]
- a = b;
- // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: slicing object from type 'DerivedWithMemberVariables' to 'Base' discards {{[0-9]*}}*sizeof(char) bytes of state
- TakesBaseByValue(b);
- // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: slicing object from type 'DerivedWithMemberVariables' to 'Base' discards {{[0-9]*}}*sizeof(char) bytes of state
-
- TwiceDerivedWithNoMemberVariables c;
- a = c;
- // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: slicing object from type 'TwiceDerivedWithNoMemberVariables' to 'Base' discards {{[0-9]*}}*sizeof(char) bytes of state
-
- a = ReturnsDerived();
- // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: slicing object from type 'DerivedWithMemberVariables' to 'Base' discards 4*sizeof(char) bytes of state
-}
-
-void positivesWithOverride() {
- DerivedWithOverride b;
- Base a{b};
- // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: slicing object from type 'DerivedWithOverride' to 'Base' discards override 'g'
- a = b;
- // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: slicing object from type 'DerivedWithOverride' to 'Base' discards override 'g'
- TakesBaseByValue(b);
- // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: slicing object from type 'DerivedWithOverride' to 'Base' discards override 'g'
-
- TwiceDerivedWithNoOverride c;
- a = c;
- // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: slicing object from type 'DerivedWithOverride' to 'Base' discards override 'g'
-}
-
-void TakesBaseByReference(Base &base);
-
-class DerivedThatAddsVirtualH : public Base {
- virtual void h();
-};
-
-class DerivedThatOverridesH : public DerivedThatAddsVirtualH {
- void h() override;
-};
-
-void negatives() {
- // OK, simple copying from the same type.
- Base a;
- TakesBaseByValue(a);
- DerivedWithMemberVariables b;
- DerivedWithMemberVariables c{b};
- b = c;
-
- // OK, derived type does not have extra state.
- TwiceDerivedWithNoMemberVariables d;
- DerivedWithMemberVariables e{d};
- e = d;
-
- // OK, derived does not override any method.
- TwiceDerivedWithNoOverride f;
- DerivedWithOverride g{f};
- g = f;
-
- // OK, no copying.
- TakesBaseByReference(d);
- TakesBaseByReference(f);
-
- // Derived type overrides methods, but these methods are not in the base type,
- // so cannot be called accidentally. Right now this triggers, but we might
- // want to allow it.
- DerivedThatOverridesH h;
- a = h;
- // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: slicing object from type 'DerivedThatOverridesH' to 'Base' discards override 'h'
-}