xref: /llvm-project/clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp (revision 11a411a49b62c129bba551df4587dd446fcdc660)
1c58c7a6eSMarco Gartmann //===--- VirtualClassDestructorCheck.cpp - clang-tidy -----------------===//
2c58c7a6eSMarco Gartmann //
3c58c7a6eSMarco Gartmann // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4c58c7a6eSMarco Gartmann // See https://llvm.org/LICENSE.txt for license information.
5c58c7a6eSMarco Gartmann // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6c58c7a6eSMarco Gartmann //
7c58c7a6eSMarco Gartmann //===----------------------------------------------------------------------===//
8c58c7a6eSMarco Gartmann 
9c58c7a6eSMarco Gartmann #include "VirtualClassDestructorCheck.h"
10c58c7a6eSMarco Gartmann #include "../utils/LexerUtils.h"
11c58c7a6eSMarco Gartmann #include "clang/AST/ASTContext.h"
12c58c7a6eSMarco Gartmann #include "clang/ASTMatchers/ASTMatchFinder.h"
13c58c7a6eSMarco Gartmann #include "clang/Lex/Lexer.h"
1471f55735SKazu Hirata #include <optional>
15c58c7a6eSMarco Gartmann #include <string>
16c58c7a6eSMarco Gartmann 
17c58c7a6eSMarco Gartmann using namespace clang::ast_matchers;
18c58c7a6eSMarco Gartmann 
19*7d2ea6c4SCarlos Galvez namespace clang::tidy::cppcoreguidelines {
20c58c7a6eSMarco Gartmann 
AST_MATCHER(CXXRecordDecl,hasPublicVirtualOrProtectedNonVirtualDestructor)21f0711106SCarlos Galvez AST_MATCHER(CXXRecordDecl, hasPublicVirtualOrProtectedNonVirtualDestructor) {
22f0711106SCarlos Galvez   // We need to call Node.getDestructor() instead of matching a
23f0711106SCarlos Galvez   // CXXDestructorDecl. Otherwise, tests will fail for class templates, since
24f0711106SCarlos Galvez   // the primary template (not the specialization) always gets a non-virtual
25f0711106SCarlos Galvez   // CXXDestructorDecl in the AST. https://bugs.llvm.org/show_bug.cgi?id=51912
26f0711106SCarlos Galvez   const CXXDestructorDecl *Destructor = Node.getDestructor();
27f0711106SCarlos Galvez   if (!Destructor)
28f0711106SCarlos Galvez     return false;
29f0711106SCarlos Galvez 
30f0711106SCarlos Galvez   return (((Destructor->getAccess() == AccessSpecifier::AS_public) &&
31f0711106SCarlos Galvez            Destructor->isVirtual()) ||
32f0711106SCarlos Galvez           ((Destructor->getAccess() == AccessSpecifier::AS_protected) &&
33f0711106SCarlos Galvez            !Destructor->isVirtual()));
34f0711106SCarlos Galvez }
35f0711106SCarlos Galvez 
registerMatchers(MatchFinder * Finder)36c58c7a6eSMarco Gartmann void VirtualClassDestructorCheck::registerMatchers(MatchFinder *Finder) {
37c58c7a6eSMarco Gartmann   ast_matchers::internal::Matcher<CXXRecordDecl> InheritsVirtualMethod =
38c58c7a6eSMarco Gartmann       hasAnyBase(hasType(cxxRecordDecl(has(cxxMethodDecl(isVirtual())))));
39c58c7a6eSMarco Gartmann 
40c58c7a6eSMarco Gartmann   Finder->addMatcher(
41c58c7a6eSMarco Gartmann       cxxRecordDecl(
42c58c7a6eSMarco Gartmann           anyOf(has(cxxMethodDecl(isVirtual())), InheritsVirtualMethod),
43d9afb8c3SBalazs Benics           unless(isFinal()),
44f0711106SCarlos Galvez           unless(hasPublicVirtualOrProtectedNonVirtualDestructor()))
45c58c7a6eSMarco Gartmann           .bind("ProblematicClassOrStruct"),
46c58c7a6eSMarco Gartmann       this);
47c58c7a6eSMarco Gartmann }
48c58c7a6eSMarco Gartmann 
49f71ffd3bSKazu Hirata static std::optional<CharSourceRange>
getVirtualKeywordRange(const CXXDestructorDecl & Destructor,const SourceManager & SM,const LangOptions & LangOpts)50c58c7a6eSMarco Gartmann getVirtualKeywordRange(const CXXDestructorDecl &Destructor,
51c58c7a6eSMarco Gartmann                        const SourceManager &SM, const LangOptions &LangOpts) {
520685e835SBalazs Benics   if (Destructor.getLocation().isMacroID())
53cd8702efSKazu Hirata     return std::nullopt;
540685e835SBalazs Benics 
55c58c7a6eSMarco Gartmann   SourceLocation VirtualBeginLoc = Destructor.getBeginLoc();
56b2cb7e81SJoachim Priesner   SourceLocation VirtualBeginSpellingLoc =
57b2cb7e81SJoachim Priesner       SM.getSpellingLoc(Destructor.getBeginLoc());
58b2cb7e81SJoachim Priesner   SourceLocation VirtualEndLoc = VirtualBeginSpellingLoc.getLocWithOffset(
59b2cb7e81SJoachim Priesner       Lexer::MeasureTokenLength(VirtualBeginSpellingLoc, SM, LangOpts));
60c58c7a6eSMarco Gartmann 
61c58c7a6eSMarco Gartmann   /// Range ends with \c StartOfNextToken so that any whitespace after \c
62c58c7a6eSMarco Gartmann   /// virtual is included.
63f71ffd3bSKazu Hirata   std::optional<Token> NextToken =
64f71ffd3bSKazu Hirata       Lexer::findNextToken(VirtualEndLoc, SM, LangOpts);
65b2cb7e81SJoachim Priesner   if (!NextToken)
66cd8702efSKazu Hirata     return std::nullopt;
67b2cb7e81SJoachim Priesner   SourceLocation StartOfNextToken = NextToken->getLocation();
68c58c7a6eSMarco Gartmann 
69c58c7a6eSMarco Gartmann   return CharSourceRange::getCharRange(VirtualBeginLoc, StartOfNextToken);
70c58c7a6eSMarco Gartmann }
71c58c7a6eSMarco Gartmann 
72c58c7a6eSMarco Gartmann static const AccessSpecDecl *
getPublicASDecl(const CXXRecordDecl & StructOrClass)73c58c7a6eSMarco Gartmann getPublicASDecl(const CXXRecordDecl &StructOrClass) {
74c58c7a6eSMarco Gartmann   for (DeclContext::specific_decl_iterator<AccessSpecDecl>
75c58c7a6eSMarco Gartmann            AS{StructOrClass.decls_begin()},
76c58c7a6eSMarco Gartmann        ASEnd{StructOrClass.decls_end()};
77c58c7a6eSMarco Gartmann        AS != ASEnd; ++AS) {
78c58c7a6eSMarco Gartmann     AccessSpecDecl *ASDecl = *AS;
79c58c7a6eSMarco Gartmann     if (ASDecl->getAccess() == AccessSpecifier::AS_public)
80c58c7a6eSMarco Gartmann       return ASDecl;
81c58c7a6eSMarco Gartmann   }
82c58c7a6eSMarco Gartmann 
83c58c7a6eSMarco Gartmann   return nullptr;
84c58c7a6eSMarco Gartmann }
85c58c7a6eSMarco Gartmann 
86c58c7a6eSMarco Gartmann static FixItHint
generateUserDeclaredDestructor(const CXXRecordDecl & StructOrClass,const SourceManager & SourceManager)87c58c7a6eSMarco Gartmann generateUserDeclaredDestructor(const CXXRecordDecl &StructOrClass,
88c58c7a6eSMarco Gartmann                                const SourceManager &SourceManager) {
89c58c7a6eSMarco Gartmann   std::string DestructorString;
90c58c7a6eSMarco Gartmann   SourceLocation Loc;
91c58c7a6eSMarco Gartmann   bool AppendLineBreak = false;
92c58c7a6eSMarco Gartmann 
93c58c7a6eSMarco Gartmann   const AccessSpecDecl *AccessSpecDecl = getPublicASDecl(StructOrClass);
94c58c7a6eSMarco Gartmann 
95c58c7a6eSMarco Gartmann   if (!AccessSpecDecl) {
96c58c7a6eSMarco Gartmann     if (StructOrClass.isClass()) {
97c58c7a6eSMarco Gartmann       Loc = StructOrClass.getEndLoc();
98c58c7a6eSMarco Gartmann       DestructorString = "public:";
99c58c7a6eSMarco Gartmann       AppendLineBreak = true;
100c58c7a6eSMarco Gartmann     } else {
101c58c7a6eSMarco Gartmann       Loc = StructOrClass.getBraceRange().getBegin().getLocWithOffset(1);
102c58c7a6eSMarco Gartmann     }
103c58c7a6eSMarco Gartmann   } else {
104c58c7a6eSMarco Gartmann     Loc = AccessSpecDecl->getEndLoc().getLocWithOffset(1);
105c58c7a6eSMarco Gartmann   }
106c58c7a6eSMarco Gartmann 
107c58c7a6eSMarco Gartmann   DestructorString = (llvm::Twine(DestructorString) + "\nvirtual ~" +
108c58c7a6eSMarco Gartmann                       StructOrClass.getName().str() + "() = default;" +
109c58c7a6eSMarco Gartmann                       (AppendLineBreak ? "\n" : ""))
110c58c7a6eSMarco Gartmann                          .str();
111c58c7a6eSMarco Gartmann 
112c58c7a6eSMarco Gartmann   return FixItHint::CreateInsertion(Loc, DestructorString);
113c58c7a6eSMarco Gartmann }
114c58c7a6eSMarco Gartmann 
getSourceText(const CXXDestructorDecl & Destructor)115c58c7a6eSMarco Gartmann static std::string getSourceText(const CXXDestructorDecl &Destructor) {
116c58c7a6eSMarco Gartmann   std::string SourceText;
117c58c7a6eSMarco Gartmann   llvm::raw_string_ostream DestructorStream(SourceText);
118c58c7a6eSMarco Gartmann   Destructor.print(DestructorStream);
119c58c7a6eSMarco Gartmann   return SourceText;
120c58c7a6eSMarco Gartmann }
121c58c7a6eSMarco Gartmann 
eraseKeyword(std::string & DestructorString,const std::string & Keyword)122c58c7a6eSMarco Gartmann static std::string eraseKeyword(std::string &DestructorString,
123c58c7a6eSMarco Gartmann                                 const std::string &Keyword) {
124c58c7a6eSMarco Gartmann   size_t KeywordIndex = DestructorString.find(Keyword);
125c58c7a6eSMarco Gartmann   if (KeywordIndex != std::string::npos)
126c58c7a6eSMarco Gartmann     DestructorString.erase(KeywordIndex, Keyword.length());
127c58c7a6eSMarco Gartmann   return DestructorString;
128c58c7a6eSMarco Gartmann }
129c58c7a6eSMarco Gartmann 
changePrivateDestructorVisibilityTo(const std::string & Visibility,const CXXDestructorDecl & Destructor,const SourceManager & SM,const LangOptions & LangOpts)130c58c7a6eSMarco Gartmann static FixItHint changePrivateDestructorVisibilityTo(
131c58c7a6eSMarco Gartmann     const std::string &Visibility, const CXXDestructorDecl &Destructor,
132c58c7a6eSMarco Gartmann     const SourceManager &SM, const LangOptions &LangOpts) {
133c58c7a6eSMarco Gartmann   std::string DestructorString =
134c58c7a6eSMarco Gartmann       (llvm::Twine() + Visibility + ":\n" +
135c58c7a6eSMarco Gartmann        (Visibility == "public" && !Destructor.isVirtual() ? "virtual " : ""))
136c58c7a6eSMarco Gartmann           .str();
137c58c7a6eSMarco Gartmann 
138c58c7a6eSMarco Gartmann   std::string OriginalDestructor = getSourceText(Destructor);
139c58c7a6eSMarco Gartmann   if (Visibility == "protected" && Destructor.isVirtualAsWritten())
140c58c7a6eSMarco Gartmann     OriginalDestructor = eraseKeyword(OriginalDestructor, "virtual ");
141c58c7a6eSMarco Gartmann 
142c58c7a6eSMarco Gartmann   DestructorString =
143c58c7a6eSMarco Gartmann       (llvm::Twine(DestructorString) + OriginalDestructor +
144c58c7a6eSMarco Gartmann        (Destructor.isExplicitlyDefaulted() ? ";\n" : "") + "private:")
145c58c7a6eSMarco Gartmann           .str();
146c58c7a6eSMarco Gartmann 
147c58c7a6eSMarco Gartmann   /// Semicolons ending an explicitly defaulted destructor have to be deleted.
148c58c7a6eSMarco Gartmann   /// Otherwise, the left-over semicolon trails the \c private: access
149c58c7a6eSMarco Gartmann   /// specifier.
150c58c7a6eSMarco Gartmann   SourceLocation EndLocation;
151c58c7a6eSMarco Gartmann   if (Destructor.isExplicitlyDefaulted())
152c58c7a6eSMarco Gartmann     EndLocation =
153c58c7a6eSMarco Gartmann         utils::lexer::findNextTerminator(Destructor.getEndLoc(), SM, LangOpts)
154c58c7a6eSMarco Gartmann             .getLocWithOffset(1);
155c58c7a6eSMarco Gartmann   else
156c58c7a6eSMarco Gartmann     EndLocation = Destructor.getEndLoc().getLocWithOffset(1);
157c58c7a6eSMarco Gartmann 
158c58c7a6eSMarco Gartmann   auto OriginalDestructorRange =
159c58c7a6eSMarco Gartmann       CharSourceRange::getCharRange(Destructor.getBeginLoc(), EndLocation);
160c58c7a6eSMarco Gartmann   return FixItHint::CreateReplacement(OriginalDestructorRange,
161c58c7a6eSMarco Gartmann                                       DestructorString);
162c58c7a6eSMarco Gartmann }
163c58c7a6eSMarco Gartmann 
check(const MatchFinder::MatchResult & Result)164c58c7a6eSMarco Gartmann void VirtualClassDestructorCheck::check(
165c58c7a6eSMarco Gartmann     const MatchFinder::MatchResult &Result) {
166c58c7a6eSMarco Gartmann 
167c58c7a6eSMarco Gartmann   const auto *MatchedClassOrStruct =
168c58c7a6eSMarco Gartmann       Result.Nodes.getNodeAs<CXXRecordDecl>("ProblematicClassOrStruct");
169c58c7a6eSMarco Gartmann 
170c58c7a6eSMarco Gartmann   const CXXDestructorDecl *Destructor = MatchedClassOrStruct->getDestructor();
171c58c7a6eSMarco Gartmann   if (!Destructor)
172c58c7a6eSMarco Gartmann     return;
173c58c7a6eSMarco Gartmann 
174c58c7a6eSMarco Gartmann   if (Destructor->getAccess() == AccessSpecifier::AS_private) {
175c58c7a6eSMarco Gartmann     diag(MatchedClassOrStruct->getLocation(),
176c58c7a6eSMarco Gartmann          "destructor of %0 is private and prevents using the type")
177c58c7a6eSMarco Gartmann         << MatchedClassOrStruct;
178c58c7a6eSMarco Gartmann     diag(MatchedClassOrStruct->getLocation(),
179b12fd138SKazu Hirata          /*Description=*/"make it public and virtual", DiagnosticIDs::Note)
180c58c7a6eSMarco Gartmann         << changePrivateDestructorVisibilityTo(
181c58c7a6eSMarco Gartmann                "public", *Destructor, *Result.SourceManager, getLangOpts());
182c58c7a6eSMarco Gartmann     diag(MatchedClassOrStruct->getLocation(),
183b12fd138SKazu Hirata          /*Description=*/"make it protected", DiagnosticIDs::Note)
184c58c7a6eSMarco Gartmann         << changePrivateDestructorVisibilityTo(
185c58c7a6eSMarco Gartmann                "protected", *Destructor, *Result.SourceManager, getLangOpts());
186c58c7a6eSMarco Gartmann 
187c58c7a6eSMarco Gartmann     return;
188c58c7a6eSMarco Gartmann   }
189c58c7a6eSMarco Gartmann 
190c58c7a6eSMarco Gartmann   // Implicit destructors are public and non-virtual for classes and structs.
191c58c7a6eSMarco Gartmann   bool ProtectedAndVirtual = false;
192c58c7a6eSMarco Gartmann   FixItHint Fix;
193c58c7a6eSMarco Gartmann 
194c58c7a6eSMarco Gartmann   if (MatchedClassOrStruct->hasUserDeclaredDestructor()) {
195c58c7a6eSMarco Gartmann     if (Destructor->getAccess() == AccessSpecifier::AS_public) {
196c58c7a6eSMarco Gartmann       Fix = FixItHint::CreateInsertion(Destructor->getLocation(), "virtual ");
197c58c7a6eSMarco Gartmann     } else if (Destructor->getAccess() == AccessSpecifier::AS_protected) {
198c58c7a6eSMarco Gartmann       ProtectedAndVirtual = true;
1990685e835SBalazs Benics       if (const auto MaybeRange =
2000685e835SBalazs Benics               getVirtualKeywordRange(*Destructor, *Result.SourceManager,
2010685e835SBalazs Benics                                      Result.Context->getLangOpts()))
2020685e835SBalazs Benics         Fix = FixItHint::CreateRemoval(*MaybeRange);
203c58c7a6eSMarco Gartmann     }
204c58c7a6eSMarco Gartmann   } else {
205c58c7a6eSMarco Gartmann     Fix = generateUserDeclaredDestructor(*MatchedClassOrStruct,
206c58c7a6eSMarco Gartmann                                          *Result.SourceManager);
207c58c7a6eSMarco Gartmann   }
208c58c7a6eSMarco Gartmann 
209c58c7a6eSMarco Gartmann   diag(MatchedClassOrStruct->getLocation(),
210c58c7a6eSMarco Gartmann        "destructor of %0 is %select{public and non-virtual|protected and "
211c58c7a6eSMarco Gartmann        "virtual}1")
212c58c7a6eSMarco Gartmann       << MatchedClassOrStruct << ProtectedAndVirtual;
213c58c7a6eSMarco Gartmann   diag(MatchedClassOrStruct->getLocation(),
214c58c7a6eSMarco Gartmann        "make it %select{public and virtual|protected and non-virtual}0",
215c58c7a6eSMarco Gartmann        DiagnosticIDs::Note)
216c58c7a6eSMarco Gartmann       << ProtectedAndVirtual << Fix;
217c58c7a6eSMarco Gartmann }
218c58c7a6eSMarco Gartmann 
219*7d2ea6c4SCarlos Galvez } // namespace clang::tidy::cppcoreguidelines
220