[clang-tools-extra] [clang-tidy] Create bugprone-incorrect-enable-shared-from-this check (PR #102299)

via cfe-commits cfe-commits at lists.llvm.org
Tue Aug 13 03:34:01 PDT 2024


MichelleCDjunaidi wrote:

@5chmidti so something like this then? ``AssignmentInIfConditionCheck`` in bugprone nests the ``RecursiveASTVisitor``, so I tried following suit, but this feels unreadable and I'll refactor it once it's working. Right now this first draft isn't outputting anything and so fails the llvm-lit test (will troubleshoot and commit once it's working; if anyone has any idea why this doesn't work, do tell).
```
//===--- IncorrectEnableSharedFromThisCheck.cpp - clang-tidy --------------===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//

#include "IncorrectEnableSharedFromThisCheck.h"
#include "clang/AST/DeclCXX.h"
#include "clang/AST/ASTContext.h"
#include "clang/AST/RecursiveASTVisitor.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/Basic/Specifiers.h"
#include "llvm/ADT/SmallPtrSet.h"

using namespace clang::ast_matchers;

namespace clang::tidy::bugprone {

void IncorrectEnableSharedFromThisCheck::registerMatchers(
    MatchFinder *Finder) {
  Finder->addMatcher(translationUnitDecl(), this);
}

void IncorrectEnableSharedFromThisCheck::check(
  const MatchFinder::MatchResult &Result) {

  class Visitor : public RecursiveASTVisitor<Visitor> {
    IncorrectEnableSharedFromThisCheck &Check;
    llvm::SmallPtrSet<CXXRecordDecl*, 8> *EnableSharedClassSet;

  public:
    explicit Visitor(IncorrectEnableSharedFromThisCheck &Check) : Check(Check), EnableSharedClassSet(EnableSharedClassSet) {}
    bool VisitCXXRecordDecl(CXXRecordDecl *RDecl) {

      class BaseVisitor : public RecursiveASTVisitor<BaseVisitor> {
        IncorrectEnableSharedFromThisCheck &Check;
        llvm::SmallPtrSet<CXXRecordDecl*, 8> *EnableSharedClassSet;
        CXXRecordDecl *RDecl;

      public:
        explicit BaseVisitor(IncorrectEnableSharedFromThisCheck &Check) : 
        Check(Check), EnableSharedClassSet(EnableSharedClassSet), RDecl(RDecl) {}

        bool VisitCXXBaseSpecifier(CXXBaseSpecifier &Base) {
          const CXXRecordDecl *BaseType = Base.getType()->getAsCXXRecordDecl();
          const clang::AccessSpecifier AccessSpec = Base.getAccessSpecifier();

          if ( BaseType && BaseType->getQualifiedNameAsString() ==
                              "enable_shared_from_this") {
            if (AccessSpec == clang::AS_public) {
              const SourceLocation InsertLocation = Base.getBaseTypeLoc();
              const FixItHint Hint = FixItHint::CreateInsertion(InsertLocation, "std::");
              Check.diag(RDecl->getLocation(),
                  "Should be std::enable_shared_from_this", DiagnosticIDs::Warning)
                  << Hint;
              
            } else {
              const SourceRange ReplacementRange = Base.getSourceRange();
              const std::string ReplacementString =
                  "public std::" + Base.getType().getAsString();
              const FixItHint Hint =
                  FixItHint::CreateReplacement(ReplacementRange, ReplacementString);
              Check.diag(RDecl->getLocation(),
                  "Should be std::enable_shared_from_this and "
                  "inheritance from std::enable_shared_from_this should be public "
                  "inheritance, otherwise the internal weak_ptr won't be "
                  "initialized",
                  DiagnosticIDs::Warning)
                  << Hint;
            }
          }

          if ( BaseType && BaseType->getQualifiedNameAsString() ==
                              "std::enable_shared_from_this") {
            CXXRecordDecl *CanonicalRDecl = RDecl->getCanonicalDecl();
            EnableSharedClassSet->insert(CanonicalRDecl);
            if (AccessSpec != clang::AS_public) {
              const SourceRange ReplacementRange = Base.getSourceRange();
              const std::string ReplacementString =
                  "public " + Base.getType().getAsString();
              const FixItHint Hint =
                  FixItHint::CreateReplacement(ReplacementRange, ReplacementString);
              Check.diag(
                  RDecl->getLocation(),
                  "inheritance from std::enable_shared_from_this should be public "
                  "inheritance, otherwise the internal weak_ptr won't be initialized",
                  DiagnosticIDs::Warning)
                  << Hint;
            }
          } 
          else if (EnableSharedClassSet->contains(BaseType)) {
            if (AccessSpec != clang::AS_public) {
              const SourceRange ReplacementRange = Base.getSourceRange();
              const std::string ReplacementString =
                  "public " + Base.getType().getAsString();
              const FixItHint Hint =
                  FixItHint::CreateReplacement(ReplacementRange, ReplacementString);
              Check.diag(
                  RDecl->getLocation(),
                  "inheritance from std::enable_shared_from_this should be public "
                  "inheritance, otherwise the internal weak_ptr won't be initialized",
                  DiagnosticIDs::Warning)
                  << Hint;
            }            
          }
          return true;
        }
      };
      
      BaseVisitor(Check).TraverseCXXRecordDecl(RDecl);
      return true;
    }

  };
 
  Visitor(*this).TraverseAST(*Result.Context);
  
}

} // namespace clang::tidy::bugprone
```


https://github.com/llvm/llvm-project/pull/102299


More information about the cfe-commits mailing list