135 lines · cpp
1//===----------------------------------------------------------------------===//2//3// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.4// See https://llvm.org/LICENSE.txt for license information.5// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception6//7//===----------------------------------------------------------------------===//8 9#include "SlicingCheck.h"10#include "clang/AST/ASTContext.h"11#include "clang/AST/RecordLayout.h"12#include "clang/ASTMatchers/ASTMatchFinder.h"13#include "clang/ASTMatchers/ASTMatchers.h"14 15using namespace clang::ast_matchers;16 17namespace clang::tidy::cppcoreguidelines {18 19void SlicingCheck::registerMatchers(MatchFinder *Finder) {20 // When we see:21 // class B : public A { ... };22 // A a;23 // B b;24 // a = b;25 // The assignment is OK if:26 // - the assignment operator is defined as taking a B as second parameter,27 // or28 // - B does not define any additional members (either variables or29 // overrides) wrt A.30 //31 // The same holds for copy ctor calls. This also captures stuff like:32 // void f(A a);33 // f(b);34 35 // Helpers.36 const auto OfBaseClass = ofClass(cxxRecordDecl().bind("BaseDecl"));37 const auto IsDerivedFromBaseDecl =38 cxxRecordDecl(isDerivedFrom(equalsBoundNode("BaseDecl")))39 .bind("DerivedDecl");40 const auto HasTypeDerivedFromBaseDecl =41 anyOf(hasType(IsDerivedFromBaseDecl),42 hasType(references(IsDerivedFromBaseDecl)));43 const auto IsCallToBaseClass = hasParent(cxxConstructorDecl(44 ofClass(isSameOrDerivedFrom(equalsBoundNode("DerivedDecl"))),45 hasAnyConstructorInitializer(allOf(46 isBaseInitializer(), withInitializer(equalsBoundNode("Call"))))));47 48 // Assignment slicing: "a = b;" and "a = std::move(b);" variants.49 const auto SlicesObjectInAssignment =50 callExpr(expr().bind("Call"),51 callee(cxxMethodDecl(anyOf(isCopyAssignmentOperator(),52 isMoveAssignmentOperator()),53 OfBaseClass)),54 hasArgument(1, HasTypeDerivedFromBaseDecl));55 56 // Construction slicing: "A a{b};" and "f(b);" variants. Note that in case of57 // slicing the letter will create a temporary and therefore call a ctor.58 const auto SlicesObjectInCtor = cxxConstructExpr(59 expr().bind("Call"),60 hasDeclaration(cxxConstructorDecl(61 anyOf(isCopyConstructor(), isMoveConstructor()), OfBaseClass)),62 hasArgument(0, HasTypeDerivedFromBaseDecl),63 // We need to disable matching on the call to the base copy/move64 // constructor in DerivedDecl's constructors.65 unless(IsCallToBaseClass));66 67 Finder->addMatcher(68 traverse(TK_AsIs, expr(SlicesObjectInAssignment).bind("Call")), this);69 Finder->addMatcher(traverse(TK_AsIs, SlicesObjectInCtor), this);70}71 72/// Warns on methods overridden in DerivedDecl with respect to BaseDecl.73/// FIXME: this warns on all overrides outside of the sliced path in case of74/// multiple inheritance.75void SlicingCheck::diagnoseSlicedOverriddenMethods(76 const Expr &Call, const CXXRecordDecl &DerivedDecl,77 const CXXRecordDecl &BaseDecl) {78 if (DerivedDecl.getCanonicalDecl() == BaseDecl.getCanonicalDecl())79 return;80 for (const auto *Method : DerivedDecl.methods()) {81 // Virtual destructors are OK. We're ignoring constructors since they are82 // tagged as overrides.83 if (isa<CXXConstructorDecl>(Method) || isa<CXXDestructorDecl>(Method))84 continue;85 if (Method->size_overridden_methods() > 0) {86 diag(Call.getExprLoc(),87 "slicing object from type %0 to %1 discards override %2")88 << &DerivedDecl << &BaseDecl << Method;89 }90 }91 // Recursively process bases.92 for (const auto &Base : DerivedDecl.bases()) {93 if (const auto *BaseRecord = Base.getType()->getAsCXXRecordDecl()) {94 if (BaseRecord->isCompleteDefinition())95 diagnoseSlicedOverriddenMethods(Call, *BaseRecord, BaseDecl);96 }97 }98}99 100void SlicingCheck::check(const MatchFinder::MatchResult &Result) {101 const auto *BaseDecl = Result.Nodes.getNodeAs<CXXRecordDecl>("BaseDecl");102 const auto *DerivedDecl =103 Result.Nodes.getNodeAs<CXXRecordDecl>("DerivedDecl");104 const auto *Call = Result.Nodes.getNodeAs<Expr>("Call");105 assert(BaseDecl != nullptr);106 assert(DerivedDecl != nullptr);107 assert(Call != nullptr);108 109 // Warn when slicing the vtable.110 // We're looking through all the methods in the derived class and see if they111 // override some methods in the base class.112 // It's not enough to just test whether the class is polymorphic because we113 // would be fine slicing B to A if no method in B (or its bases) overrides114 // anything in A:115 // class A { virtual void f(); };116 // class B : public A {};117 // because in that case calling A::f is the same as calling B::f.118 diagnoseSlicedOverriddenMethods(*Call, *DerivedDecl, *BaseDecl);119 120 // Warn when slicing member variables.121 const auto &BaseLayout =122 BaseDecl->getASTContext().getASTRecordLayout(BaseDecl);123 const auto &DerivedLayout =124 DerivedDecl->getASTContext().getASTRecordLayout(DerivedDecl);125 const CharUnits StateSize =126 DerivedLayout.getDataSize() - BaseLayout.getDataSize();127 if (StateSize.isPositive()) {128 diag(Call->getExprLoc(), "slicing object from type %0 to %1 discards "129 "%2 bytes of state")130 << DerivedDecl << BaseDecl << static_cast<int>(StateSize.getQuantity());131 }132}133 134} // namespace clang::tidy::cppcoreguidelines135